You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[C] Renewal CSR from the certificate provider; issued chains always stored - #411
The certificate provider is the only source of CSRs, and every issued chain is stored in it.
Hub renewal (az_iot_connection_client_send_csr)
Breaking: the csr parameter is removed. The client gets the CSR from the provider's get_csr() (subject: device id) and releases it once the request is built.
The provider needs get_csr, release_csr and store_issued_certificate; otherwise AZ_IOT_ERR_NOT_SUPPORTED before anything is published. A get_csr() error is returned as is.
On 200, the chain is stored with store_issued_certificate() before the callback. Breaking:az_iot_csr_event gains store_status.
A failed store keeps the live session; the next connect uses the previous credential.
Breaking: requires the same three hooks (open() returns AZ_IOT_ERR_NOT_SUPPORTED; also checked when a DPS session is started without open()). The chain is always stored; there is no callback-only path.
Breaking:az_iot_operational_cert_callback gains store_result. It fires with the chain even when the store fails; the registration then fails.
Both
A truncated, unbalanced or trailing-text response (hub 200, DPS ASSIGNED) is never stored: hub renewal fails with AZ_IOT_ERR_PROTOCOL, the DPS registration fails. A chain array must close.
Provider contract: store_issued_certificate() must be all-or-nothing.
A credentials response is matched only for a 3-digit status followed by /? with $rid as one of the properties.
The sample provider (samples/common/sample_cert_provider.c) stores all-or-nothing: unique exclusive temporary file, then rename; an empty entry is refused.
Samples (hub_renew, dps_csr_managed, dps_sas_key_issued_cert, custom_certificate_provider), the e2e CSR test and the docs are updated.
Tests: unit tests for each behavior.
Validation (Linux, gcc): all unit suites pass, also under ASan/UBSan; e2e suite compiles; clang-tidy 18.1.8 clean on changed sources and samples; clang-format 18 and repo lint scripts pass; MinGW syntax check of changed C files. The hub-renewal and DPS e2e tests were not run.
…tored
- send_csr() drops its csr parameter: the CSR comes from the provider's
get_csr() (subject: device id). The provider needs get_csr, release_csr
and store_issued_certificate, else AZ_IOT_ERR_NOT_SUPPORTED before
anything is published.
- On a 200, the chain is stored with store_issued_certificate() before
the callback; az_iot_csr_event gains store_status. A failed store keeps
the session and the previous credential.
- DPS enrollment requires the same three hooks; the chain is always
stored, and az_iot_operational_cert_callback gains store_result. A
failed store fails the registration.
- A truncated or malformed hub 200 or DPS ASSIGNED payload is never
stored; a chain array must close.
- store_issued_certificate() must be all-or-nothing.
…back docs
- A credentials response is matched only for a 3-digit status followed by
'/?' with $rid as one of the properties.
- The sample provider stores a chain all-or-nothing: unique exclusive temp
file, then rename; an empty entry is refused.
- The operational-cert callback fires after the store attempt.
Reset cached profile after rejected assignment payload
c/src/core/connection_client.c:2989
This completeness check runs after the ASSIGNED handler has already called dps_read_connection_profile(), which mutates c->connection_profile. If a trailing-text response carries (for example) mqttV5, this check fails the registration and schedules a retry, but the next valid assignment with no profile (which should default to classic) does not reset that field, so the client can connect using the profile from the rejected payload. Validate the full payload before reading/caching any assignment fields, or reset the profile at the start of each assignment attempt.
Log effective error status when CSR output is missing
c/src/core/connection_client.c:9095
When get_csr() returns AZ_IOT_OK but leaves csr_base64 null, this logs the failure as AZ_IOT_OK even though the function correctly returns AZ_IOT_ERR_INTERNAL. Log the effective returned status so provider contract violations are diagnosable.
Re the previously-missed finding at connection_client.c:2989 (review on 5fab8b6): fixed in 18cf577. With CSR enrollment, the ASSIGNED payload is checked for completeness before any assignment field (hub, device id, connection profile) is read or cached. Test: dps_a_rejected_assignment_leaves_the_profile_alone.
… the rid search
- The sample certificate provider sets its vtable version again; without
it, CSR enrollment refused the provider.
- The $rid search advances past each separator in one place.
Assert DPS lifecycle faults when registration fails
c/tests/unit/connection_client_test.c:1556
This assertion checks the HUB lifecycle, which is already IDLE before provisioning starts, so it does not prove the stated “registration fails” behavior. With retries disabled, the failed DPS store should settle the DPS lifecycle in AZ_IOT_CONN_STATE_FAULTED; assert that scope/state so a regression that leaves provisioning connected or pending is caught.
This issue also appears on line 1578 of the same file.
Align ownership documentation and architecture diagrams
c/docs/eng/certificate-management.md:325
The revised ownership decision still contradicts item 4 earlier in this same document (c/docs/eng/certificate-management.md:264-266), which says the app callback is needed when the app owns persistence and is decoupled from provider storage. Under the new required-provider model the callback only observes the provider's storage result. Update that item (and the corresponding architecture diagrams that still show a missing chain continuing with bootstrap credentials) so the documented model is consistent.
Add failure-path tests for atomic certificate replacement
c/samples/common/sample_cert_provider.c:47
This new cross-platform file-creation helper and the all-or-nothing replacement path have no automated coverage under c/tests (the only test reference to sample_cert_provider is absent). Add tests for successful replacement, an empty entry, write/close/rename failure preserving the previous file, and temporary-file cleanup; the C-library convention requires boundary/failure tests for new helpers.
…orage; tests check DPS state
- The operational-cert callback runs inside message processing; it must
not call close() or deinit().
- Design doc and diagrams: the provider always stores the chain; a missing
chain, malformed payload or failed store fails the registration.
- DPS store-failure and truncated-payload tests assert DPS ends FAULTED.
Re the previously-missed findings (review on 7b82fae), fixed in ff45a8b:
connection_client_test.c:1556/1578: the tests now assert DPS ends FAULTED.
certificate-management.md item 4 and the connection.md/connection-c.md diagrams: the callback only observes; the provider always stores; a missing chain, malformed payload or failed store fails the registration.
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
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.
The certificate provider is the only source of CSRs, and every issued chain is stored in it.
Hub renewal (
az_iot_connection_client_send_csr)csrparameter is removed. The client gets the CSR from the provider'sget_csr()(subject: device id) and releases it once the request is built.get_csr,release_csrandstore_issued_certificate; otherwiseAZ_IOT_ERR_NOT_SUPPORTEDbefore anything is published. Aget_csr()error is returned as is.200, the chain is stored withstore_issued_certificate()before the callback. Breaking:az_iot_csr_eventgainsstore_status.DPS enrollment (
dps.request_operational_certificate)open()returnsAZ_IOT_ERR_NOT_SUPPORTED; also checked when a DPS session is started withoutopen()). The chain is always stored; there is no callback-only path.az_iot_operational_cert_callbackgainsstore_result. It fires with the chain even when the store fails; the registration then fails.Both
200, DPS ASSIGNED) is never stored: hub renewal fails withAZ_IOT_ERR_PROTOCOL, the DPS registration fails. A chain array must close.store_issued_certificate()must be all-or-nothing./?with$ridas one of the properties.samples/common/sample_cert_provider.c) stores all-or-nothing: unique exclusive temporary file, then rename; an empty entry is refused.Samples (
hub_renew,dps_csr_managed,dps_sas_key_issued_cert,custom_certificate_provider), the e2e CSR test and the docs are updated.Tests: unit tests for each behavior.
Validation (Linux, gcc): all unit suites pass, also under ASan/UBSan; e2e suite compiles; clang-tidy 18.1.8 clean on changed sources and samples; clang-format 18 and repo lint scripts pass; MinGW syntax check of changed C files. The hub-renewal and DPS e2e tests were not run.