New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add attribute "ciphers" to ssladapter #22

Merged
merged 5 commits into from Mar 16, 2017

Conversation

Projects
None yet
4 participants
@LenovoCindywang
Contributor

LenovoCindywang commented Mar 16, 2017

Hi Team,
Our security team got a request recently, need to dynamiclly set cipher suites of https connections.
Since Python(version >= 2.7.9)support "context.set_ciphers()", then we can add attribute "ssl_ciphers", and transfer it to "ssl_builtin.py". Thus, if cherrypy use builtin ssl lib, it can dynamicly configure the cipher suites.

I have a solution to add the attribute, could you please merge it once you pass the review?

Thanks,
Cindy

if hasattr(ssl, 'create_default_context'):
self.context = ssl.create_default_context(
purpose=ssl.Purpose.CLIENT_AUTH,
cafile=certificate_chain
)
self.context.load_cert_chain(certificate, private_key)
if(self.ciphers != None):

This comment has been minimized.

@webknjaz

webknjaz Mar 16, 2017

Member

if self.ciphers is not None:

if ssl is None:
raise ImportError('You must install the ssl module to use HTTPS.')
self.certificate = certificate
self.private_key = private_key
self.certificate_chain = certificate_chain
self.ciphers=None

This comment has been minimized.

@webknjaz

webknjaz Mar 16, 2017

Member

self.ciphers = None

This comment has been minimized.

@webknjaz

webknjaz Mar 16, 2017

Member

self.ciphers = ciphers

@@ -43,20 +43,25 @@ class BuiltinSSLAdapter(Adapter):
"""The ssl.SSLContext that will be used to wrap sockets where available
(on Python > 2.7.9 / 3.3)
"""
ciphers = None
"""The ciphers list of SSL."""

This comment has been minimized.

@The-Compiler

The-Compiler Mar 16, 2017

Contributor

I've never seen a docstring used to document variables FWIW

This comment has been minimized.

@The-Compiler

The-Compiler Mar 16, 2017

Contributor

Note that pep was rejected though 😉

This comment has been minimized.

@webknjaz

webknjaz Mar 17, 2017

Member

At least this is supported by linters. Perhaps @jaraco could link smth relevant.

This comment has been minimized.

@webknjaz

webknjaz Mar 17, 2017

Member

The active PEP257 still has attribute docstrings mentions

This comment has been minimized.

@jaraco

jaraco Mar 25, 2017

Member

Yes, attribute docstrings exist. I endorse them despite the modest adoption in the wild.

webknjaz added some commits Mar 16, 2017

@webknjaz webknjaz merged commit b7b03b6 into cherrypy:master Mar 16, 2017

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

webknjaz added a commit to cherrypy/cherrypy that referenced this pull request Mar 17, 2017

jaraco added a commit that referenced this pull request Mar 19, 2017

@jaraco

This comment has been minimized.

Member

jaraco commented Mar 19, 2017

Released as 5.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment