Conversation
mattcaswell
added
branch: master
test/ssl-tests/05-sni.cnf.in
Outdated
There was a problem hiding this comment.
Will this be re-enabled at some point for FIPS_MODE? If so maybe TODO(3.0) it?
Ahh ok NO TLS 1.1 support for FIPS MODE..
test/ssl-tests/20-cert-select.cnf.in
Outdated
test/ssl-tests/20-cert-select.cnf
Outdated
There was a problem hiding this comment.
This reordering sure makes this a lot harder to figure out what changed in this file.. Looks ok from what I can tell.
slontis
left a comment
slontis
left a comment
There was a problem hiding this comment.
Looks ok to me..
Maybe add a few more #TODO(3.0)
I would probably add the comment that TLS 1.1 is not supported in FIPS MODE to the commit.
mattcaswell
changed the title
|
Fixups pushed. Please take another look. |
|
Looks much more sane from a perl perspective |
levitte
changed the title
paulidale
left a comment
paulidale
left a comment
There was a problem hiding this comment.
The TODOs in test/ssl-tests/20-cert-select.cnf.in and test/ssl-tests/28-seclevel.cnf.in are possible once #11371 is in and this PR already depends on that. We should probably fix them before pushing this, although it isn't critical.
Otherwise, I'd approve subject to #11371 going in first and the CIs here working.
|
Woohoo.. No longer pending on #11371 |
mattcaswell
changed the title
mattcaswell
force-pushed
the
run-ssl_test_new_in-fips
branch
from
eec4b32 to
52226ef
Compare
|
Rebased now that #11371 has been merged. I have had to include the same libssl fixup commit that I've added to #11508 here, in order to get the tests to pass. Please don't review the libssl changes in this PR. Please provide any review comments on that aspect in #11508 instead. I've taken this out of WIP, although it cannot now be pushed until #11508 goes in. |
|
Fixup push addressing @paulidale's comment above. |
|
Travis looks unhappy :( |
|
This also needs the no_fips checks added to the .t file. |
|
Fixups pushed hopefully addressing the Travis failures. I've applied a similar fix to the one that @slontis applied to #11135 There's also a fix to ssl3_cbc_digest_record in libssl to make sure we use fetched digests where applicable. So now the fixes related to ssl3_cbc_digest_record in libssl should be reviewed - but the other changes in libssl remain out of scope of this PR (review them in #11508) |
|
Darn - travis was still not happy. Another fixup pushed. |
|
Sigh. Third time lucky...another fixup. |
|
Travis is green!! |
slontis
removed
the
approval: review pending
slontis
added
the
approval: done
We also prepare the way for a future commit to run ssl_test_new with just the FIPS provider loaded.
…ders We now run the tests twice: Once with no specific providers loaded and just using the default libctx, and a second time with a non-default libctx and the default provider. In the second run we disable tests which use a PSS cert/key because we don't yet have support for that.
We load the FIPS module and make sure it is configured before running the ssl_test_new tests.
HMACs used via the legacy EVP_DigestSign interface are strange in that they use legacy codepath's which eventually (under the covers) transform the operation into a new style EVP_MAC. This can mean the digest in use can be a legacy one, so we need to be careful with any digest we extract from the ctx.
mattcaswell
force-pushed
the
run-ssl_test_new_in-fips
branch
from
f307c3f to
66ae5d3
Compare
mattcaswell
changed the title
|
Rebased now that #11508 has gone in. No other changes were made. |
|
@levitte - I would prefer to push this as is, and modify the fipsinstall stuff with a follow on PR since this PR is now otherwise ready-to-merge. |
|
24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually. |
Okie |
mattcaswell
added
approval: ready to merge
We also prepare the way for a future commit to run ssl_test_new with just the FIPS provider loaded. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11511)
…ders We now run the tests twice: Once with no specific providers loaded and just using the default libctx, and a second time with a non-default libctx and the default provider. In the second run we disable tests which use a PSS cert/key because we don't yet have support for that. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11511)
We load the FIPS module and make sure it is configured before running the ssl_test_new tests. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11511)
HMACs used via the legacy EVP_DigestSign interface are strange in that they use legacy codepath's which eventually (under the covers) transform the operation into a new style EVP_MAC. This can mean the digest in use can be a legacy one, so we need to be careful with any digest we extract from the ctx. Reviewed-by: Shane Lontis <[email protected]> (Merged from #11511)
|
Pushed. Thanks! |
|
5 participants
We run ssl_test_new three times:
In both of the latter two cases we load the "null" provider into the default context to make sure we don't accidentally pick up algorithms from there.
We do the run with a default library context because there are some tests which only work in that configuration at the moment. Namely any which involve an RSA PSS cert/key - because we do not have provider support for that at the moment. Eventually I expect we can dispense with that run and just do the second two runs.
These tests will fail since they require all the key gen PRs to be merged first as well as #11494 and #11507 (and I have not included them here). However, aside from the dependencies this should be fairly complete and can be reviewed.