Issue #85: Stop logging TA Key Provision success with SEC_LOG_ERROR.

[riwoh]

Fixes #85

Problem

Successful SoC TA key provisioning (Widevine, PlayReady 2K/3K, Apple MFi, Apple FairPlay, Netflix) logged its progress and success messages through SEC_LOG_ERROR, so a clean provisioning run showed up as a series of ERROR: lines in the log.

Change

No public header changes. The existing SEC_LOG macro already logs through the same logger hook without the ERROR: prefix, so the non-error call sites now use it.

• src/sec_adapter_soc_provisioning.c: 13 non-error messages ("Handling <x> provisioning" and "<x> provisioning completed successfully") moved from SEC_LOG_ERROR to SEC_LOG. Genuine failure paths still use SEC_LOG_ERROR.
• src/sec_adapter_utils.c: SecUtils_MkDir logged SEC_LOG_ERROR("Warning Mkdir %s failed", ...) for a condition it deliberately continues past; that is now SEC_LOG("Warning: Mkdir %s failed", ...).

I swept every SEC_LOG_ERROR call site in src/, include/, and test/ looking for messages that were not reporting a failure; the ones above were the only matches.

Testing

make sec_api builds clean (only pre-existing warnings).

Also fixed the "AppeFairplay" typo in the success message touched by this change.

[Copilot]

🟡 Changes recommended

The touched provisioning code still has a bool/Sec_Result return-type mismatch risk and a confirmed cleanup bug in the Netflix read-failure path that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR corrects misleading logging in the adapter by introducing non-error log macros and updating SoC TA provisioning and utility logs so successful/expected paths no longer emit ERROR: messages.

Changes:

• Added SEC_LOG_INFO and SEC_LOG_WARNING macros (built on existing SEC_LOG plumbing) to support informational and warning messages.
• Converted SoC TA provisioning “handling” and “completed successfully” logs from SEC_LOG_ERROR to SEC_LOG_INFO (and fixed the “AppeFairplay” typo in the touched success message).
• Updated SecUtils_MkDir to use SEC_LOG_WARNING for mkdir failures that are intentionally non-fatal.

File summaries

File	Description
include/sec_security.h	Adds SEC_LOG_INFO / SEC_LOG_WARNING macros with INFO: / WARNING: prefixes.
src/sec_adapter_soc_provisioning.c	Switches successful provisioning progress/success logs from ERROR to INFO; fixes one success-message typo.
src/sec_adapter_utils.c	Changes non-fatal mkdir failures from ERROR to WARNING.

Review details

Suppressed comments (4)

src/sec_adapter_soc_provisioning.c:705

• On this early return, parameters (allocated above) is not freed, leaking memory on provisioning-data read failures.

        if (readNetflixData(processorHandle, &netflixProvisioningData) == false) {
            SEC_LOG_ERROR("Failed to read Netflix provisioning data");
            return SEC_RESULT_FAILURE;
        }

src/sec_adapter_soc_provisioning.c:645

• Typo in error message: "Falied" -> "Failed".

            SEC_LOG_ERROR("Falied sa_key_provision_ta call in playready 2k");

src/sec_adapter_soc_provisioning.c:661

• Typo in error message: "Falied" -> "Failed".

            SEC_LOG_ERROR("Falied sa_key_provision_ta call in playready 3k");

src/sec_adapter_utils.c:327

• This warning doesn’t include the underlying errno reason, which makes diagnosing mkdir failures difficult. Consider logging strerror(errno).

    if (mkdir(tmp, S_IRWXU) != 0 && errno != EEXIST) {
        SEC_LOG("Warning: Mkdir %s failed", tmp);
        //return SEC_RESULT_FAILURE;

• Files reviewed: 2/2 changed files
• Comments generated: 3
• Review effort level: Lite

---

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

[Copilot]

🟢 Approval recommended

The functional change is low-risk and aligns logging severity with actual failure vs. success paths, with only minor wording/log-detail nits noted.

Review details

Suppressed comments (6)

src/sec_adapter_soc_provisioning.c:645

• Typo in error log message: "Falied" -> "Failed".

            SEC_LOG_ERROR("Falied sa_key_provision_ta call in playready 2k");

src/sec_adapter_soc_provisioning.c:661

• Typo in error log message: "Falied" -> "Failed".

            SEC_LOG_ERROR("Falied sa_key_provision_ta call in playready 3k");

src/sec_adapter_soc_provisioning.c:629

• Typo in error log message: "Falied" -> "Failed".

            SEC_LOG_ERROR("Falied sa_key_provision_ta call in widevine");

src/sec_adapter_soc_provisioning.c:696

• Log messages use inconsistent naming/casing ("AppleFairplay") compared to the earlier "Apple FairPlay" handling message; keeping the same spelling/casing improves grep-ability.

            SEC_LOG_ERROR("Failed to call sa_key_provision_ta in AppleFairplay");
            return SEC_RESULT_FAILURE;
        }
        SEC_LOG("AppleFairplay provisioning completed successfully");

src/sec_adapter_utils.c:317

• This warning log drops the reason for the mkdir failure; including errno makes field debugging easier while still keeping it non-error severity.

                SEC_LOG("Warning: Mkdir %s failed", tmp);

src/sec_adapter_utils.c:326

• This warning log drops the reason for the mkdir failure; including errno makes field debugging easier while still keeping it non-error severity.

        SEC_LOG("Warning: Mkdir %s failed", tmp);

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[mhabrat]

Review and address Copilot review comments.

[Copilot]

🟢 Approval recommended

The changes are low-risk and limited to logging/message correctness, with only minor message wording/capitalization nits identified.

Review details

Suppressed comments (5)

src/sec_adapter_soc_provisioning.c:691

• Error message uses "Fairplay" capitalization; use consistent "FairPlay" product name in logs.

            SEC_LOG_ERROR("Failed to read Apple Fairplay provisioning data");

src/sec_adapter_soc_provisioning.c:699

• Error message uses "Fairplay" capitalization; use consistent "FairPlay" product name in logs.

            SEC_LOG_ERROR("Failed to call sa_key_provision_ta in Apple Fairplay");

src/sec_adapter_soc_provisioning.c:702

• Success message uses "Fairplay" capitalization; use consistent "FairPlay" product name in logs.

        SEC_LOG("Apple Fairplay provisioning completed successfully");

src/sec_adapter_soc_provisioning.c:665

• This error message is grammatically unclear (missing "to") and uses inconsistent product capitalization ("playready 3k" vs other "PlayReady 3K" logs).

            SEC_LOG_ERROR("Failed sa_key_provision_ta call in playready 3k");

src/sec_adapter_soc_provisioning.c:648

• This error message is grammatically unclear (missing "to") and uses inconsistent product capitalization ("playready 2k" vs other "PlayReady 2K" logs).

            SEC_LOG_ERROR("Failed sa_key_provision_ta call in playready 2k");

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

It introduces a couple of concrete correctness/quality issues (unsafe errno usage with unspecified argument evaluation order in logging, and an unused provisioning_ta parameter) that should be addressed before approval.

Review details

Suppressed comments (5)

Previously missed (3) — in code that hasn't changed since the last review.

src/sec_adapter_soc_provisioning.c:607

• numPaths is never used inside provisioning_ta, which will trigger unused-parameter warnings and makes the signature misleading. If it’s intentionally unused, explicitly mark it unused (or remove it and update call sites).
  src/sec_adapter_utils.c:319
• strerror(errno) is passed as an argument to SEC_LOG, but C does not specify function argument evaluation order; SEC_LOG also evaluates other arguments (e.g., syscall(SYS_gettid)) that could clobber errno, resulting in an incorrect error string. Capture errno to a local variable before logging.
  src/sec_adapter_utils.c:328
• Same errno/argument-evaluation-order issue here: capture errno before calling SEC_LOG so the logged strerror matches the failing mkdir.

src/sec_adapter_soc_provisioning.c:692

• The log message uses "Fairplay" while other messages in this file use "FairPlay"; for consistency (and to match the earlier "Handling Apple FairPlay provisioning"), this should use "FairPlay".

        if (readAppleFairPlayData(processorHandle, &appleFairplayProvisioningData) == false) {
            SEC_LOG_ERROR("Failed to read Apple Fairplay provisioning data");
            SEC_FREE(parameters);

src/sec_adapter_soc_provisioning.c:702

• These messages use "Fairplay"; elsewhere the file uses "FairPlay". Update both strings to keep log output consistent.

        if (status != SA_STATUS_OK) {
            SEC_LOG_ERROR("Failed to call sa_key_provision_ta in Apple Fairplay");
            return SEC_RESULT_FAILURE;
        }
        SEC_LOG("Apple Fairplay provisioning completed successfully");

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are low-risk and improve logging correctness (with consistent Sec_Result returns) without altering public headers.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/sec_adapter_soc_provisioning.c:610

• PR description focuses on logging-only changes, but this hunk also changes provisioning_ta from bool to Sec_Result (and updates return values), which is a functional behavior change that impacts the SecSocProv_Ta_Provision return value. Please update the PR description (or add a note in the commit/PR) to explicitly call out this functional fix so reviewers/consumers aren’t surprised.

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite