-
Notifications
You must be signed in to change notification settings - Fork 47
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
CI: Simplify kryoptic build #473
Conversation
58160c4
to
f666ae6
Compare
f666ae6
to
a952b8b
Compare
Ok, not ready yet. The tls test fails now with kryoptic when we force all operations to the token
Not sure if this is the relevant error or something else got broken (or the dynamic feature messes up the openssl context). Will have to investigate further. |
a952b8b
to
6c47890
Compare
Kryoptic supports linking against system OpenSSL, which should be much faster than rebuilding the whole OpenSSL. Signed-off-by: Jakub Jelen <[email protected]>
6c47890
to
e4cb06a
Compare
I pushed a commit that resolves most of the TLS errors, which were due to us "poisoning" the openssl error stack emitting suprious messages when a pkcs#11 API call was failing. Sometimes those failures are expected and are properly handled, but the OpenSSL TLS code considers any dirty stack to be a fatal error even if the functions it called all succeeded. This patch makes all the TLS tests green however there is still an error left in softhsm / rsapssam that manifests this way:
|
Somehow the rsapssam error in softhsm appears ... once we suppress the errors we emit by pkcs11-provider itself ... I am baffled how this can be, but here we are, playing whack-a-mole ... |
The plot thickens, if I revert the commit I added and run the whole test suite then all softhsm tests pass. |
Unfortunately by the time I was ready to run it in gdb the error "disappeared" and now I can't easily repro it again ... now it fails like once every 10 tries, which reinforces my suspicion that there is a memory layout issue somewhere paired with some buffer overrun |
So more and more interesting, although it is signature verification that fails it seems to be the generated signature that is bad (signature verification happens fully within openssl, only the generation happens in the token |
Tested with valgrind but found no apparent issue, yet this time I got a different verification error:
|
The bin file is exactly 256 bytes in length, and I wonder if this is an error with 2048 bit keys which have a modulus that simply does not have enough bits in the most significant byte. |
P11PROV_raise() emits errors in the openssl error stack. The OpenSSL TLS code checks the error stack and fails TLS operations if any error is found on the stack. Change pkcs11-provider code to not emit on the openssl error stack when operations fail, as sometimes that is expected and the code can still complete the overall requested operation via fallbacks. The code now emits only a debug error, which is routed to the pkcs11-provider debug file and does not poison the OpenSSL error stack. NOTE: This requires a change in a test. As we change the errors we throw on the Stack OpenSSL interprests the error differently and prints a different error message. Signed-off-by: Simo Sorce <[email protected]>
41426ec
to
6f02859
Compare
lgtm (can't approve my PR)! |
Description
Kryoptic supports linking against system OpenSSL, which should be much faster than rebuilding the whole OpenSSL.
I do not think we need the build against specific OpenSSL version now.
Reviewer's checklist: