Opened 5 years ago
Closed 4 years ago
#6801 enhancement closed fixed (fixed)
Set OP_NO_COMPRESSION & OP_CIPHER_SERVER_PREFERENCE on TLS contexts
Reported by: | Hynek Schlawack | Owned by: | Hynek Schlawack |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | core | Keywords: | |
Cc: | Tobias Oberstein | Branch: |
branches/harden-tls-options-6801
branch-diff, diff-cov, branch-cov, buildbot |
Author: | hynek |
Description
There is a broad consensus that there are two options that should always be set:
- OP_NO_COMPRESSION (CRIME attacks, seems like most OpenSSLs set that as default nowadays but still)
- OP_CIPHER_SERVER_PREFERENCE (if we push for secure ciphers, we should make sure they get used too)
Both are missing in pyOpenSSL at the moment but we can easily wing it for now.
Confusingly, OP_NO_COMPRESSION is documented in my self-built pyOpenSSL docs ( http://glui.me/?i=3b37w0j9wemcrsl/2013-10-25_at_14.30.png – maybe I’ve built them from a bzr checkout? been a while) and my OpenSSL should have support for it but I can’t find it neither in the official docs at http://pythonhosted.org/pyOpenSSL/openssl-ssl.html nor in the actual library. In any case we’d have to take into account that the underlying (Py)OpenSSL may not support either.
Implementation is trivial, I’d just wait for #6772 to land in trunk because it’s laying groundwork for setting options.
Change History (6)
comment:1 Changed 5 years ago by
Cc: | Tobias Oberstein added |
---|
comment:2 Changed 4 years ago by
Author: | → hynek |
---|---|
Branch: | → branches/harden-tls-options-6801 |
(In [41219]) Branching to harden-tls-options-6801.
comment:3 Changed 4 years ago by
Keywords: | review added |
---|
I’ve done the following:
- Consolidated options handling by baking them into
self._options
on__init__
to avoid repeated calls to set_options in_makeContext
. - Set
OP_NO_COMPRESSION
andOP_CIPHER_SERVER_PREFERENCE
unconditionally. - Bonus: Added tests that
OP_SINGLE_DH_USE
is set ifenableSingleUseKeys
is active and also addedOP_SINGLE_ECDH_USE
in the same case so we don't forget it once we actually add ECDH support.
Buildbots are green, please let me know what you think.
comment:4 Changed 4 years ago by
Owner: | set to Jean-Paul Calderone |
---|---|
Status: | new → assigned |
comment:5 Changed 4 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to Hynek Schlawack |
Status: | assigned → new |
- The docstring for
test_singleUseKeys
says what the test is doing rather than what the implementation is expected to do (though it does so without using the word should for which I guess you get a few points :). - The news fragment says "and makes servers choose the cipher". I think this might be a little ambiguous wrt
CertificateOptions
usage for clients. I think thatOP_CIPHER_SERVER_PREFERENCE
is a no-op on clients? So maybe "and, for servers, uses server preference to choose the cipher." That's a little awkward due to the repeated word "server" but maybe it's an improvement anyway; I'll leave it up to you.
Otherwise looks great, please address those issues and merge. Thanks a lot!
comment:6 Changed 4 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Probably these 2 should be set also: