Opened 6 years ago

Closed 5 years ago

#6801 enhancement closed fixed (fixed)


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


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 ( – 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 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 6 years ago by Tobias Oberstein

Cc: Tobias Oberstein added

Probably these 2 should be set also:

SSL.OP_SINGLE_DH_USE = 0x00100000L

comment:2 Changed 5 years ago by Hynek Schlawack

Author: hynek
Branch: branches/harden-tls-options-6801

(In [41219]) Branching to harden-tls-options-6801.

comment:3 Changed 5 years ago by Hynek Schlawack

Keywords: review added

I’ve done the following:

  1. Consolidated options handling by baking them into self._options on __init__ to avoid repeated calls to set_options in _makeContext.
  3. Bonus: Added tests that OP_SINGLE_DH_USE is set if enableSingleUseKeys is active and also added OP_SINGLE_ECDH_USEin 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 5 years ago by Jean-Paul Calderone

Owner: set to Jean-Paul Calderone
Status: newassigned

comment:5 Changed 5 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: changed from Jean-Paul Calderone to Hynek Schlawack
Status: assignednew
  1. 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 :).
  2. 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 that OP_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 5 years ago by Hynek Schlawack

Resolution: fixed
Status: newclosed

(In [41286]) Merge harden-tls-options-6801

Author: hynek Reviewer: exarkun Fixes: #6801

twisted.internet.ssl.CertificateOptions now disables TLS compression to avoid CRIME attacks and, for servers, uses server preference to choose the cipher.

Note: See TracTickets for help on using tickets.