-
Notifications
You must be signed in to change notification settings - Fork 323
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
TLSv1.3 and options --cipher-list and --seclevel-1 #687
Comments
@martinetd Yes, I had noticed that SSL_CTX_set_cipher_list() apply only to TLSv1.2 or lower. Would a clarification in the docs be sufficient? In the long term I believe it would make sense to add a call to SSL_CTX_set_ciphersuites() in addition to SSL_CTX_set_cipher_list(). While the list of TLSv1.3 ciphers may look sensible today, we might want to disable some ciphers in the long term as a new attacks are disclosed and ciphers get obsolete. |
Having an option in the long term might make sense, for now I believe clarifying the documentation should be enough (e.g. adding a 'for TLS versions <= 1.2' or similar). I'm not convinced adding the option before a usecase requires it is worth it but that's more a policy decision than anything else, leaving that choice up to you :) |
Just to clarify, I wasn't thinking of changing command line options, just calling SSL_CTX_set_ciphersuites() in addition to SSL_CTX_set_cipher_list(), so that TLSv1.3 ciphers can be constrained the same way TLSv1.2 ciphers are constrained. That's really for the sake of correctness, not to support new use cases. This would only apply only for: #if OPENSSL_VERSION_NUMBER >= 0x10100000L
#ifdef TLS1_3_VERSION |
Hm, not sure how reusing the same option would work.. the man page does say setting ciphers should never fail as resolving names is only done later on and unknown ciphers just ignored, so it ought to be possible, but if you give only TLSv1.3 ciphers you'll effectively disable TLSv1.2 (and same the other way around). |
indeed I was thinking of mixing ciphers for TLSv1.2 and below and TLSv1.3 in the same str argument. I'm really not knowledgeable in TLS but is TLSV1.3 really different from TLSv1.2 and lower? Aren't some ciphers specific to TLSv1.0 or TLSv1.2, the same way some are specific to TLSv1.3? Is there a more fundamental difference between TLSv1.2 and below and TLSv1.3? |
As far as I know it's really the same ciphers for SSLv3 + TLS1.0 to 1.2, and no common cipher suite with 1.3.
(I assume I have nothing for ssl3 because my openssl no longer supports it, on an older openssl -ssl3 looks the same as tls1) So using the same string really is abusing it a bit, but I'm curious how openssl would handle e.g. a server compatible with TLS1.3 with no common cipher (no cipher in list) but some cipher given for TLS1.2 -- would it fallback appropriately or error out because no cipher were given? I think whether we can do this or not depends on this test. |
I have tried the same command on Ubuntu 16.04 and Ubuntu 20.04, I don't have access to earlier machines that would support ssl3. Not sure what the Ubuntu 16.04
Ubuntu 20.04
|
Anyway, point is there are no common ciphers so if we give a string to the ciphersuite function without any TLS1.3 cipher we need to test how openssl reacts. From
|
I'm not sure whether our test FortiGate appliance is compatible with TLS1.3:
On the other hand:
EDIT: The Nmap script does not support TLSv1.3: nmap/nmap#1348 |
Anyway, even if the server doesn't support TLSv1.3, the last command I gave should work -- by giving no ciphersuites I would have expected tls1.3 init to fail and fallback to 1.2 |
Indeed the combination of an appliance capable of TLSv1.3 with option
That said this doesn't surprise me, as the user:
What's really bad is that even a combination of TLSv1.2/TLSv1.3 ciphers fails:
This is counter-intuitive as the S_CLIENT(1) man page states that the lists of TLSv1.2 and TLSv1.3 ciphers will be "combined" anyway:
This doesn't make any sense to me. I think issue openssl/openssl#9335 is related - cannot pass unknown ciphers to SSL_CTX_set_ciphersuites(). |
OK, according to this openssl/openssl#10936 (comment), TLSv1.2 and TLSv1.3 ciphers cannot be combined:
This looks like a documentation bug, I've opened a documentation issue (openssl/openssl#11724). As far as openfortivpn is concerned, in the short term we can only update our own documentation and explain |
FortiOS 6.0.x supports TLS 1.1 and TLS 1.2 |
From the man page for SSL_CTX_set_cipher_list:
I noticed openfortivpn only calls SSL_set_cipher_list() so options
--cipher-list
and--seclevel-1
are only valid for TLSv1.2 and below. Perhaps this needs to be clarified in the documentation?The text was updated successfully, but these errors were encountered: