Opened 3 years ago

Closed 3 years ago

#6801 enhancement closed fixed (fixed)


Reported by: hynek Owned by: hynek
Priority: normal Milestone:
Component: core Keywords:
Cc: oberstet 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 3 years ago by oberstet

  • Cc oberstet added

Probably these 2 should be set also:

SSL.OP_SINGLE_DH_USE = 0x00100000L

comment:2 Changed 3 years ago by hynek

  • Author set to hynek
  • Branch set to branches/harden-tls-options-6801

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

comment:3 Changed 3 years ago by hynek

  • 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 3 years ago by exarkun

  • Owner set to exarkun
  • Status changed from new to assigned

comment:5 Changed 3 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to hynek
  • Status changed from assigned to new
  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 3 years ago by hynek

  • Resolution set to fixed
  • Status changed from new to closed

(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.