[C] Managed provider: validate stored chains; atomic owner-only writes

[Ewerton Scaboro da Silva (ewertons)]

The managed certificate provider stores only chains for its own key and writes its files atomically, readable only by the current user.

Store (store_issued_certificate)

• Refused (AZ_IOT_ERR_INVALID_ARG) unless every entry is a non-empty, parseable PEM certificate, the parsed count matches the issued count, the leaf certifies the operational key, and the PEM chain is at most 64 KiB. Each entry is sized before it is decoded.
• A refused or failed store keeps the previous chain.

Init

• A persisted chain is used only if it passes the same checks; the read is bounded to 64 KiB.

Writes (key and chain)

• Uniquely named, exclusively created file next to the destination: mkstemp + fchmod(0600) on POSIX, owner-only protected DACL (owner, SYSTEM) on Windows. Flushed, then renamed over the destination.
• A failed write leaves the destination untouched. A symlink at the destination is replaced, not followed. The mode of an existing key file is not changed.

Tests: unit tests for each behavior. Permission, mode and symlink tests are POSIX only; a Windows test checks the DACL.

Validation (Linux, gcc): all unit suites pass, the provider suite also under ASan/UBSan; clang-tidy 18.1.8 clean on the provider; clang-format 18 and repo lint scripts pass. The Windows code was syntax-checked with MinGW only.

[Copilot]

🟡 Changes recommended

Windows consumers lack the provider’s required system-library linkage, and malformed base64 returns the wrong result code.

3 open findings

• Propagate advapi32 dependency through the managed provider target · New
• Test atomic replacement failures preserve destination and remove temp files · New
• Return AZ_IOT_ERR_INVALID_ARG for malformed base64 certificates · New

What changed in this PR

Hardens managed certificate persistence through chain validation, bounded reads, and atomic owner-only file replacement.

Changes:

• Validates certificate chains against the operational key and 64 KiB limit.
• Adds secure atomic writes for POSIX and Windows.
• Expands platform-specific tests and coverage documentation.

File	Description
c/​adapters/​cert_openssl/​az_iot_certificate_provider_managed.c	Implements validation and secure atomic persistence.
c/​adapters/​cert_openssl/​az_iot_certificate_provider_managed.h	Documents validation and file protections.
c/​tests/​unit/​certificate_provider_managed_test.c	Adds validation, permissions, DACL, and symlink tests.
c/​tests/​CMakeLists.txt	Adds Windows security-library linkage for tests.
c/​docs/​eng/​test-coverage.md	Records expanded managed-provider coverage.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The managed provider’s required vtable ABI-version assertion was removed without replacement.

3 open findings

• Propagate advapi32 dependency through the managed provider target
• Retain test asserting managed provider vtable ABI version · New
• Test atomic replacement failures preserve destination and remove temp files

1 resolved since last review

• Return AZ_IOT_ERR_INVALID_ARG for malformed base64 certificates

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Strict certificate validation still accepts DER certificates with trailing decoded data, and one documented key-file guarantee is inaccurate.

4 open findings

• Propagate advapi32 dependency through the managed provider target
• Accepts trailing bytes after a valid certificate · New
• Test atomic replacement failures preserve destination and remove temp files
• Replaces unreadable keys despite preserving existing modes · New

1 resolved since last review

• Retain test asserting managed provider vtable ABI version

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Exact 64 KiB acceptance and 64 KiB-plus-one rejection remain untested.

3 open findings

• Propagate advapi32 dependency through the managed provider target
• Test atomic replacement failures preserve destination and remove temp files
• Add exact 64 KiB boundary tests for read and store paths · New

2 resolved since last review

• Accepts trailing bytes after a valid certificate
• Replaces unreadable keys despite preserving existing modes

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Security-sensitive cross-platform filesystem behavior, particularly the Windows path without native runtime validation, warrants final human review.

3 open findings

• Propagate advapi32 dependency through the managed provider target
• Test atomic replacement failures preserve destination and remove temp files
• Add exact 64 KiB boundary tests for read and store paths

🧠 Review effort: Balanced

[Ewerton Scaboro da Silva (ewertons)]

Approved