Repository navigation
[fix] support AES128, AES256 and CHACHA20 cipher aliases - #390
Conversation
These were not registered, so ciphers = 'AES128' raised "no cipher match", and in a "+" rule the unknown name was skipped: EECDH+CHACHA20 and EECDH+AES128 both selected every EECDH suite. AES128 and AES256 match on key size. An unknown name in a "+" rule now drops the whole rule, as libssl's ssl_cipher_process_rulestr does.
| else { | ||
| matching.retainAll( matchingPattern(pattern, all, false, setSuite) ); | ||
| } | ||
| if ( pattern == null ) return null; // unknown name drops the rule (libssl ssl_cipher_process_rulestr) |
There was a problem hiding this comment.
this will reject valid combined rules containing an individual cipher name
OpenSSL accepts AES128+ECDHE-RSA-AES128-GCM-SHA256 and select that suite
PR raises OpenSSL::SSL::SSLError: no cipher match
we should resolve offered cipher names, including RFC names, before applying the unknown rule...
There was a problem hiding this comment.
Thanks, should be fixed now. Seems like it was broken before this PR too, master raises no cipher match here.
A + rule only looked up aliases, so cipher names got skipped (EECDH+ECDHE-RSA-AES128-GCM-SHA256 gave all of EECDH). An RFC name as a plain string didn't work either, only inside an Array.
Now each part can be an alias, an OpenSSL name or an RFC name, and the rule is what they have in common, like CRuby:
ctx.ciphers = "AES128+ECDHE-RSA-AES128-GCM-SHA256" # ECDHE-RSA-AES128-GCM-SHA256
ctx.ciphers = "AES128+TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" # ECDHE-RSA-AES128-GCM-SHA256
ctx.ciphers = "AES256+ECDHE-RSA-AES128-GCM-SHA256" # no cipher match
ctx.ciphers = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" # works nowThere was a problem hiding this comment.
yeah, sorry - seems like it was broken indeed.
didn't realize that, just recalled we support the raw names and that they likely won't cut if with the changes.
2 participants
These were not registered, so ciphers = 'AES128' raised "no cipher match", and in a "+" rule the unknown name was skipped: EECDH+CHACHA20 and EECDH+AES128 both selected every EECDH suite.
AES128 and AES256 match on key size. An unknown name in a "+" rule now drops the whole rule, as libssl's ssl_cipher_process_rulestr does.
Found while checking falcon's TLS tests. They failed on 0.19.3 because of the missing EECDH (fixed in #387). The localhost gem's SERVER_CIPHERS (EECDH+CHACHA20:EECDH+AES128:...) then still selected the wrong suites on master. Falcon's tests pass either way, since they don't check which suites are chosen.