feat: Integration with Cumulocity CA by didier-wenzek · Pull Request #3259 · thin-edge/thin-edge.io

didier-wenzek · 2024-11-21T16:15:48Z

Proposed changes

Impl tedge cert download c8y
Impl tedge cert renew c8y
Enhance tedge config with setting for certificate validity period
Enhance tedge cert show to display number of days before expiration
Enhance tedge cert show to display certificate serial number
Enhance tedge cert show to display any cert
Get and renew certificates using CSRs that have been generated elsewhere (typically an HSM)
Fix tedge cert renew arguments: cloud vs ca
~~Make sure the current certificate is not erased on renewal~~ This will be done by an external tool (wrapping the renewal with a reconnect and a rollback on error)

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
Documentation Update (if none of the other choices apply)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3248

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA (in all commits with git commit -s)
I ran cargo fmt as mentioned in CODING_GUIDELINES
I used cargo clippy as mentioned in CODING_GUIDELINES
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

codecov · 2024-11-21T16:30:25Z

Codecov Report

Attention: Patch coverage is 35.48951% with 369 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...tes/core/tedge/src/cli/certificate/c8y/download.rs	0.00%	88 Missing ⚠️
crates/core/tedge/src/cli/certificate/cli.rs	19.60%	81 Missing and 1 partial ⚠️
crates/core/tedge/src/cli/certificate/c8y/renew.rs	0.00%	63 Missing ⚠️
crates/core/tedge/src/cli/certificate/show.rs	40.00%	37 Missing and 8 partials ⚠️
crates/core/tedge/src/cli/certificate/c8y/mod.rs	62.38%	36 Missing and 5 partials ⚠️
crates/core/tedge/src/cli/certificate/create.rs	62.16%	12 Missing and 2 partials ⚠️
crates/common/certificate/src/lib.rs	77.50%	6 Missing and 3 partials ⚠️
crates/core/tedge/src/cli/certificate/mod.rs	0.00%	9 Missing ⚠️
...common/tedge_config/src/tedge_toml/tedge_config.rs	72.72%	0 Missing and 6 partials ⚠️
...rates/core/tedge/src/cli/certificate/create_csr.rs	60.00%	1 Missing and 3 partials ⚠️
... and 4 more

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

crates/core/tedge/src/cli/certificate/download.rs

crates/core/tedge/src/cli/certificate/c8y/renew.rs

github-actions · 2024-11-28T14:08:46Z

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
603	0	3	603	100	1h45m24.206169999s

reubenmiller · 2025-03-24T15:51:48Z

Cert renewal

To start out, the certificate renewal will be facilitated by an external process (default using system timer service) to actively renew the certificate.

In order to make it easy for external processes to trigger a certificate renewal, there needs to be a way a process can check if the certificate is about to expire (with the minimum value as defined in certificate.validity.minimum_duration in the tedge.toml).

The command used to check the validity of the certificate should produce the following exit code, so that the calling process can respond accordingly. There are several options where the above certificate validity can be placed, below are some options.

Inspiration for this type of renewal is taken from smallstep

Option 1: New subcommand to check if the cert needs renewal or not

tedge cert needs-renewal c8y

Exit codes:

0 - certificate needs renewal as it is no longer valid, or it will expire within the duration, certificate.validity.minimum_duration
1 - certificate is still valid and does not need renewal
2 - unexpected error (e.g. certificate does not exist, or can't be read)

Option 2: Flag in renew command

tedge cert renew c8y --needs-renewal

Exit codes:

0 - certificate needs renewal as it is no longer valid, or it will expire within the duration, certificate.validity.minimum_duration
1 - certificate is still valid and does not need renewal
2 - unexpected error (e.g. certificate does not exist, or can't be read)

didier-wenzek · 2025-03-24T16:09:27Z

Exit codes:

0 - certificate needs renewal as it is no longer valid, or it will expire within the duration, certificate.validity.minimum_duration

1 - certificate is still valid and does not need renewal

2 - unexpected error (e.g. certificate does not exist, or can't be read)

I would return a non 0 value when the certificate is already expired - because it can no more be renew then. But maybe, this is a design choice to proceed in such in a case (maybe device time is not correctly set and the certificate is actually still valid). Isn't it?

reubenmiller · 2025-03-24T16:17:25Z

Exit codes:

0 - certificate needs renewal as it is no longer valid, or it will expire within the duration, certificate.validity.minimum_duration

1 - certificate is still valid and does not need renewal

2 - unexpected error (e.g. certificate does not exist, or can't be read)

I would return a non 0 value when the certificate is already expired - because it can no more be renew then. But maybe, this is a design choice to proceed in such in a case (maybe device time is not correctly set and the certificate is actually still valid). Isn't it??

Yes basically. The renewal might not work, but at least it will be attempted. This choice was also taken from the smallstep cli command, step certificate needs-renewal

Below is the example of the exist code description from their docs:

This command returns '0' if the X509 certificate needs renewal, '1' if the X509 certificate does not need renewal, '2' if the X509 certificate file does not exist, and '255' for any other error.

didier-wenzek · 2025-03-26T10:02:46Z

Cert renewal

Option 1: New subcommand to check if the cert needs renewal or not

Implemented with ea03cc5

reubenmiller · 2025-03-27T20:02:15Z

crates/core/tedge/src/cli/certificate/cli.rs

+                    }
+                };
+
+                if is_self_signed {


It would be useful if we allow users to migrate from a self-signed certificate to a certificate issued by the Cumulocity CA, as this is probably the position that a lot of thin-edge.io c8y users are currently in.

So it would be useful to be able to control whether we're re-creating a new self-signed certificate, or doing the default action to request a new certificate via the new Cumulocity CA service.

Good point. Indeed, directly asking c8y CA for a renewal should work in such a case despite the current certificate having not been issued by v8y.

Yeah I can confirm that Cumulocity can issue a new certificate to the device (as it is using the JWT for validation and not the certificate, which works nicely)

I see two options, my preference being for the second one.

implicit:

When the certificate is used to connect c8y, try first to post the renewal request to c8y CA and fallback to a self-signed certificate when the response is a 404 and the current certificate is self-signed.

Warn the user when the new certificate is self-signed: this certificate has to be uploaded on the tenant to be trusted.

Pros: No new command line options

Cons: the command seems simpler to use, but it is not, as a second critical step might be required.

explicit:

Add a --self-signed option (which default is false).

When --self-signed is false and the endpoint is c8y, then request the renewal to c8y CA. Let the current certificate unchanged if this fails.

Return an error if --self-signed is true but the certificate is not self-signed.

Return an error if --self-signed is false and the endpoint is not c8y.

Cons: This changes how tedge cert renew behaves

Pros: Make self-signed certificates second-class citizens

It would be useful if we allow users to migrate from a self-signed certificate to a certificate issued by the Cumulocity CA, as this is probably the position that a lot of thin-edge.io c8y users are currently in.

Similarly, we can support users moving away from device passwords.

So it would be useful to be able to control whether we're re-creating a new self-signed certificate, or doing the default action to request a new certificate via the new Cumulocity CA service.

And, along the same lines, it would be useful to improve tedge cert download or tedge cert renew so a very first certificate can be retrieved using the renewal API and authenticating via JWT.

Add a --self-signed option (which default is false).

Implemented: db15855

rina23q

The changes are very clear and easy to read. I was not able to test with c8y fully since my tenant is missing something for now. I'll test again once my tenant issue is solved.

crates/core/tedge/src/cli/certificate/show.rs

crates/core/tedge/src/cli/certificate/c8y/download.rs

rina23q

It works like a charm, really nice feature! Now it's super easy to register a device in c8y!

reubenmiller

Approved (from a functional level). It has all of the fundamental elements to build a renewal service on top.

Bravo555

As for the code itself, LGTM, only managed to find a single typo. But are documentation additions, like a reference or a user guide planned in this PR or in a follow-up?

Bravo555 · 2025-03-31T18:54:08Z

Cargo.toml

 proptest = "1.0"
 quote = "1"
 rand = "0.8"
+rasn = "0.18.0"


question: Is there a specific reason why we're not using the latest version?

Regarding crate versions, long time ago, we decided to specify until minor version (like 0.18), but I see now our Cargo.toml is mixed of major.minor.patch and major.minor. I'm also curious why here is 3 digits.

Because not compatible with our current MSRV.

Added a comment and removed the patch suffix: e777d3e

Bravo555 · 2025-04-01T08:53:04Z

crates/core/tedge/src/cli/certificate/cli.rs

+        /// This can be used to bypass the default behavior
+        /// which is to forward the renewal request to the cloud CA
+        /// even if the current certificate has not been signed by this CA.
+        /// In most case, the default behavior is what you want:


typo:

Suggested change

/// In most case, the default behavior is what you want:

/// In most cases, the default behavior is what you want:

fixed bf2f748

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

albinsuresh

LGTM.

albinsuresh · 2025-04-01T10:58:54Z

crates/common/tedge_config/src/tedge_toml/tedge_config.rs

+            #[tedge_config(example = "365d", default(from_str = "365d"))]
+            requested_duration: SecondsOrHumanTime,
+
+            /// Minimum validity duration bellow which a new certificate should be requested


Suggested change

/// Minimum validity duration bellow which a new certificate should be requested

/// Minimum validity duration below which a new certificate should be requested

Fixed: ff6c58f

albinsuresh · 2025-04-01T11:32:27Z

crates/core/tedge/src/cli/certificate/cli.rs

+    #[clap(verbatim_doc_comment)]
+    /// Request and download the device certificate from Cumulocity
+    ///
+    /// - Generate a private key and Signing Certificate Request (CSR) for the device


Suggested change

/// - Generate a private key and Signing Certificate Request (CSR) for the device

/// - Generate a private key and Certificate Signing Request (CSR) for the device

albinsuresh · 2025-04-01T11:33:21Z

crates/core/tedge/src/cli/certificate/cli.rs

+    /// - Generate a private key and Signing Certificate Request (CSR) for the device
+    /// - Upload this CSR on Cumulocity, using the provided device identifier and security token
+    /// - Loop till the device is registered by an administrator and the CSR accepted
+    /// - Store the certificate created by Cumulocity


Suggested change

/// - Store the certificate created by Cumulocity

/// - Download and store the certificate created by Cumulocity

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Previously, this command was only able to show the device certificate (for some cloud profile) Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

The current implementation is simplified knowing that `tedge cert renew` only supports self-signed certificates and certificates signed by c8y and more precisely by the CA of the c8y tenant the device is connected to. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

This licence is indirectly required by `rasn-cms` the crate used to decode pkcs#7 certificates received from EST. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

…gned Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

jarhodes314 · 2025-04-01T13:57:56Z

crates/core/tedge/src/cli/certificate/c8y/download.rs

+
+impl DownloadCertCmd {
+    async fn download_device_certificate(&self) -> Result<(), Error> {
+        let (common_name, security_token) = self.get_registration_data()?;


This should probably use spawn_blocking

This has been addressed by ec2bf68

didier-wenzek had a problem deploying to Test Pull Request November 21, 2024 16:15 — with GitHub Actions Failure

didier-wenzek added feature Change request theme:c8y Theme: Cumulocity related topics theme:certificates Theme: Device certificate topics labels Nov 21, 2024

didier-wenzek had a problem deploying to Test Pull Request November 21, 2024 20:25 — with GitHub Actions Failure

didier-wenzek commented Nov 21, 2024

View reviewed changes

didier-wenzek force-pushed the feat/integration-with-cumulocity-ca branch from 844dc34 to 284c8ee Compare November 22, 2024 10:16

didier-wenzek had a problem deploying to Test Pull Request November 22, 2024 10:16 — with GitHub Actions Failure

didier-wenzek force-pushed the feat/integration-with-cumulocity-ca branch from 284c8ee to fba8ed6 Compare November 22, 2024 16:00

didier-wenzek had a problem deploying to Test Pull Request November 22, 2024 16:00 — with GitHub Actions Failure

didier-wenzek had a problem deploying to Test Pull Request November 22, 2024 16:41 — with GitHub Actions Failure

didier-wenzek commented Nov 25, 2024

View reviewed changes

crates/core/tedge/src/cli/certificate/c8y/renew.rs Outdated Show resolved Hide resolved

didier-wenzek force-pushed the feat/integration-with-cumulocity-ca branch from 5f284b5 to 5335af8 Compare November 28, 2024 13:46

didier-wenzek temporarily deployed to Test Pull Request November 28, 2024 13:46 — with GitHub Actions Inactive

didier-wenzek temporarily deployed to Test Auto November 28, 2024 13:52 — with GitHub Actions Inactive

didier-wenzek temporarily deployed to Test Pull Request November 28, 2024 14:38 — with GitHub Actions Inactive

didier-wenzek had a problem deploying to Test Auto November 28, 2024 14:45 — with GitHub Actions Failure

didier-wenzek temporarily deployed to Test Auto November 28, 2024 16:13 — with GitHub Actions Inactive

didier-wenzek had a problem deploying to Test Pull Request December 2, 2024 09:31 — with GitHub Actions Failure

didier-wenzek temporarily deployed to Test Pull Request December 2, 2024 17:13 — with GitHub Actions Inactive

didier-wenzek had a problem deploying to Test Auto December 2, 2024 17:20 — with GitHub Actions Failure

didier-wenzek had a problem deploying to Test Pull Request December 3, 2024 14:46 — with GitHub Actions Failure

didier-wenzek temporarily deployed to Test Pull Request December 3, 2024 15:11 — with GitHub Actions Inactive

didier-wenzek temporarily deployed to Test Auto December 3, 2024 15:18 — with GitHub Actions Inactive

reubenmiller had a problem deploying to Test Pull Request December 3, 2024 22:56 — with GitHub Actions Failure

didier-wenzek force-pushed the feat/integration-with-cumulocity-ca branch from 8ee9b20 to abfa57e Compare December 4, 2024 17:29

didier-wenzek temporarily deployed to Test Pull Request December 4, 2024 17:29 — with GitHub Actions Inactive

didier-wenzek temporarily deployed to Test Auto December 4, 2024 17:35 — with GitHub Actions Inactive

reubenmiller reviewed Mar 27, 2025

View reviewed changes

rina23q reviewed Mar 28, 2025

View reviewed changes

crates/core/tedge/src/cli/certificate/show.rs Show resolved Hide resolved

crates/core/tedge/src/cli/certificate/c8y/download.rs Show resolved Hide resolved

rina23q approved these changes Mar 31, 2025

View reviewed changes

reubenmiller approved these changes Mar 31, 2025

View reviewed changes

Bravo555 reviewed Apr 1, 2025

View reviewed changes

Group c8y certificate-related feature in a new module

037836b

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Bravo555 approved these changes Apr 1, 2025

View reviewed changes

albinsuresh approved these changes Apr 1, 2025

View reviewed changes

didier-wenzek and others added 17 commits April 1, 2025 14:25

Impl tedge cert download c8y

990f869

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Impl tedge cert renew c8y

abb16d2

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

tedge cert download --retry-every and --max-timeout

c3f4ce2

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Improve error messages

81b40e3

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

add on demand system test (not run by default in ci)

7eddbcf

Make 'tedge cert download' EST compliant

bcc9278

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Improve tedge cert show with cert serial number

8aa99fd

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Improve tedge cert show with validity duration

6518b55

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Improve tedge cert show to show any certificate

03aaa31

Previously, this command was only able to show the device certificate (for some cloud profile) Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Ability to provide the CSR to tedge cert download and renew

c54ce91

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Improve tedge cert show with colour status

5b4c6c8

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

New tedge config settings for CSR defaults

9c3501a

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Fix just format warning

489286a

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Allow Zlib license

1e496e5

This licence is indirectly required by `rasn-cms` the crate used to decode pkcs#7 certificates received from EST. Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Add tedge cert need-renewal

c3a6ecf

Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

Add --self-signed option to renew self-signed certificates as self-si…

f625699

…gned Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>

jarhodes314 reviewed Apr 1, 2025

View reviewed changes

	/// In most case, the default behavior is what you want:
	/// In most cases, the default behavior is what you want:

	/// Minimum validity duration bellow which a new certificate should be requested
	/// Minimum validity duration below which a new certificate should be requested

	/// - Generate a private key and Signing Certificate Request (CSR) for the device
	/// - Generate a private key and Certificate Signing Request (CSR) for the device

	/// - Store the certificate created by Cumulocity
	/// - Download and store the certificate created by Cumulocity

Conversation

didier-wenzek commented Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Types of changes

Paste Link to the issue

Checklist

Further comments

Uh oh!

codecov bot commented Nov 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Nov 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Robot Results

Uh oh!

reubenmiller commented Mar 24, 2025

Cert renewal

Option 1: New subcommand to check if the cert needs renewal or not

Option 2: Flag in renew command

Uh oh!

didier-wenzek commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

reubenmiller commented Mar 24, 2025 • edited by didier-wenzek Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

didier-wenzek commented Mar 26, 2025

Cert renewal

Option 1: New subcommand to check if the cert needs renewal or not

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rina23q left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rina23q left a comment

Choose a reason for hiding this comment

Uh oh!

reubenmiller left a comment

Choose a reason for hiding this comment

Uh oh!

Bravo555 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albinsuresh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

didier-wenzek commented Nov 21, 2024 •

edited

Loading

codecov bot commented Nov 21, 2024 •

edited

Loading

github-actions bot commented Nov 28, 2024 •

edited

Loading

didier-wenzek commented Mar 24, 2025 •

edited

Loading

reubenmiller commented Mar 24, 2025 •

edited by didier-wenzek

Loading