- Download Diff

Summary:

Adding --verify for SSL certificate verification

Review Request #6783 — Created Jan. 16, 2015 and discarded Feb. 8, 2017, 4:18 p.m.

Information
Owner:	abhishek.mukher.g
Repository:	RBTools
Branch:	master
Bugs:
Depends On:
Reviewers
Groups:	rbtools
People:

Description

This is diff is for passing a self signed certificate down through to the SSLContext. This is necessary because python 2.7.9 has recently made urllib2.urlopen and its kin actually verify ssl certificates. I know, how dare they, right? So us peons that don't give money to thawte are S.O.L. without something like this.

This will store everything into an SSL_CONTEXT variable, which can either be a CA file in PEM format, or False. The former will obviously verify against that certificate while the latter will disable certificate stuff entirely.

Some considerations of this approach: * If you hit two different reviewboard servers and one has a self-signed certificate, it is likely there is no value besides False which you can supply that will make both servers happy. I think this could be done with the capath= argument and using a directory of certificates, but I couldn't figure out how openssl wanted the directory structure formatted

NB I just ran tests with python2.6 and they just exploded -- that said I'm late for an engagement so will fix later

Testing Done

Just preliminary testing, hitting our own server, hitting reviewboard.org's server, and hitting some rando's server i don't have the certificate for. Have only tested on python2.7.9 so far

Issues

Description	From	Last Updated
'ReviewBoardServer' imported but unused	reviewbot	Jan. 18, 2015, 7:57 p.m.

Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/transport/sync.py
    rbtools/api/request.py
    rbtools/commands/__init__.py



Tool: Pyflakes
Processed Files:
    rbtools/api/transport/sync.py
    rbtools/api/request.py
    rbtools/commands/__init__.py

I agree that we must do something, and this seems like a reasonable first step.

I'm fine having the options that only apply on 2.7.9, so long as the docstrings say that it's ignored prior to 2.7.9.

We're about to ship 0.7, which sadly won't include this at this point... but if you'd like to continue with it and get it into a landable state, I'd be very grateful.

Summary:

-	Unfinished prototype for self-signed certificate verification
+	Adding --verify for SSL certificate verification

Description:

~		This is an unfinished prototype for passing a self signed certificate down through to the SSLContext. This is necessary because python 2.7.9 has recently made urllib2.urlopen and its kin actually verify ssl certificates. I know, how dare they, right? So us peons that don't give money to thawte are S.O.L. without something like this.
	~	This is an mostly finished version for passing a self signed certificate down through to the SSLContext. This is necessary because python 2.7.9 has recently made urllib2.urlopen and its kin actually verify ssl certificates. I know, how dare they, right? So us peons that don't give money to thawte are S.O.L. without something like this.

~		I was picturing that the verify option would accept a filepath to a ca file, and there may be a --no-verify option to disable it entirely? not sure what the reviewboard style guide would be for making these options only available on 2.7.9 since I couldn't find equivalent methods on versions before that.
	~	This will store everything into an `SSL_CONTEXT` variable, which can either be a CA file in PEM format, or `False`. The former will obviously verify against that certificate while the latter will disable certificate stuff entirely.

~		Well, like i said, this is just a prototype to raise awareness. I'll close it as soon as someone suggests so -- or feel free to if you have this power :)
	~	Some considerations of this approach:
	+	* If you hit two different reviewboard servers and one has a self-signed certificate, it is likely there is no value besides `False` which you can supply that will make both servers happy. I think this could be done with the `capath=` argument and using a directory of certificates, but I couldn't figure out how openssl wanted the directory structure formatted

Testing Done:

~		Little -- just a prototype to raise the issue and maybe start someone off doing the right thing.
	~	Just preliminary testing, hitting our own server, hitting reviewboard.org's server, and hitting some rando's server i don't have the certificate for. Have only tested on python2.7.9 so far

Branch:

+	master

Commit:

-	16c85174ce175ee7c37b6263756156e1d1192359
+	051854c9feaf7ba8bad71bfc41719a0111429a88

Diff:

Revision 2 (+186 -4)

Show changes

	rbtools/api/request.py
	rbtools/api/transport/sync.py
	rbtools/commands/__init__.py
	rbtools/commands/tests.py
	rbtools/commands/testdata/cafiles/digicert.pem

Tool: Pyflakes
Processed Files:
    rbtools/api/transport/sync.py
    rbtools/api/request.py
    rbtools/commands/tests.py
    rbtools/commands/__init__.py

Ignored Files:
    rbtools/commands/testdata/cafiles/digicert.pem



Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/transport/sync.py
    rbtools/api/request.py
    rbtools/commands/tests.py
    rbtools/commands/__init__.py

Ignored Files:
    rbtools/commands/testdata/cafiles/digicert.pem

Description:

		This is an mostly finished version for passing a self signed certificate down through to the SSLContext. This is necessary because python 2.7.9 has recently made urllib2.urlopen and its kin actually verify ssl certificates. I know, how dare they, right? So us peons that don't give money to thawte are S.O.L. without something like this.

		This will store everything into an `SSL_CONTEXT` variable, which can either be a CA file in PEM format, or `False`. The former will obviously verify against that certificate while the latter will disable certificate stuff entirely.

		Some considerations of this approach:
		* If you hit two different reviewboard servers and one has a self-signed certificate, it is likely there is no value besides `False` which you can supply that will make both servers happy. I think this could be done with the `capath=` argument and using a directory of certificates, but I couldn't figure out how openssl wanted the directory structure formatted
	+
	+	NB I just ran tests with python2.6 and they just exploded -- that said I'm late for an engagement so will fix later

Change Summary:

Fixed for python2.6. Apparently it has the ssl module but is missing a bunch of attributes that seem to be required to add this support.

Description:

~		This is an mostly finished version for passing a self signed certificate down through to the SSLContext. This is necessary because python 2.7.9 has recently made urllib2.urlopen and its kin actually verify ssl certificates. I know, how dare they, right? So us peons that don't give money to thawte are S.O.L. without something like this.
	~	This is diff is for passing a self signed certificate down through to the SSLContext. This is necessary because python 2.7.9 has recently made urllib2.urlopen and its kin actually verify ssl certificates. I know, how dare they, right? So us peons that don't give money to thawte are S.O.L. without something like this.

		This will store everything into an `SSL_CONTEXT` variable, which can either be a CA file in PEM format, or `False`. The former will obviously verify against that certificate while the latter will disable certificate stuff entirely.

		Some considerations of this approach:
		* If you hit two different reviewboard servers and one has a self-signed certificate, it is likely there is no value besides `False` which you can supply that will make both servers happy. I think this could be done with the `capath=` argument and using a directory of certificates, but I couldn't figure out how openssl wanted the directory structure formatted

		NB I just ran tests with python2.6 and they just exploded -- that said I'm late for an engagement so will fix later

Commit:

-	051854c9feaf7ba8bad71bfc41719a0111429a88
+	0ee8e9414bec1969e0a63d7fcb2c7590ce3cb52e

Diff:

Revision 3 (+214 -5)

Show changes

	rbtools/api/request.py
	rbtools/api/transport/sync.py
	rbtools/commands/__init__.py
	rbtools/commands/tests.py
	rbtools/commands/testdata/cafiles/digicert.pem

Fix it!

Tool: PEP8 Style Checker
Processed Files:
    rbtools/api/transport/sync.py
    rbtools/api/request.py
    rbtools/commands/tests.py
    rbtools/commands/__init__.py

Ignored Files:
    rbtools/commands/testdata/cafiles/digicert.pem



Tool: Pyflakes
Processed Files:
    rbtools/api/transport/sync.py
    rbtools/api/request.py
    rbtools/commands/tests.py
    rbtools/commands/__init__.py

Ignored Files:
    rbtools/commands/testdata/cafiles/digicert.pem

rbtools/commands/tests.py (Diff revision 3)
Show all issues
```
 'ReviewBoardServer' imported but unused
```

You have a pending review.

Review Board 5.0.2 alpha 0 (dev)

Adding --verify for SSL certificate verification

Summary:

Description:

Testing Done:

Branch:

Commit:

Diff:

Description:

Change Summary:

Description:

Commit:

Diff:

Status: Discarded

rbtools/commands/tests.py (Diff revision 3)