fix: PKCS11 slots/tokens/objects logging #3785
Conversation
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Bravo555
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
Bravo555
had a problem deploying
to
Test Auto
GitHub Actions
Failure
Robot Results
|
Bravo555
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
Bravo555
force-pushed
the
improve/pkcs11-tokens-objects-logging
branch
from
93513da to
5d8ad3e
Compare
Bravo555
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
Bravo555
had a problem deploying
to
Test Auto
GitHub Actions
Failure
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
Bravo555
changed the title
Bravo555
force-pushed
the
improve/pkcs11-tokens-objects-logging
branch
from
5d8ad3e to
69816ad
Compare
Bravo555
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
Bravo555
requested review from
a team,
albinsuresh,
didier-wenzek,
jarhodes314 and
rina23q
as code owners
| #[derive(Debug, Clone, Copy)] | ||
| enum CryptokiSessionType { | ||
| ReadOnly, | ||
| _ReadWrite, |
There was a problem hiding this comment.
Why introduce a case that is never used? Somehow this even makes CryptokiSessionType useless.
There was a problem hiding this comment.
It's a part of #3709 where we do need it because we need to open a RW session, this variant will be used when that's eventually merged.
| debug!(slots = ?get_all_slots_info(&context)); | ||
| debug!(tokens = ?get_all_token_info(&context)); |
There was a problem hiding this comment.
Is debug appropriate? Wouldn't info be better as these lists of slots and tokens are critical to check that a setting is correct.
What I'm missing is how often open_session() is called.
There was a problem hiding this comment.
Is debug appropriate?
We already do print the SlotInfo and TokenInfo of the token we select using debug, so kept debug for printing all to be consistent.
Wouldn't info be better as these lists of slots and tokens are critical to check that a setting is correct.
What I'm missing is how often open_session() is called.
open_session() is called for every request, so having this be info by default would spam the output quite a bit. And it has to be for every request, because slots and objects can change in between.
Bravo555
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
There was a problem hiding this comment.
Approved.
Note that this PR reverts "the slot refresh fix and ignores its test suite", because this fix introduces TLS timeouts. For details see the commit message 7bcf8a9
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Disable the slot refresh fix and ignore its test suite. We do this because in the user's environment, using a Nitrokey HSM, reloading the library is too slow and leads to hitting a TLS timeout and getting a TLS close_notify error: error: Connection error while creating device in Cumulocity: Mqtt state: Mqtt serialization/deserialization error: IO: peer closed connection without sending TLS close_notify: https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof: Mqtt serialization/deserialization error: IO: peer closed connection without sending TLS close_notify: https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof: IO: peer closed connection without sending TLS close_notify: https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof: peer closed connection without sending TLS close_notify: https://docs.rs/rustls/latest/rustls/manual/_03_howto/index.html#unexpected-eof This is the same error that TedgeP11Client::with_ready_check prevents, but the slot fix makes it reappear on that environment. As the slot issue concerns only softhsm which probably isn't used in any production environments, for now we revert this fix until we solve more pressing issues, i.e. thin-edge#3766. Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
Bravo555
force-pushed
the
improve/pkcs11-tokens-objects-logging
branch
from
bcd3df2 to
50daea5
Compare
Bravo555
temporarily deployed
to
Test Pull Request
GitHub Actions
Inactive
reubenmiller
changed the title
reubenmiller
changed the title
Proposed changes
When opening a session to a token as part of a request, all slots and tokens visible by the system will be printed.
We will also print URIs of all objects that we can see on the token.
NOTE: this PR also reverts "#3772", because this fix introduces TLS timeouts. For details see the commit message 7bcf8a9
Types of changes
Paste Link to the issue
Checklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments
This is a follow-up to #3766 that we thought was solved but is not. The issue can't be reliably reproduced, so we need to improve diagnostics for when it eventually reappears.