ssl: Revert checking of legacy schemes #8886
Conversation
IngelaAndin
added
team:PS
CT Test Results 2 files 67 suites 49m 25s ⏱️ Results for commit 71bb9f5. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
IngelaAndin
force-pushed
the
ingela/ssl/legacy-cert-sing/ERIERL-1137/OTP-19249
branch
from
1785f15
to
95259b5
Compare
|
| ssl_test_lib:basic_test(COpts ++ ClientOpts, SOpts ++ ServerOpts, Config). | ||
|
|
||
| root_key(SHA) -> | ||
| %% As rsa keygen is not guaranteed to be fast |
There was a problem hiding this comment.
I'm not sure what comment above relates to.
If it explaining why hardcoded RSA key is used, maybe would be better to include that in single placed comment for ssl_test_lib:hardcode_rsa_key rather than having it in every place when this function is callled?
There was a problem hiding this comment.
Comment is copy pasted. Yes it means that generating rsa keys on the fly might some odd time be very time consuming so of we do it for all test it is bound to happen sooner or later giving flaky test cases.
IngelaAndin
force-pushed
the
ingela/ssl/legacy-cert-sing/ERIERL-1137/OTP-19249
branch
from
95259b5
to
d09a988
Compare
As comment above indicate in branch name.
It was first broken in: a8f1f4d Partly fixed in 682b59f. rsa_pcks1_sha256 and rsa_pss_rsae_sha256 are different signature algorithms and supporting one does not mean we support the other. But if we have certificate using the first we can use the latter for the protocol signatures if it is supported. |
IngelaAndin
force-pushed
the
ingela/ssl/legacy-cert-sing/ERIERL-1137/OTP-19249
branch
from
d09a988
to
09cdd39
Compare
| signature_algs_cert(_,_,_) -> | ||
| undefined. |
There was a problem hiding this comment.
Yes it can happen for pre TLS-1.2 versions as well as when only signature_algs option is explicitly set but signature_algs_cert option is not. Proof from only running one test suite:
375 | | signature_algs_cert(Version, SslOpts, SigAlgs) when ?TLS_GTE(Version, ?TLS_1_2) ->
376 | | case maps:get(signature_algs_cert, SslOpts, undefined) of
377 | 145 | undefined ->
378 | | SigAlgs;
379 | 117 | SigAlgsCert ->
380 | | ssl_handshake:supported_hashsigns(SigAlgsCert)
381 | 28 | end;
382 | | signature_algs_cert(_,_,_) ->
383 | | undefined.
384 | 124 |
Revert back to original implementation for checking of legacy schemes as it was broken by refactor in OTP-26.2.1 and later only partly fixed as the refactor change how the code was interpreted. Now also add explicit test for these algorithms.
IngelaAndin
force-pushed
the
ingela/ssl/legacy-cert-sing/ERIERL-1137/OTP-19249
branch
from
09cdd39
to
71bb9f5
Compare
Revert back to original implementation for checking of legacy schemes as it was broken by refactor in OTP-26-2.1 and later only partly fixed as the refactor change how the code was interpreted. Now also add explicit test for these algorithms.