feat: tedge cert renew c8y using HSM#3677
Merged
Bravo555 merged 2 commits intothin-edge:mainfrom Jul 2, 2025
Merged
Conversation
Contributor
Robot Results
|
5599cbc to
6636118
Compare
6636118 to
1bcdda9
Compare
c1055c2 to
1bcdda9
Compare
1bcdda9 to
9962540
Compare
Contributor
|
@Bravo555 The feature works nicely. The only open question would be if the errors messages could be a bit more informative when the # tedge cert renew c8y
Error: failed to renew the device certificate via Cumulocity HTTP proxy http://127.0.0.1:8001
Caused by:
0: Cryptography related error
1: Remote key errorThe above error doesn't tell anything about the tedge-p11-server not being reachable, which socket is being used, etc, so it would definitely help people debug a bit of the error message included more context. |
9962540 to
abfecbd
Compare
abfecbd to
8b43a28
Compare
55ca816 to
18c056b
Compare
18c056b to
3feecbb
Compare
3feecbb to
c81a5bf
Compare
Instead of returning a rustls::SigningKey, which can't immediately be used for signing, make tedge_p11_server::signing_key return an object on which one can call .sign() immediately. rustls::SigningKey first requires calling `choose_scheme` to obtain a signer that could be used for signing. So far `tedge_p11_server::signing_key` was called by only one caller which wanted a rustls::SigningKey, but now for CSR we want to call `.sign()`. There were also some changes in the pkcs11 module to remove some unnecessary structs that in practice were really duplicates - instead of having a separate `Pkcs11SigningKey` and `Pkcs11Signer` structs, just have `Pkcs11Signer` impl both `SigningKey` and `Signer`. Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
c81a5bf to
09fb381
Compare
Member
Author
|
@reubenmiller @didier-wenzek Change made in #3706 to start with a cert signed by C8y CA made the test stop testing if renewing a self-signed certificate actually works, so I changed it back to upload a self-signed cert when testing renewal. |
didier-wenzek
approved these changes
Jul 1, 2025
| @@ -7,6 +7,7 @@ Documentation Test thin-edge.io MQTT client authentication using a Hardwar | |||
|
|
|||
| # it would be good to explain here why we use the tedge-p11-server exclusively and not the module mode | |||
Contributor
There was a problem hiding this comment.
It might be good to resolve this todo.
| let current_cert = self | ||
| .current_cert | ||
| .clone() | ||
| .expect("Need an existing cert when using an HSM"); |
Contributor
There was a problem hiding this comment.
A bit surprising, but okay as a temporary solution.
This commit introduces support for tedge cert renew c8y when the private key is located on the HSM. To generate a CSR, one needs to put the public key in the CSR (it will become a part of the certificate (SPKI)) and to sign the certificate using the private key to prove the ownership of it. Because the public key can be derived from the private key, and because we need to sign anyway, until this PR we just read the private key to do both these things. Now, with an HSM, we can't read the private key, so instead for CSR creation with HSM being enabled, we read the public key from the currently used certificate and sign using the TedgeP11Client. Signed-off-by: Marcel Guzik <marcel.guzik@cumulocity.com>
0b97869 to
b5222c8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TODO
Proposed changes
This PR introduces support for
tedge cert renew c8ywhen the private key is located on the HSM.To generate a CSR, one needs to put the public key in the CSR (it will become a part of the certificate (SPKI)) and to sign the certificate using the private key to prove the ownership of it. Because the public key can be derived from the private key, and because we need to sign anyway, until this PR we just read the private key to do both these things.
Now, with an HSM, we can't read the private key, so instead for CSR creation with HSM being enabled, we read the public key from the currently used certificate and sign using the
TedgeP11Client.It's also perhaps possible to derive the public key from the public attributes of the private key in the HSM (that's what gnutls seems to be doing), so that would be an alternative approach to reusing the pubkey from the certificate that needs to be considered as well.
Types of changes
Paste Link to the issue
#3664
Checklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments