sectransp: Use common code for cipher suite lookup #13521
Conversation
jan2000
force-pushed
the
sectransp-update-ssl-cipher-list
branch
from
1f7118d
to
1ae7131
Compare
| @@ -235,348 +225,244 @@ struct st_cipher { | |||
| #define CIPHER_STRONG_ENOUGH FALSE | |||
|
|
|||
| /* Please do not change the order of the first ciphers available for SSL. | |||
| Do not insert and do not delete any of them. Code below | |||
| depends on their order and continuity. | |||
| Do not insert and do not delete any of them. | |||
| If you add a new cipher, please maintain order by number, i.e. | |||
| insert in between existing items to appropriate place based on | |||
| cipher suite IANA number | |||
| */ | |||
| static const struct st_cipher ciphertable[] = { | |||
There was a problem hiding this comment.
This big table can be removed (or replaced with something smaller) in a future PR. However, to make sure there is no change in behaviour with this commit it is left in for now.
| @@ -1453,18 +1265,57 @@ static bool is_cipher_suite_strong(SSLCipherSuite suite_num) | |||
| return true; | |||
| } | |||
|
|
|||
| static bool sectransp_is_separator(char c) | |||
| static int sectransp_cipher_suite_get_str(uint16_t id, char *buf, | |||
There was a problem hiding this comment.
These wrapper functions for Curl_cipher_suite_get_str and Curl_cipher_suite_walk_str are here so there is no change in behavior with this commit. But, I guess no one will shed a tear if support for setting/getting these ciphers is removed?
lib/vtls/sectransp.c
Outdated
| return true; | ||
| } | ||
| return false; | ||
| /* are these fortezza suites ever supported ? */ |
There was a problem hiding this comment.
That should be “even” supported. And I have no idea if they’re supported or not.
There was a problem hiding this comment.
Well actually I meant to say "were these ever supported ?", but also in present and future tense. As in: can this be safely removed, or should this be kept in for backward compatibility?
lib/vtls/sectransp.c
Outdated
| size_t len = *end - *str; | ||
|
|
||
| if(!id) { | ||
| /* are these fortezza suites ever supported ? */ |
jan2000
force-pushed
the
sectransp-update-ssl-cipher-list
branch
from
1ae7131
to
7746df8
Compare
Take advantage of the Curl_cipher_suite_walk_str() and Curl_cipher_suite_get_str() functions introduced in commit fba9afe.
jan2000
force-pushed
the
sectransp-update-ssl-cipher-list
branch
from
7746df8
to
18ed50c
Compare
|
@nickzman I made the changes you suggested. See https://github.com/curl/curl/compare/1ae71316953f0aa57e5c6a8ad0cb00b9a7deb317..7746df8cf110219407aeeb07e04167d50b336fe9 However, after pushing I saw CI tests failing due to unit2604. So I rebased to latest master and pushed again. Sorry if this caused any inconvenience for the review. But now I am a bit puzzled as to how the test could be failing on unit2604 in the first place, as it was not yet in the master that this PR was initially based on. How does that work? |
Take advantage of the Curl_cipher_suite_walk_str() and Curl_cipher_suite_get_str() functions introduced in commit fba9afe.