From 80b954d56d4c3438b0fad75b117b8db793bb24c8 Mon Sep 17 00:00:00 2001 From: ewertons <12401690+ewertons@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:01:19 +0000 Subject: [PATCH 1/4] [C] Software updates: size signature buffers from the request buffer - AZ_IOT_SU_REQUEST_BUFFER_SIZE defaults to 16384 (was 4096). A one-file offer is about 5 KiB, so the former default refused it. - Manifest verification decodes into a caller-supplied scratch instead of fixed stack buffers (header and sjwk 2048, sjwk payload 2048, sjwk header 512). The managed client passes its persistence scratch, so any signature that fits the request buffer decodes; the sjwk and modulus are unescaped in place instead of copied. Verifier stack: 6544 -> 624 bytes. - az_iot_su_parse_update_request() uses AZ_IOT_SU_VERIFY_SCRATCH_SIZE (8192) bytes of stack. - A part that does not fit is logged as too large, with its size, instead of as invalid base64url. - Base64 input is checked against its alphabet before decoding, avoiding a negative left shift in azure-sdk-for-c on a standard-base64 modulus. --- c/CHANGELOG.md | 7 + c/docs/client-configuration.md | 3 +- c/docs/eng/test-coverage.md | 2 + c/inc/azure/iot/az_iot_su.h | 23 +- .../software_update/esp32/main/app_main.c | 2 +- c/src/features/su/su_client.c | 395 ++++++++++++------ c/tests/unit/su_client_test.c | 123 +++++- 7 files changed, 419 insertions(+), 136 deletions(-) diff --git a/c/CHANGELOG.md b/c/CHANGELOG.md index 1761a914d..daf36eb64 100644 --- a/c/CHANGELOG.md +++ b/c/CHANGELOG.md @@ -40,3 +40,10 @@ `az_iot_connection_client_update_sas_token()` supplies a token for the request, or at any time (a connected session then renews with it). At renewal the token is requested while the session stays up; once supplied, the session disconnects and reconnects with it. See [docs/connecting.md](docs/connecting.md#authentication). +- Software updates: `AZ_IOT_SU_REQUEST_BUFFER_SIZE` defaults to 16384 (was 4096), so offers above + 4 KiB are no longer refused; the client struct grows by about 24 KiB. The manifest signature is + decoded into the client's own scratch rather than fixed stack buffers, so a signing key or JWS + header of any size that fits the request buffer verifies, and the verifier's stack drops from + about 6.5 KiB to under 1 KiB. `az_iot_su_parse_update_request()` uses + `AZ_IOT_SU_VERIFY_SCRATCH_SIZE` (8192) bytes of stack instead. A part that does not fit is + logged as too large, not as invalid. diff --git a/c/docs/client-configuration.md b/c/docs/client-configuration.md index 34d63cf62..1833b5c62 100644 --- a/c/docs/client-configuration.md +++ b/c/docs/client-configuration.md @@ -170,7 +170,8 @@ Defaults used when the matching option is left at 0: | Macro | Default | Bounds | | --- | --- | --- | | `AZ_IOT_SU_MAX_ROOT_KEYS` | 4 | Root keys in the trust store. | -| `AZ_IOT_SU_REQUEST_BUFFER_SIZE` | 4096 | Copy of the update metadata (manifest and signature) for the current deployment. A larger deployment is refused: `AZ_IOT_SU_EVENT_UPDATE_REFUSED` is raised with `AZ_IOT_ERR_NOT_ENOUGH_SPACE`. | +| `AZ_IOT_SU_REQUEST_BUFFER_SIZE` | 16384 | Copy of the update metadata (manifest, signature and file URLs) for the current deployment; a one-file offer is about 5 KiB. A larger deployment is refused: `AZ_IOT_SU_EVENT_UPDATE_REFUSED` is raised with `AZ_IOT_ERR_NOT_ENOUGH_SPACE`. Also sizes `AZ_IOT_SU_STATE_BLOB_MAX_SIZE` and the scratch the client verifies the signature in, so the client holds about twice this value. | +| `AZ_IOT_SU_VERIFY_SCRATCH_SIZE` | 8192 | Stack used by `az_iot_su_parse_update_request()` for the decoded signature (JWS header with the signing key, and the signatures). A signature that does not fit fails with `AZ_IOT_ERR_AUTH` and an `ERROR` log naming the part that is too large. | | `AZ_IOT_SU_WORKFLOW_ID_SIZE` | 64 | Workflow ID kept for reporting, duplicate detection and persistence. A deployment with a longer ID is refused: nothing is processed or reported, and `AZ_IOT_SU_EVENT_UPDATE_REFUSED` is raised with `AZ_IOT_ERR_NOT_ENOUGH_SPACE`. | | `AZ_IOT_SU_PERSIST_MAX_ATTEMPTS` | 5 | Consecutive failed state writes before the client stops retrying (1 to `0xFFFFFFFF`). | | `AZ_IOT_SU_DEVICE_PROPERTIES_BUFFER_SIZE` | 512 | Default size of the device-properties buffer type. Application-side only; `az_iot_su_device_properties_buffer_size()` gives the exact size. | diff --git a/c/docs/eng/test-coverage.md b/c/docs/eng/test-coverage.md index 3a07c92d2..5c9aa6b08 100644 --- a/c/docs/eng/test-coverage.md +++ b/c/docs/eng/test-coverage.md @@ -954,6 +954,8 @@ RS256 signature verification) through the crypto backends in `c/adapters/crypto_ | | Standalone entry points check the backend they need | `az_iot_su_parse_update_request()`: no backend is INVALID_ARG, no `verify_rs256` is NOT_SUPPORTED, and non-NULL outputs are zeroed on both. `az_iot_su_verify_file_hash()`: no backend is INVALID_ARG; a SHA-256-only backend verifies. | unit | Done | [standalone_entry_points_check_the_backend_they_need](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c) | | | Manifest signed by an unknown root key is rejected | End-to-end through `az_iot_su_parse_update_request()`. | unit | Done | [manifest_signed_by_an_unknown_root_key_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1428) | | | Malformed jws is rejected | Wrong segment count, bad base64url, missing header. | unit | Done | [malformed_jws_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1449) | +| | Large signature is verified | JWS header and signing key over 2 KiB in an offer over 4 KiB; modulus in standard base64 with escaped slashes (managed and public parser). | unit | Done | [large_signature_is_verified](../../tests/unit/su_client_test.c) | +| | Oversized signature is reported as too large | A header that does not fit `AZ_IOT_SU_VERIFY_SCRATCH_SIZE` is logged as too large; a malformed one as not valid. | unit | Done | [oversized_signature_is_reported_as_too_large](../../tests/unit/su_client_test.c) | | | Malformed manifest JSON is rejected | — | unit | Done | [malformed_manifest_json_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1481) | | | Verify file hash rejects an unsupported algorithm | — | unit | Done | [verify_file_hash_rejects_an_unsupported_algorithm](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1534) | | End-to-end | Real update is downloaded, verified, installed and reported | A real offer through the whole client: real roots, real download and hash, SUCCEEDED acknowledged. Passes live; not yet in CI. | e2e | Pending | [real_update_is_downloaded_verified_installed_and_reported](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/e2e/tests/e2e_su_offer_test.c) | diff --git a/c/inc/azure/iot/az_iot_su.h b/c/inc/azure/iot/az_iot_su.h index b32928c35..ebc441c74 100644 --- a/c/inc/azure/iot/az_iot_su.h +++ b/c/inc/azure/iot/az_iot_su.h @@ -85,11 +85,23 @@ extern "C" * valid during the subscriber callback, but the workflow is processed * asynchronously across many do_work() calls; the upstream parser stores spans * that point INTO this payload (and unescapes the manifest in place), so it must - * outlive the callback. Sized for a v5 manifest with the upstream's bounded - * step/file counts plus the JWS signature. Override if your deployments are - * larger. No heap is used. */ + * outlive the callback. Holds the escaped manifest, its JWS signature and the + * file URLs; a one-file offer is about 5 KiB. Override if your deployments are + * larger. It also bounds the scratch the client verifies the signature in (see + * AZ_IOT_SU_PERSIST_BLOB_SIZE). No heap is used. */ #ifndef AZ_IOT_SU_REQUEST_BUFFER_SIZE -#define AZ_IOT_SU_REQUEST_BUFFER_SIZE 4096 +#define AZ_IOT_SU_REQUEST_BUFFER_SIZE 16384 +#endif + +/** + * @brief Stack scratch az_iot_su_parse_update_request() decodes the manifest signature into. + * + * Holds the decoded JWS header (which embeds the signing key), the signing key and the + * signatures. A signature that does not fit is rejected with AZ_IOT_ERR_AUTH and an error + * log naming the part that is too large. The managed client does not use it. + */ +#ifndef AZ_IOT_SU_VERIFY_SCRATCH_SIZE +#define AZ_IOT_SU_VERIFY_SCRATCH_SIZE 8192 #endif /** @brief Capacity of the packed applied update id (provider, name, version). */ @@ -1141,7 +1153,8 @@ extern "C" * * Verifies the manifest trust chain (JWS/SJWK, root-key `kid`, RS256, both * RSA checks, SHA-256 binding), then parses the manifest. Fail-closed: - * non-NULL outputs are zeroed on every error. + * non-NULL outputs are zeroed on every error. Uses + * AZ_IOT_SU_VERIFY_SCRATCH_SIZE bytes of stack for the decoded signature. * * @param request_json `{ workflowId, updateManifest, updateManifestSignature, * fileUrls }` as the service sends it. The diff --git a/c/samples/software_update/esp32/main/app_main.c b/c/samples/software_update/esp32/main/app_main.c index 48ab2c9ce..841866f22 100644 --- a/c/samples/software_update/esp32/main/app_main.c +++ b/c/samples/software_update/esp32/main/app_main.c @@ -295,7 +295,7 @@ void app_main(void) dp.installed_update_id.version = SU_UPDATE_VERSION; static AZ_IOT_SU_DEVICE_PROPERTIES_STORAGE(dp_buffer); - /* ~14 KB: too large for the 12 KB main task stack. */ + /* ~38 KB: too large for the 12 KB main task stack. */ static az_iot_su_client su; az_iot_su_client_config_options su_opts = az_iot_su_client_config_options_default(); su_opts.hooks = &hooks; diff --git a/c/src/features/su/su_client.c b/c/src/features/su/su_client.c index 4ebdec882..6dce43813 100644 --- a/c/src/features/su/su_client.c +++ b/c/src/features/su/su_client.c @@ -240,10 +240,13 @@ static void result_overall_success(az_iot_su_client* client) * backend only for the two RSA signature checks and the binding SHA-256 (see * design doc section 6). The backend remains pure primitives. * - * Stack note: the chain decodes several base64url segments into local scratch - * buffers (the largest is the manifest protected header, which embeds the whole - * SJWK). The frame is a few KiB; acceptable for the Linux/desktop OpenSSL target. - * Constrained backends should mind the per-task stack budget. + * Buffers: the chain decodes its base64url segments into a caller-supplied + * scratch, not fixed stack arrays, because signing keys and their headers grow + * as keys rotate. The managed client passes its persistence scratch, which holds + * more than AZ_IOT_SU_REQUEST_BUFFER_SIZE, so any signature that fits the + * request buffer decodes. az_iot_su_parse_update_request() uses + * AZ_IOT_SU_VERIFY_SCRATCH_SIZE bytes of stack. Values that do not fit are + * logged as too large, not as invalid. */ /* Split a compact JWS "header.payload.signature" (exactly two '.'). On success @@ -289,55 +292,96 @@ static bool jws_split( return true; } -/* base64url-decode `src` into `buf`; returns the decoded span, or AZ_SPAN_EMPTY - * on any error (including insufficient capacity). */ -static az_span jws_b64url(az_span src, uint8_t* buf, int32_t cap) +/** @brief Bump allocator over the scratch manifest verification decodes into. */ +typedef struct { - int32_t written = 0; - if (az_span_size(src) <= 0 - || az_result_failed(az_base64_url_decode(az_span_create(buf, cap), src, &written)) - || written <= 0) - { - return AZ_SPAN_EMPTY; - } - return az_span_create(buf, written); -} + uint8_t* buf; + int32_t cap; + int32_t used; + /** Bytes the last scratch_b64_decode() needed; logged when they did not fit. */ + int32_t need; +} su_scratch; -/* Decode a JWK field that may be encoded as either base64url (RFC 7515) or - * standard base64. The software updates service emits the signing-key modulus (n) as - * standard base64 (with '+'/'/'), while the surrounding JWS segments are - * base64url; az_base64_url_decode rejects '+'/'/' outright, so try base64url - * first and fall back to standard base64. */ -static az_span jws_b64_any(az_span src, uint8_t* buf, int32_t cap) +/** + * @brief Decode base64url @p src into the free part of @p s and claim the bytes written. + * + * @param s Scratch. + * @param src Encoded text. + * @param allow_std Also accept standard (padded) base64: the service encodes the signing-key + * modulus that way. + * @param out The decoded bytes, in @p s. + * @return AZ_IOT_OK; AZ_IOT_ERR_NOT_ENOUGH_SPACE if they do not fit; AZ_IOT_ERR_INVALID_ARG if + * @p src is empty or not valid. + */ +static az_iot_result scratch_b64_decode(su_scratch* s, az_span src, bool allow_std, az_span* out) { + int32_t len = az_span_size(src); + /* Exact for unpadded base64url; an upper bound for padded base64. */ + s->need = (len / 4) * 3 + ((len % 4) * 3) / 4; + if (s->need <= 0) + { + return AZ_IOT_ERR_INVALID_ARG; + } + if (s->need > s->cap - s->used) + { + return AZ_IOT_ERR_NOT_ENOUGH_SPACE; + } + az_span dst = az_span_create(s->buf + s->used, s->cap - s->used); int32_t written = 0; - if (az_span_size(src) <= 0) + /* Check the alphabet first: azure-sdk-for-c shifts a negative value on a + * character outside it. */ + bool std = false; + const uint8_t* p = az_span_ptr(src); + for (int32_t i = 0; i < len; ++i) { - return AZ_SPAN_EMPTY; + uint8_t c = p[i]; + bool common = (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9'); + if (common || c == '-' || c == '_') + { + continue; + } + /* Standard base64: '+', '/', and up to two trailing '='. */ + if (!allow_std || (c != '+' && c != '/' && !(c == '=' && i >= len - 2 && p[len - 1] == '='))) + { + return AZ_IOT_ERR_INVALID_ARG; + } + std = true; } - if (az_result_succeeded(az_base64_url_decode(az_span_create(buf, cap), src, &written)) - && written > 0) + if (std) { - return az_span_create(buf, written); + for (int32_t i = 0; i < len; ++i) + { + if (p[i] == '-' || p[i] == '_') + { + return AZ_IOT_ERR_INVALID_ARG; + } + } } - if (az_result_succeeded(az_base64_decode(az_span_create(buf, cap), src, &written)) && written > 0) + az_result r = std + ? (len % 4 == 0 ? az_base64_decode(dst, src, &written) : AZ_ERROR_UNEXPECTED_END) + : az_base64_url_decode(dst, src, &written); + if (az_result_failed(r) || written <= 0) { - return az_span_create(buf, written); + return AZ_IOT_ERR_INVALID_ARG; } - return AZ_SPAN_EMPTY; + *out = az_span_create(s->buf + s->used, written); + s->used += written; + return AZ_IOT_OK; } -/* Read a top-level string property `name` from JSON object `obj`, copying its - * (unescaped) value into `out_buf`. Returns the value span, or AZ_SPAN_EMPTY if - * the property is absent or not a string. */ -static az_span jws_json_str(az_span obj, az_span name, char* out_buf, int32_t cap) +/** + * @brief Find top-level property @p name of JSON object @p obj. + * + * @return true with @p out on its value token; false if absent or @p obj is not an object. + */ +static bool jws_json_find(az_span obj, az_span name, az_json_token* out) { az_json_reader jr; if (az_result_failed(az_json_reader_init(&jr, obj, NULL)) || az_result_failed(az_json_reader_next_token(&jr)) || jr.token.kind != AZ_JSON_TOKEN_BEGIN_OBJECT) { - return AZ_SPAN_EMPTY; + return false; } while (az_result_succeeded(az_json_reader_next_token(&jr)) && jr.token.kind != AZ_JSON_TOKEN_END_OBJECT) @@ -349,31 +393,88 @@ static az_span jws_json_str(az_span obj, az_span name, char* out_buf, int32_t ca bool match = az_json_token_is_text_equal(&jr.token, name); if (az_result_failed(az_json_reader_next_token(&jr))) { - return AZ_SPAN_EMPTY; + return false; } if (match) { - if (jr.token.kind != AZ_JSON_TOKEN_STRING) - { - return AZ_SPAN_EMPTY; - } - int32_t len = 0; - if (az_result_failed(az_json_token_get_string(&jr.token, out_buf, cap, &len))) - { - return AZ_SPAN_EMPTY; - } - return az_span_create((uint8_t*)out_buf, len); + *out = jr.token; + return true; } /* Skip a non-matching object/array value so we stay at object scope. */ if (jr.token.kind == AZ_JSON_TOKEN_BEGIN_OBJECT || jr.token.kind == AZ_JSON_TOKEN_BEGIN_ARRAY) { if (az_result_failed(az_json_reader_skip_children(&jr))) { - return AZ_SPAN_EMPTY; + return false; } } } - return AZ_SPAN_EMPTY; + return false; +} + +/** + * @brief Copy the unescaped value of top-level string property @p name of @p obj to @p out_buf. + * + * @return AZ_IOT_OK with @p out on it; AZ_IOT_ERR_NOT_FOUND if absent, empty or not a string; + * AZ_IOT_ERR_NOT_ENOUGH_SPACE if it does not fit @p cap with a terminator; + * AZ_IOT_ERR_INVALID_ARG if it cannot be unescaped. + */ +static az_iot_result jws_json_str( + az_span obj, + az_span name, + char* out_buf, + int32_t cap, + az_span* out) +{ + az_json_token t; + if (!jws_json_find(obj, name, &t) || t.kind != AZ_JSON_TOKEN_STRING) + { + return AZ_IOT_ERR_NOT_FOUND; + } + int32_t len = 0; + az_result r = az_json_token_get_string(&t, out_buf, cap, &len); + if (r == AZ_ERROR_NOT_ENOUGH_SPACE) + { + return AZ_IOT_ERR_NOT_ENOUGH_SPACE; + } + if (az_result_failed(r)) + { + return AZ_IOT_ERR_INVALID_ARG; + } + if (len <= 0) + { + return AZ_IOT_ERR_NOT_FOUND; + } + *out = az_span_create((uint8_t*)out_buf, len); + return AZ_IOT_OK; +} + +/** + * @brief Unescape the value of top-level string property @p name in place in @p obj. + * + * Avoids copying a value nearly as large as @p obj. @p obj must be writable; if the value had + * escapes it is no longer valid JSON afterwards, so read other properties first. + * + * @return As jws_json_str(), except AZ_IOT_ERR_NOT_ENOUGH_SPACE. + */ +static az_iot_result jws_json_str_in_place(az_span obj, az_span name, az_span* out) +{ + az_json_token t; + if (!jws_json_find(obj, name, &t) || t.kind != AZ_JSON_TOKEN_STRING) + { + return AZ_IOT_ERR_NOT_FOUND; + } + az_span value; + if (az_iot_json_string_decode(t.slice, t.slice, &value) != AZ_IOT_OK) + { + return AZ_IOT_ERR_INVALID_ARG; + } + if (az_span_size(value) <= 0) + { + return AZ_IOT_ERR_NOT_FOUND; + } + *out = value; + return AZ_IOT_OK; } static const az_span k_alg_rs256 = AZ_SPAN_LITERAL_FROM_STR("RS256"); @@ -426,12 +527,63 @@ static const az_iot_su_root_key* resolve_root_key( return AZ_IOT_SU_RESULT_FAILURE; \ } while (0) +/** + * @brief Log why a scratch_b64_decode() of @p what failed. + * + * @param s The scratch it decoded into. + * @param r Its result, not AZ_IOT_OK. + * @param what The decoded part, for the log. + */ +static void log_decode_failure(const su_scratch* s, az_iot_result r, const char* what) +{ + if (r == AZ_IOT_ERR_NOT_ENOUGH_SPACE) + { + AZ_IOT_LOG_ERRORF( + AZ_IOT_LOG_COMPONENT_SU, + "manifest verification failed: %s too large (%ld bytes, %ld of %ld free)", + what, + (long)s->need, + (long)(s->cap - s->used), + (long)s->cap); + } + else + { + AZ_IOT_LOG_ERRORF(AZ_IOT_LOG_COMPONENT_SU, "manifest verification failed: %s not valid", what); + } +} + +/* Fail verification unless scratch_b64_decode() result `r` is AZ_IOT_OK. */ +#define SU_VERIFY_DECODED(r, s, what) \ + do \ + { \ + az_iot_result r_ = (r); \ + if (r_ != AZ_IOT_OK) \ + { \ + log_decode_failure((s), r_, (what)); \ + return AZ_IOT_SU_RESULT_FAILURE; \ + } \ + } while (0) + +/** + * @brief Verify the manifest signature chain (design doc section 6). + * + * @param crypto Crypto backend; needs verify_rs256. + * @param root_keys Trusted root keys. + * @param root_key_count Entries in @p root_keys. + * @param manifest The unescaped manifest body. + * @param jws The compact updateManifestSignature. + * @param scratch Decoded segments are written here; not read before. + * @param scratch_size Size of @p scratch. + * @return AZ_IOT_SU_RESULT_SUCCESS; AZ_IOT_SU_RESULT_FAILURE, with the reason logged. + */ static int32_t verify_manifest_core( const az_iot_crypto* crypto, const az_iot_su_root_key* root_keys, size_t root_key_count, az_span manifest, - az_span jws) + az_span jws, + uint8_t* scratch, + size_t scratch_size) { if (az_iot_crypto__validate(crypto) != AZ_IOT_OK || crypto->verify_rs256 == NULL) { @@ -449,32 +601,32 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("empty updateManifest or updateManifestSignature"); } - /* 1. Split the manifest JWS and decode + parse its protected header. */ + su_scratch s = { scratch, (int32_t)(scratch_size > INT32_MAX ? INT32_MAX : scratch_size), 0, 0 }; + + /* 1. Split the manifest JWS and decode + parse its protected header. The + * header, and the sjwk unescaped within it, stay in scratch through step 4. */ az_span m_hdr_b64, m_pl_b64, m_sig_b64, m_signed; if (!jws_split(jws, &m_hdr_b64, &m_pl_b64, &m_sig_b64, &m_signed)) { SU_VERIFY_FAIL("step 1: manifest JWS is not a valid 3-part token"); } - uint8_t sjwk_buf[2048]; /* outlives steps 2-6: SJWK segments point into it */ az_span sjwk; { - uint8_t hdr_buf[2048]; - az_span hdr = jws_b64url(m_hdr_b64, hdr_buf, (int32_t)sizeof(hdr_buf)); - if (az_span_size(hdr) <= 0) - { - SU_VERIFY_FAIL("step 1: manifest JWS header is not valid base64url"); - } + az_span hdr; + SU_VERIFY_DECODED( + scratch_b64_decode(&s, m_hdr_b64, false, &hdr), &s, "step 1: manifest JWS header"); char alg_buf[16]; - az_span alg = jws_json_str(hdr, AZ_SPAN_FROM_STR("alg"), alg_buf, (int32_t)sizeof(alg_buf)); - if (!az_span_is_content_equal(alg, k_alg_rs256)) + az_span alg = AZ_SPAN_EMPTY; + if (jws_json_str(hdr, AZ_SPAN_FROM_STR("alg"), alg_buf, (int32_t)sizeof(alg_buf), &alg) + != AZ_IOT_OK + || !az_span_is_content_equal(alg, k_alg_rs256)) { SU_VERIFY_FAIL("step 1: manifest JWS alg is not RS256"); } - sjwk = jws_json_str(hdr, AZ_SPAN_FROM_STR("sjwk"), (char*)sjwk_buf, (int32_t)sizeof(sjwk_buf)); - if (az_span_size(sjwk) <= 0) + if (jws_json_str_in_place(hdr, AZ_SPAN_FROM_STR("sjwk"), &sjwk) != AZ_IOT_OK) { SU_VERIFY_FAIL("step 1: manifest JWS header has no sjwk"); } @@ -487,25 +639,31 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 2: sjwk is not a valid 3-part token"); } + /* Steps 2 and 3 release their scratch; step 4 keeps the signing key. */ + int32_t after_hdr = s.used; const az_iot_su_root_key* root = NULL; { - uint8_t shdr_buf[512]; - az_span shdr = jws_b64url(s_hdr_b64, shdr_buf, (int32_t)sizeof(shdr_buf)); - if (az_span_size(shdr) <= 0) - { - SU_VERIFY_FAIL("step 2: sjwk header is not valid base64url"); - } + az_span shdr; + SU_VERIFY_DECODED(scratch_b64_decode(&s, s_hdr_b64, false, &shdr), &s, "step 2: sjwk header"); char salg_buf[16]; - az_span salg = jws_json_str(shdr, AZ_SPAN_FROM_STR("alg"), salg_buf, (int32_t)sizeof(salg_buf)); - if (!az_span_is_content_equal(salg, k_alg_rs256)) + az_span salg = AZ_SPAN_EMPTY; + if (jws_json_str(shdr, AZ_SPAN_FROM_STR("alg"), salg_buf, (int32_t)sizeof(salg_buf), &salg) + != AZ_IOT_OK + || !az_span_is_content_equal(salg, k_alg_rs256)) { SU_VERIFY_FAIL("step 2: sjwk alg is not RS256"); } char kid_buf[128]; - az_span kid = jws_json_str(shdr, AZ_SPAN_FROM_STR("kid"), kid_buf, (int32_t)sizeof(kid_buf)); - if (az_span_size(kid) <= 0) + az_span kid = AZ_SPAN_EMPTY; + az_iot_result r + = jws_json_str(shdr, AZ_SPAN_FROM_STR("kid"), kid_buf, (int32_t)sizeof(kid_buf), &kid); + if (r == AZ_IOT_ERR_NOT_ENOUGH_SPACE) + { + SU_VERIFY_FAIL("step 2: sjwk kid too large (127 bytes max)"); + } + if (r != AZ_IOT_OK) { SU_VERIFY_FAIL("step 2: sjwk header has no kid"); } @@ -516,15 +674,13 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 2: sjwk kid does not match any known (enabled) root key"); } } + s.used = after_hdr; /* 3. Verify the SJWK signature with the resolved root key. */ { - uint8_t s_sig_buf[1024]; - az_span s_sig = jws_b64url(s_sig_b64, s_sig_buf, (int32_t)sizeof(s_sig_buf)); - if (az_span_size(s_sig) <= 0) - { - SU_VERIFY_FAIL("step 3: sjwk signature is not valid base64url"); - } + az_span s_sig; + SU_VERIFY_DECODED( + scratch_b64_decode(&s, s_sig_b64, false, &s_sig), &s, "step 3: sjwk signature"); if (crypto->verify_rs256( crypto, root->modulus, @@ -540,47 +696,42 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 3: sjwk signature does not verify against the root key"); } } + s.used = after_hdr; - /* 4. Parse the now-trusted SJWK payload as a JWK → signing key (n, e). - * Buffers are sized for up to 4096-bit RSA keys: the base64 modulus of a - * 3072-bit key is already 512 chars, and az_json_token_get_string needs the - * destination strictly larger than the string to fit its NUL terminator. */ - uint8_t n_buf[1024]; - uint8_t e_buf[16]; + /* 4. Parse the now-trusted SJWK payload as a JWK → signing key (n, e). The + * modulus may be standard base64; the payload is unescaped in place, after + * `e` is read. */ az_span n_raw; az_span e_raw; { - uint8_t spl_buf[2048]; - az_span spl = jws_b64url(s_pl_b64, spl_buf, (int32_t)sizeof(spl_buf)); - if (az_span_size(spl) <= 0) - { - SU_VERIFY_FAIL("step 4: sjwk payload is not valid base64url"); - } + az_span spl; + SU_VERIFY_DECODED(scratch_b64_decode(&s, s_pl_b64, false, &spl), &s, "step 4: sjwk payload"); - char n_b64[1024]; char e_b64[64]; - az_span n_field = jws_json_str(spl, AZ_SPAN_FROM_STR("n"), n_b64, (int32_t)sizeof(n_b64)); - az_span e_field = jws_json_str(spl, AZ_SPAN_FROM_STR("e"), e_b64, (int32_t)sizeof(e_b64)); - if (az_span_size(n_field) <= 0 || az_span_size(e_field) <= 0) + az_span e_field = AZ_SPAN_EMPTY; + az_span n_field = AZ_SPAN_EMPTY; + az_iot_result r + = jws_json_str(spl, AZ_SPAN_FROM_STR("e"), e_b64, (int32_t)sizeof(e_b64), &e_field); + if (r == AZ_IOT_ERR_NOT_ENOUGH_SPACE) { - SU_VERIFY_FAIL("step 4: signing JWK is missing modulus (n) or exponent (e)"); + SU_VERIFY_FAIL("step 4: signing JWK exponent (e) too large (63 bytes max)"); } - n_raw = jws_b64_any(n_field, n_buf, (int32_t)sizeof(n_buf)); - e_raw = jws_b64_any(e_field, e_buf, (int32_t)sizeof(e_buf)); - if (az_span_size(n_raw) <= 0 || az_span_size(e_raw) <= 0) + if (r != AZ_IOT_OK || jws_json_str_in_place(spl, AZ_SPAN_FROM_STR("n"), &n_field) != AZ_IOT_OK) { - SU_VERIFY_FAIL("step 4: signing JWK n/e are not valid base64url"); + SU_VERIFY_FAIL("step 4: signing JWK is missing modulus (n) or exponent (e)"); } + SU_VERIFY_DECODED( + scratch_b64_decode(&s, n_field, true, &n_raw), &s, "step 4: signing JWK modulus (n)"); + SU_VERIFY_DECODED( + scratch_b64_decode(&s, e_field, true, &e_raw), &s, "step 4: signing JWK exponent (e)"); } /* 5. Verify the manifest JWS signature with the trusted signing key. */ + int32_t after_key = s.used; { - uint8_t m_sig_buf[1024]; - az_span m_sig = jws_b64url(m_sig_b64, m_sig_buf, (int32_t)sizeof(m_sig_buf)); - if (az_span_size(m_sig) <= 0) - { - SU_VERIFY_FAIL("step 5: manifest signature is not valid base64url"); - } + az_span m_sig; + SU_VERIFY_DECODED( + scratch_b64_decode(&s, m_sig_b64, false, &m_sig), &s, "step 5: manifest signature"); if (crypto->verify_rs256( crypto, az_span_ptr(n_raw), @@ -596,28 +747,28 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 5: manifest signature does not verify against the signing key"); } } + s.used = after_key; /* 6. Bind the signed manifest to THIS deployment: the manifest JWS payload * carries SHA-256(manifest body); recompute and compare. */ { - uint8_t pl_buf[256]; - az_span pl = jws_b64url(m_pl_b64, pl_buf, (int32_t)sizeof(pl_buf)); - if (az_span_size(pl) <= 0) - { - SU_VERIFY_FAIL("step 6: manifest JWS payload is not valid base64url"); - } + az_span pl; + SU_VERIFY_DECODED( + scratch_b64_decode(&s, m_pl_b64, false, &pl), &s, "step 6: manifest JWS payload"); char hash_b64[128]; - az_span hash_field - = jws_json_str(pl, AZ_SPAN_FROM_STR("sha256"), hash_b64, (int32_t)sizeof(hash_b64)); - if (az_span_size(hash_field) <= 0) + az_span hash_field = AZ_SPAN_EMPTY; + az_iot_result r = jws_json_str( + pl, AZ_SPAN_FROM_STR("sha256"), hash_b64, (int32_t)sizeof(hash_b64), &hash_field); + if (r == AZ_IOT_ERR_NOT_FOUND) { SU_VERIFY_FAIL("step 6: manifest JWS payload has no sha256"); } uint8_t expected[32]; int32_t exp_written = 0; - if (az_result_failed(az_base64_decode( + if (r != AZ_IOT_OK + || az_result_failed(az_base64_decode( az_span_create(expected, (int32_t)sizeof(expected)), hash_field, &exp_written)) || exp_written != 32) { @@ -640,7 +791,8 @@ static int32_t verify_manifest_core( } /* Thin wrapper: verify the current deployment's manifest using the client's - * crypto backend + root-key store. */ + * crypto backend + root-key store. The persistence scratch is free here: it + * only holds data within one persist or resume call. */ static int32_t verify_manifest(az_iot_su_client* client) { return verify_manifest_core( @@ -648,7 +800,9 @@ static int32_t verify_manifest(az_iot_su_client* client) SU_I(client).root_keys, SU_I(client).root_key_count, SU_I(client).manifest_text, - SU_I(client).current_request.update_manifest_signature); + SU_I(client).current_request.update_manifest_signature, + SU_I(client).persist_scratch, + sizeof(SU_I(client).persist_scratch)); } /* Verify a downloaded file's SHA-256 against the signed manifest by streaming @@ -3435,8 +3589,15 @@ AZ_NODISCARD az_iot_result az_iot_su_parse_update_request( req.update_manifest = manifest_text; /* Trust gate before the manifest is parsed. */ + uint8_t scratch[AZ_IOT_SU_VERIFY_SCRATCH_SIZE]; if (verify_manifest_core( - crypto, root_keys, root_key_count, manifest_text, req.update_manifest_signature) + crypto, + root_keys, + root_key_count, + manifest_text, + req.update_manifest_signature, + scratch, + sizeof(scratch)) != AZ_IOT_SU_RESULT_SUCCESS) { return AZ_IOT_ERR_AUTH; diff --git a/c/tests/unit/su_client_test.c b/c/tests/unit/su_client_test.c index c79707893..a393fdd04 100644 --- a/c/tests/unit/su_client_test.c +++ b/c/tests/unit/su_client_test.c @@ -110,12 +110,31 @@ static void b64url_str(const void* data, int32_t len, char* dst, int32_t cap) dst[out] = '\0'; } +/* Write `"name":"aaa...",` with `len` filler bytes into `dst`, or "" when `len` is 0. */ +static void pad_property(char* dst, size_t cap, const char* name, size_t len) +{ + dst[0] = '\0'; + if (len == 0) + { + return; + } + int n = snprintf(dst, cap, "\"%s\":\"", name); + assert_true(n > 0 && (size_t)n + len + 3 < cap); + memset(dst + n, 'a', len); + memcpy(dst + (size_t)n + len, "\",", 3); +} + /* Build a structurally-valid manifest JWS: header carries alg=RS256 + an SJWK * (itself a JWS over a JWK signing key, signed by the root key `testkid`); the * payload carries SHA-256(manifest) so the binding check passes. Signature bytes - * are arbitrary because the mock backend does not validate them. */ -static void build_jws(char* out, int32_t out_cap) + * are arbitrary because the mock backend does not validate them. + * `jwk_pad` and `hdr_pad` add that many bytes of an unknown property to the + * signing JWK and to the manifest JWS header. `escaped_n` encodes the modulus as + * standard base64 with JSON-escaped slashes, as some serializers emit it. */ +static void build_jws_ex(char* out, int32_t out_cap, size_t jwk_pad, size_t hdr_pad, bool escaped_n) { + static char pad[16384]; + uint8_t fixed_hash[32]; memset(fixed_hash, SU_TEST_HASH_BYTE, sizeof(fixed_hash)); char hash_b64[64]; @@ -135,31 +154,44 @@ static void build_jws(char* out, int32_t out_cap) char shdr_b64[128]; b64url_str(sjwk_hdr, (int32_t)strlen(sjwk_hdr), shdr_b64, (int32_t)sizeof(shdr_b64)); - char n_b64[16]; + char n_b64[32]; char e_b64[16]; b64url_str(k_root_mod, (int32_t)sizeof(k_root_mod), n_b64, (int32_t)sizeof(n_b64)); b64url_str(k_root_exp, (int32_t)sizeof(k_root_exp), e_b64, (int32_t)sizeof(e_b64)); - char sp_json[128]; - snprintf(sp_json, sizeof(sp_json), "{\"kty\":\"RSA\",\"n\":\"%s\",\"e\":\"%s\"}", n_b64, e_b64); - char sp_b64[256]; + if (escaped_n) + { + /* Standard base64 of FF FF FF is "////". */ + snprintf(n_b64, sizeof(n_b64), "\\/\\/\\/\\/"); + } + pad_property(pad, sizeof(pad), "x5t", jwk_pad); + static char sp_json[16384]; + int n = snprintf( + sp_json, sizeof(sp_json), "{\"kty\":\"RSA\",%s\"n\":\"%s\",\"e\":\"%s\"}", pad, n_b64, e_b64); + assert_true(n > 0 && (size_t)n < sizeof(sp_json)); + static char sp_b64[24576]; b64url_str(sp_json, (int32_t)strlen(sp_json), sp_b64, (int32_t)sizeof(sp_b64)); const uint8_t dummy_sig[] = { 0xDE, 0xAD, 0xBE, 0xEF }; char sig_b64[16]; b64url_str(dummy_sig, (int32_t)sizeof(dummy_sig), sig_b64, (int32_t)sizeof(sig_b64)); - char sjwk[800]; - snprintf(sjwk, sizeof(sjwk), "%s.%s.%s", shdr_b64, sp_b64, sig_b64); + static char sjwk[24576]; + n = snprintf(sjwk, sizeof(sjwk), "%s.%s.%s", shdr_b64, sp_b64, sig_b64); + assert_true(n > 0 && (size_t)n < sizeof(sjwk)); - char mhdr_json[1024]; - snprintf(mhdr_json, sizeof(mhdr_json), "{\"alg\":\"RS256\",\"sjwk\":\"%s\"}", sjwk); - char mhdr_b64[1536]; + pad_property(pad, sizeof(pad), "x5c", hdr_pad); + static char mhdr_json[49152]; + n = snprintf(mhdr_json, sizeof(mhdr_json), "{\"alg\":\"RS256\",%s\"sjwk\":\"%s\"}", pad, sjwk); + assert_true(n > 0 && (size_t)n < sizeof(mhdr_json)); + static char mhdr_b64[65536]; b64url_str(mhdr_json, (int32_t)strlen(mhdr_json), mhdr_b64, (int32_t)sizeof(mhdr_b64)); - int n = snprintf(out, (size_t)out_cap, "%s.%s.%s", mhdr_b64, pl_b64, sig_b64); + n = snprintf(out, (size_t)out_cap, "%s.%s.%s", mhdr_b64, pl_b64, sig_b64); assert_true(n > 0 && n < out_cap); } +static void build_jws(char* out, int32_t out_cap) { build_jws_ex(out, out_cap, 0, 0, false); } + /* Build a single-step payload with a caller-chosen workflow `id` and manifest * `version`. Returns a static buffer, valid until the next call. */ static const char* build_patch_ex(const char* id, const char* version) @@ -4614,6 +4646,71 @@ static void malformed_jws_is_rejected(void** state) } } +/* A signing key and JWS header larger than 2 KiB, as the service sends after a + * key rotation, in an offer larger than 4 KiB. The modulus is standard base64 + * with JSON-escaped slashes. */ +static const char* large_signature_patch(void) +{ + static char jws[AZ_IOT_SU_REQUEST_BUFFER_SIZE]; + static char patch[AZ_IOT_SU_REQUEST_BUFFER_SIZE]; + build_jws_ex(jws, (int32_t)sizeof(jws), 2400, 600, true); + int n = snprintf(patch, sizeof(patch), k_patch_fmt, "large-signature", "1.1", jws); + assert_true(n > 4096 && (size_t)n < sizeof(patch)); + return patch; +} + +static void large_signature_is_verified(void** state) +{ + fixture* fx = (fixture*)*state; + open_to_connected(fx); + + inject_patch(fx, large_signature_patch()); + pump(fx, 40); + assert_int_equal(az_iot_su_client_get_state(&fx->su), AZ_IOT_SU_STATE_IDLE); + assert_int_equal(fx->chan.last_report.outcome, AZ_IOT_SU_OUTCOME_SUCCEEDED); + + az_iot_su_client_update_request req; + az_iot_su_client_update_manifest manifest; + assert_int_equal( + parse_with_roots(&fx->log, large_signature_patch(), k_root_keys, 1, &req, &manifest), + AZ_IOT_OK); +} + +/* A signature part that does not fit the scratch is logged as too large; a + * malformed one as not valid. */ +static void oversized_signature_is_reported_as_too_large(void** state) +{ + fixture* fx = (fixture*)*state; + static char jws[AZ_IOT_SU_REQUEST_BUFFER_SIZE]; + static char patch[AZ_IOT_SU_REQUEST_BUFFER_SIZE]; + az_iot_su_client_update_request req; + az_iot_su_client_update_manifest manifest; + + build_jws_ex(jws, (int32_t)sizeof(jws), 0, AZ_IOT_SU_VERIFY_SCRATCH_SIZE, false); + int n = snprintf(patch, sizeof(patch), k_patch_fmt, "too-large", "1.1", jws); + assert_true(n > 0 && (size_t)n < sizeof(patch)); + su_error_log_capture too_large = { .needle = "manifest JWS header too large", .count = 0 }; + az_iot_log_sink sink + = { .sink = su_error_log_sink, .user_ctx = &too_large, .min_level = AZ_IOT_LOG_LEVEL_ERROR }; + az_iot_log_set_global_sink(&sink); + az_iot_result r = parse_with_roots(&fx->log, patch, k_root_keys, 1, &req, &manifest); + az_iot_log_set_global_sink(NULL); + assert_int_equal(r, AZ_IOT_ERR_AUTH); + assert_int_equal(too_large.count, 1); + + build_jws(jws, (int32_t)sizeof(jws)); + jws[0] = '!'; + n = snprintf(patch, sizeof(patch), k_patch_fmt, "not-valid", "1.1", jws); + assert_true(n > 0 && (size_t)n < sizeof(patch)); + su_error_log_capture invalid = { .needle = "manifest JWS header not valid", .count = 0 }; + sink.user_ctx = &invalid; + az_iot_log_set_global_sink(&sink); + r = parse_with_roots(&fx->log, patch, k_root_keys, 1, &req, &manifest); + az_iot_log_set_global_sink(NULL); + assert_int_equal(r, AZ_IOT_ERR_AUTH); + assert_int_equal(invalid.count, 1); +} + static void malformed_manifest_json_is_rejected(void** state) { fixture* fx = (fixture*)*state; @@ -6249,6 +6346,8 @@ int main(void) cmocka_unit_test_setup_teardown( manifest_signed_by_an_unknown_root_key_is_rejected, setup, teardown), cmocka_unit_test_setup_teardown(malformed_jws_is_rejected, setup, teardown), + cmocka_unit_test_setup_teardown(large_signature_is_verified, setup, teardown), + cmocka_unit_test_setup_teardown(oversized_signature_is_reported_as_too_large, setup, teardown), cmocka_unit_test_setup_teardown(malformed_manifest_json_is_rejected, setup, teardown), cmocka_unit_test_setup_teardown( verify_file_hash_rejects_an_unsupported_algorithm, setup, teardown), From 97fa924f4b932729c16c1bbd121b1b16e1508d60 Mon Sep 17 00:00:00 2001 From: ewertons <12401690+ewertons@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:39:10 +0000 Subject: [PATCH 2/4] [C] Software updates: decode the SJWK in place; size ESP32 NVS for checkpoints - The SJWK signature and payload, and the signing key in it, are decoded in place within the decoded outer header, with a decoder that is safe in place. Only the SJWK header, the manifest signature or the JWS payload is decoded beside the outer header, so a near-limit offer whose size is mostly the signing key verifies in the client's scratch. - ESP32 sample: AZ_IOT_SU_REQUEST_BUFFER_SIZE=8192 for the whole build and a 36 KB NVS partition, so two generations of the largest checkpoint (8844 bytes) fit while NVS replaces one with the other. --- c/CHANGELOG.md | 11 +- c/docs/eng/test-coverage.md | 1 + .../software_update/esp32/CMakeLists.txt | 7 + c/samples/software_update/esp32/README.md | 2 +- .../software_update/esp32/main/app_main.c | 2 +- .../software_update/esp32/partitions.csv | 6 +- .../esp32/partitions_secure.csv | 2 +- c/src/features/su/su_client.c | 184 ++++++++++++------ c/tests/unit/su_client_test.c | 22 +++ 9 files changed, 163 insertions(+), 74 deletions(-) diff --git a/c/CHANGELOG.md b/c/CHANGELOG.md index daf36eb64..f8904e58c 100644 --- a/c/CHANGELOG.md +++ b/c/CHANGELOG.md @@ -42,8 +42,9 @@ session stays up; once supplied, the session disconnects and reconnects with it. See [docs/connecting.md](docs/connecting.md#authentication). - Software updates: `AZ_IOT_SU_REQUEST_BUFFER_SIZE` defaults to 16384 (was 4096), so offers above 4 KiB are no longer refused; the client struct grows by about 24 KiB. The manifest signature is - decoded into the client's own scratch rather than fixed stack buffers, so a signing key or JWS - header of any size that fits the request buffer verifies, and the verifier's stack drops from - about 6.5 KiB to under 1 KiB. `az_iot_su_parse_update_request()` uses - `AZ_IOT_SU_VERIFY_SCRATCH_SIZE` (8192) bytes of stack instead. A part that does not fit is - logged as too large, not as invalid. + decoded into the client's own scratch, mostly in place, rather than fixed stack buffers: a + signature that fits the request buffer verifies unless its SJWK header alone exceeds a quarter + of it, and the verifier's stack drops from about 6.5 KiB to under 1 KiB. + `az_iot_su_parse_update_request()` uses `AZ_IOT_SU_VERIFY_SCRATCH_SIZE` (8192) bytes of stack + instead. A part that does not fit is logged as too large, not as invalid. The ESP32 sample sets + the request buffer to 8192 and its NVS partition to 36 KB (erase the flash to move to it). diff --git a/c/docs/eng/test-coverage.md b/c/docs/eng/test-coverage.md index 5c9aa6b08..10e8b3c33 100644 --- a/c/docs/eng/test-coverage.md +++ b/c/docs/eng/test-coverage.md @@ -954,6 +954,7 @@ RS256 signature verification) through the crypto backends in `c/adapters/crypto_ | | Standalone entry points check the backend they need | `az_iot_su_parse_update_request()`: no backend is INVALID_ARG, no `verify_rs256` is NOT_SUPPORTED, and non-NULL outputs are zeroed on both. `az_iot_su_verify_file_hash()`: no backend is INVALID_ARG; a SHA-256-only backend verifies. | unit | Done | [standalone_entry_points_check_the_backend_they_need](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c) | | | Manifest signed by an unknown root key is rejected | End-to-end through `az_iot_su_parse_update_request()`. | unit | Done | [manifest_signed_by_an_unknown_root_key_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1428) | | | Malformed jws is rejected | Wrong segment count, bad base64url, missing header. | unit | Done | [malformed_jws_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1449) | +| | Near-limit nested signing key is verified | An offer near `AZ_IOT_SU_REQUEST_BUFFER_SIZE`, almost all signing JWK, verifies in the managed client. | unit | Done | [near_limit_nested_signing_key_is_verified](../../tests/unit/su_client_test.c) | | | Large signature is verified | JWS header and signing key over 2 KiB in an offer over 4 KiB; modulus in standard base64 with escaped slashes (managed and public parser). | unit | Done | [large_signature_is_verified](../../tests/unit/su_client_test.c) | | | Oversized signature is reported as too large | A header that does not fit `AZ_IOT_SU_VERIFY_SCRATCH_SIZE` is logged as too large; a malformed one as not valid. | unit | Done | [oversized_signature_is_reported_as_too_large](../../tests/unit/su_client_test.c) | | | Malformed manifest JSON is rejected | — | unit | Done | [malformed_manifest_json_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1481) | diff --git a/c/samples/software_update/esp32/CMakeLists.txt b/c/samples/software_update/esp32/CMakeLists.txt index ee5722923..a5a7124fa 100644 --- a/c/samples/software_update/esp32/CMakeLists.txt +++ b/c/samples/software_update/esp32/CMakeLists.txt @@ -18,4 +18,11 @@ cmake_minimum_required(VERSION 3.16) set(EXTRA_COMPONENT_DIRS "${CMAKE_CURRENT_LIST_DIR}/components") include($ENV{IDF_PATH}/tools/cmake/project.cmake) + +# Applies to the SDK component and the app alike, as their struct layouts must +# match. Halves the 16 KiB default: a one-file offer is about 5 KiB, and NVS +# (partitions*.csv) must hold two generations of the largest checkpoint +# (AZ_IOT_SU_STATE_BLOB_MAX_SIZE, about 8.6 KiB) while one replaces the other. +idf_build_set_property(COMPILE_DEFINITIONS "AZ_IOT_SU_REQUEST_BUFFER_SIZE=8192" APPEND) + project(su_esp32) diff --git a/c/samples/software_update/esp32/README.md b/c/samples/software_update/esp32/README.md index 35954743c..a22fd778f 100644 --- a/c/samples/software_update/esp32/README.md +++ b/c/samples/software_update/esp32/README.md @@ -49,7 +49,7 @@ sequenceDiagram | `install` | `esp_ota_end` + `esp_ota_set_boot_partition`; returns `REBOOT_REQUIRED` | | `apply` | No-op; the new image is already running | | `restore` | Rollback to the running partition | -| `persist` / `load` state | NVS namespace `su_sample`, key `wf_state` (resume across the reboot) | +| `persist` / `load` state | NVS namespace `su_sample`, key `wf_state` (resume across the reboot). The 36 KB `nvs` partition holds two generations of the largest checkpoint; [CMakeLists.txt](CMakeLists.txt) sets `AZ_IOT_SU_REQUEST_BUFFER_SIZE` to 8192 to bound it. Erase the flash when moving from the earlier 24 KB layout. | Once connected to its hub, the new image calls `su_esp32_ota_mark_valid()` (`esp_ota_mark_app_valid_cancel_rollback`). If it crashes or never connects, the diff --git a/c/samples/software_update/esp32/main/app_main.c b/c/samples/software_update/esp32/main/app_main.c index 841866f22..f6a2a0c3c 100644 --- a/c/samples/software_update/esp32/main/app_main.c +++ b/c/samples/software_update/esp32/main/app_main.c @@ -295,7 +295,7 @@ void app_main(void) dp.installed_update_id.version = SU_UPDATE_VERSION; static AZ_IOT_SU_DEVICE_PROPERTIES_STORAGE(dp_buffer); - /* ~38 KB: too large for the 12 KB main task stack. */ + /* ~22 KB: too large for the 12 KB main task stack. */ static az_iot_su_client su; az_iot_su_client_config_options su_opts = az_iot_su_client_config_options_default(); su_opts.hooks = &hooks; diff --git a/c/samples/software_update/esp32/partitions.csv b/c/samples/software_update/esp32/partitions.csv index 4f5f336a1..d3fc65ebb 100644 --- a/c/samples/software_update/esp32/partitions.csv +++ b/c/samples/software_update/esp32/partitions.csv @@ -6,8 +6,8 @@ # Sized for a 4 MB flash (the original ESP32-WROOM-32 module). # # Name, Type, SubType, Offset, Size, Flags -nvs, data, nvs, 0x9000, 0x6000, -otadata, data, ota, 0xf000, 0x2000, -phy_init, data, phy, 0x11000, 0x1000, +nvs, data, nvs, 0x9000, 0x9000, +otadata, data, ota, 0x12000, 0x2000, +phy_init, data, phy, 0x14000, 0x1000, ota_0, app, ota_0, 0x20000, 0x1D0000, ota_1, app, ota_1, 0x1F0000, 0x1D0000, diff --git a/c/samples/software_update/esp32/partitions_secure.csv b/c/samples/software_update/esp32/partitions_secure.csv index 6b38ec58f..008e62ed0 100644 --- a/c/samples/software_update/esp32/partitions_secure.csv +++ b/c/samples/software_update/esp32/partitions_secure.csv @@ -6,7 +6,7 @@ # efuse_em is used only with sdkconfig.secure_virtual_efuse. # # Name, Type, SubType, Offset, Size, Flags -nvs, data, nvs, , 0x6000, +nvs, data, nvs, , 0x9000, otadata, data, ota, , 0x2000, phy_init, data, phy, , 0x1000, nvs_keys, data, nvs_keys, , 0x1000, encrypted diff --git a/c/src/features/su/su_client.c b/c/src/features/su/su_client.c index 6dce43813..66ff678e0 100644 --- a/c/src/features/su/su_client.c +++ b/c/src/features/su/su_client.c @@ -242,10 +242,14 @@ static void result_overall_success(az_iot_su_client* client) * * Buffers: the chain decodes its base64url segments into a caller-supplied * scratch, not fixed stack arrays, because signing keys and their headers grow - * as keys rotate. The managed client passes its persistence scratch, which holds - * more than AZ_IOT_SU_REQUEST_BUFFER_SIZE, so any signature that fits the - * request buffer decodes. az_iot_su_parse_update_request() uses - * AZ_IOT_SU_VERIFY_SCRATCH_SIZE bytes of stack. Values that do not fit are + * as keys rotate. Only the outer JWS header is decoded into it beside one of: + * the SJWK header (alg, kid), the manifest signature or the JWS payload; the + * SJWK signature, payload and signing key are decoded in place within the + * header. Decoding shrinks base64 by a quarter, so the managed client, which + * passes its persistence scratch (more than AZ_IOT_SU_REQUEST_BUFFER_SIZE), + * verifies any signature that fits the request buffer unless its SJWK header + * alone exceeds a quarter of that buffer. az_iot_su_parse_update_request() + * uses AZ_IOT_SU_VERIFY_SCRATCH_SIZE bytes of stack. Values that do not fit are * logged as too large, not as invalid. */ @@ -302,65 +306,107 @@ typedef struct int32_t need; } su_scratch; +/** @brief 6-bit value of base64url (or, if @p std, standard base64) character @p c; -1 if none. */ +static int32_t b64_value(uint8_t c, bool std) +{ + if (c >= 'A' && c <= 'Z') + { + return c - 'A'; + } + if (c >= 'a' && c <= 'z') + { + return c - 'a' + 26; + } + if (c >= '0' && c <= '9') + { + return c - '0' + 52; + } + if (c == (std ? '+' : '-')) + { + return 62; + } + if (c == (std ? '/' : '_')) + { + return 63; + } + return -1; +} + /** - * @brief Decode base64url @p src into the free part of @p s and claim the bytes written. + * @brief Decode unpadded base64url, or padded standard base64, from @p src into @p dst. + * + * Writes never pass the input read so far, so @p dst may be az_span_ptr(@p src) (in place). * - * @param s Scratch. * @param src Encoded text. - * @param allow_std Also accept standard (padded) base64: the service encodes the signing-key - * modulus that way. - * @param out The decoded bytes, in @p s. - * @return AZ_IOT_OK; AZ_IOT_ERR_NOT_ENOUGH_SPACE if they do not fit; AZ_IOT_ERR_INVALID_ARG if - * @p src is empty or not valid. + * @param allow_std Also accept standard base64: the service encodes the signing-key modulus that + * way. Chosen when @p src has '+', '/' or '='. + * @param dst Destination of at least (size of @p src) * 3 / 4 bytes. + * @param out_len Bytes written. + * @return AZ_IOT_OK; AZ_IOT_ERR_INVALID_ARG if @p src is empty or malformed. */ -static az_iot_result scratch_b64_decode(su_scratch* s, az_span src, bool allow_std, az_span* out) +static az_iot_result b64_decode(az_span src, bool allow_std, uint8_t* dst, int32_t* out_len) { + const uint8_t* p = az_span_ptr(src); int32_t len = az_span_size(src); - /* Exact for unpadded base64url; an upper bound for padded base64. */ - s->need = (len / 4) * 3 + ((len % 4) * 3) / 4; - if (s->need <= 0) + bool std = false; + for (int32_t i = 0; allow_std && i < len; ++i) { - return AZ_IOT_ERR_INVALID_ARG; + std = std || p[i] == '+' || p[i] == '/' || p[i] == '='; } - if (s->need > s->cap - s->used) + if (std) { - return AZ_IOT_ERR_NOT_ENOUGH_SPACE; + if (len % 4 != 0) + { + return AZ_IOT_ERR_INVALID_ARG; + } + for (int32_t pad = 0; pad < 2 && len > 0 && p[len - 1] == '='; ++pad) + { + --len; + } } - az_span dst = az_span_create(s->buf + s->used, s->cap - s->used); - int32_t written = 0; - /* Check the alphabet first: azure-sdk-for-c shifts a negative value on a - * character outside it. */ - bool std = false; - const uint8_t* p = az_span_ptr(src); + if (len <= 0 || len % 4 == 1) + { + return AZ_IOT_ERR_INVALID_ARG; + } + uint32_t acc = 0; + int32_t bits = 0; + int32_t w = 0; for (int32_t i = 0; i < len; ++i) { - uint8_t c = p[i]; - bool common = (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9'); - if (common || c == '-' || c == '_') + int32_t v = b64_value(p[i], std); + if (v < 0) { - continue; + return AZ_IOT_ERR_INVALID_ARG; } - /* Standard base64: '+', '/', and up to two trailing '='. */ - if (!allow_std || (c != '+' && c != '/' && !(c == '=' && i >= len - 2 && p[len - 1] == '='))) + acc = (acc << 6) | (uint32_t)v; + bits += 6; + if (bits >= 8) { - return AZ_IOT_ERR_INVALID_ARG; + bits -= 8; + dst[w++] = (uint8_t)(acc >> bits); + acc &= (1u << bits) - 1u; } - std = true; } - if (std) + *out_len = w; + return AZ_IOT_OK; +} + +/** + * @brief Decode @p src (see b64_decode()) into the free part of @p s and claim the bytes written. + * + * @return AZ_IOT_OK with @p out in @p s; AZ_IOT_ERR_NOT_ENOUGH_SPACE if they do not fit; + * AZ_IOT_ERR_INVALID_ARG if @p src is empty or malformed. + */ +static az_iot_result scratch_b64_decode(su_scratch* s, az_span src, bool allow_std, az_span* out) +{ + int32_t len = az_span_size(src); + s->need = (len / 4) * 3 + ((len % 4) * 3) / 4; + if (s->need > s->cap - s->used) { - for (int32_t i = 0; i < len; ++i) - { - if (p[i] == '-' || p[i] == '_') - { - return AZ_IOT_ERR_INVALID_ARG; - } - } + return AZ_IOT_ERR_NOT_ENOUGH_SPACE; } - az_result r = std - ? (len % 4 == 0 ? az_base64_decode(dst, src, &written) : AZ_ERROR_UNEXPECTED_END) - : az_base64_url_decode(dst, src, &written); - if (az_result_failed(r) || written <= 0) + int32_t written = 0; + if (b64_decode(src, allow_std, s->buf + s->used, &written) != AZ_IOT_OK) { return AZ_IOT_ERR_INVALID_ARG; } @@ -369,6 +415,23 @@ static az_iot_result scratch_b64_decode(su_scratch* s, az_span src, bool allow_s return AZ_IOT_OK; } +/** + * @brief Decode writable @p src (see b64_decode()) in place. + * + * @return AZ_IOT_OK with @p out at the start of @p src; AZ_IOT_ERR_INVALID_ARG if empty or + * malformed. + */ +static az_iot_result b64_decode_in_place(az_span src, bool allow_std, az_span* out) +{ + int32_t written = 0; + if (b64_decode(src, allow_std, az_span_ptr(src), &written) != AZ_IOT_OK) + { + return AZ_IOT_ERR_INVALID_ARG; + } + *out = az_span_create(az_span_ptr(src), written); + return AZ_IOT_OK; +} + /** * @brief Find top-level property @p name of JSON object @p obj. * @@ -639,7 +702,8 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 2: sjwk is not a valid 3-part token"); } - /* Steps 2 and 3 release their scratch; step 4 keeps the signing key. */ + /* Only the SJWK header is decoded beside the outer header; the SJWK signature + * and payload, and the signing key in it, are decoded in place within it. */ int32_t after_hdr = s.used; const az_iot_su_root_key* root = NULL; { @@ -679,8 +743,7 @@ static int32_t verify_manifest_core( /* 3. Verify the SJWK signature with the resolved root key. */ { az_span s_sig; - SU_VERIFY_DECODED( - scratch_b64_decode(&s, s_sig_b64, false, &s_sig), &s, "step 3: sjwk signature"); + SU_VERIFY_DECODED(b64_decode_in_place(s_sig_b64, false, &s_sig), &s, "step 3: sjwk signature"); if (crypto->verify_rs256( crypto, root->modulus, @@ -696,18 +759,17 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 3: sjwk signature does not verify against the root key"); } } - s.used = after_hdr; /* 4. Parse the now-trusted SJWK payload as a JWK → signing key (n, e). The - * modulus may be standard base64; the payload is unescaped in place, after - * `e` is read. */ + * modulus may be standard base64; it is unescaped in place, after `e` is + * copied out. The SJWK's signed bytes are no longer needed. */ az_span n_raw; az_span e_raw; + char e_b64[64]; { az_span spl; - SU_VERIFY_DECODED(scratch_b64_decode(&s, s_pl_b64, false, &spl), &s, "step 4: sjwk payload"); + SU_VERIFY_DECODED(b64_decode_in_place(s_pl_b64, false, &spl), &s, "step 4: sjwk payload"); - char e_b64[64]; az_span e_field = AZ_SPAN_EMPTY; az_span n_field = AZ_SPAN_EMPTY; az_iot_result r @@ -721,13 +783,12 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 4: signing JWK is missing modulus (n) or exponent (e)"); } SU_VERIFY_DECODED( - scratch_b64_decode(&s, n_field, true, &n_raw), &s, "step 4: signing JWK modulus (n)"); + b64_decode_in_place(n_field, true, &n_raw), &s, "step 4: signing JWK modulus (n)"); SU_VERIFY_DECODED( - scratch_b64_decode(&s, e_field, true, &e_raw), &s, "step 4: signing JWK exponent (e)"); + b64_decode_in_place(e_field, true, &e_raw), &s, "step 4: signing JWK exponent (e)"); } /* 5. Verify the manifest JWS signature with the trusted signing key. */ - int32_t after_key = s.used; { az_span m_sig; SU_VERIFY_DECODED( @@ -747,7 +808,7 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 5: manifest signature does not verify against the signing key"); } } - s.used = after_key; + s.used = after_hdr; /* 6. Bind the signed manifest to THIS deployment: the manifest JWS payload * carries SHA-256(manifest body); recompute and compare. */ @@ -765,12 +826,9 @@ static int32_t verify_manifest_core( SU_VERIFY_FAIL("step 6: manifest JWS payload has no sha256"); } - uint8_t expected[32]; - int32_t exp_written = 0; - if (r != AZ_IOT_OK - || az_result_failed(az_base64_decode( - az_span_create(expected, (int32_t)sizeof(expected)), hash_field, &exp_written)) - || exp_written != 32) + az_span expected = AZ_SPAN_EMPTY; + if (r != AZ_IOT_OK || b64_decode_in_place(hash_field, true, &expected) != AZ_IOT_OK + || az_span_size(expected) != 32) { SU_VERIFY_FAIL("step 6: manifest sha256 field is not a 32-byte base64 hash"); } @@ -781,7 +839,7 @@ static int32_t verify_manifest_core( { SU_VERIFY_FAIL("step 6: SHA-256 failed over the manifest body"); } - if (!az_span_is_content_equal(AZ_SPAN_FROM_BUFFER(expected), AZ_SPAN_FROM_BUFFER(actual))) + if (!az_span_is_content_equal(expected, AZ_SPAN_FROM_BUFFER(actual))) { SU_VERIFY_FAIL("step 6: computed manifest SHA-256 does not match the signed hash"); } diff --git a/c/tests/unit/su_client_test.c b/c/tests/unit/su_client_test.c index a393fdd04..9a856c2cf 100644 --- a/c/tests/unit/su_client_test.c +++ b/c/tests/unit/su_client_test.c @@ -4676,6 +4676,27 @@ static void large_signature_is_verified(void** state) AZ_IOT_OK); } +/* An offer near AZ_IOT_SU_REQUEST_BUFFER_SIZE whose size is almost all the + * signing JWK, nested in the SJWK in the outer header. Decoding the JWK beside + * the outer header would need more than the client's scratch. */ +static void near_limit_nested_signing_key_is_verified(void** state) +{ + fixture* fx = (fixture*)*state; + open_to_connected(fx); + + static char jws[AZ_IOT_SU_REQUEST_BUFFER_SIZE]; + static char patch[AZ_IOT_SU_REQUEST_BUFFER_SIZE]; + size_t jwk_pad = (AZ_IOT_SU_REQUEST_BUFFER_SIZE - 1500) * 9 / 16; + build_jws_ex(jws, (int32_t)sizeof(jws), jwk_pad, 0, false); + int n = snprintf(patch, sizeof(patch), k_patch_fmt, "near-limit", "1.1", jws); + assert_true(n > AZ_IOT_SU_REQUEST_BUFFER_SIZE * 7 / 8 && (size_t)n < sizeof(patch)); + + inject_patch(fx, patch); + pump(fx, 40); + assert_int_equal(az_iot_su_client_get_state(&fx->su), AZ_IOT_SU_STATE_IDLE); + assert_int_equal(fx->chan.last_report.outcome, AZ_IOT_SU_OUTCOME_SUCCEEDED); +} + /* A signature part that does not fit the scratch is logged as too large; a * malformed one as not valid. */ static void oversized_signature_is_reported_as_too_large(void** state) @@ -6347,6 +6368,7 @@ int main(void) manifest_signed_by_an_unknown_root_key_is_rejected, setup, teardown), cmocka_unit_test_setup_teardown(malformed_jws_is_rejected, setup, teardown), cmocka_unit_test_setup_teardown(large_signature_is_verified, setup, teardown), + cmocka_unit_test_setup_teardown(near_limit_nested_signing_key_is_verified, setup, teardown), cmocka_unit_test_setup_teardown(oversized_signature_is_reported_as_too_large, setup, teardown), cmocka_unit_test_setup_teardown(malformed_manifest_json_is_rejected, setup, teardown), cmocka_unit_test_setup_teardown( From 7a372b085eaf8d957fd865c3b963e7f94dd67b5d Mon Sep 17 00:00:00 2001 From: ewertons <12401690+ewertons@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:53:55 +0000 Subject: [PATCH 3/4] [C] Software updates: decode base64 through az_core in a canonical helper - az_iot_base64_decode() (src/core/base64.c) validates the alphabet, padding and zero trailing bits, then decodes through az_base64_url_decode() / az_base64_decode() in 64-character blocks, so it may decode in place. Replaces the local decoder in su_client.c. - Tests: base64_test.c (tails, alphabets, malformed and non-canonical input, bounds, block boundaries in place). The SU mock records the key, exponent and signature passed to verify_rs256; the large-signature and near-limit tests assert the decoded bytes. --- c/docs/eng/software-updates.md | 3 +- c/docs/eng/test-coverage.md | 8 +- .../components/azure-iot-sdk/CMakeLists.txt | 1 + c/src/CMakeLists.txt | 1 + c/src/core/base64.c | 118 ++++++++++++ c/src/core/internal/base64.h | 55 ++++++ c/src/features/su/su_client.c | 115 ++---------- c/tests/CMakeLists.txt | 3 + c/tests/unit/base64_test.c | 168 ++++++++++++++++++ c/tests/unit/su_client_test.c | 40 ++++- 10 files changed, 399 insertions(+), 113 deletions(-) create mode 100644 c/src/core/base64.c create mode 100644 c/src/core/internal/base64.h create mode 100644 c/tests/unit/base64_test.c diff --git a/c/docs/eng/software-updates.md b/c/docs/eng/software-updates.md index dc64ec2df..9c0a344a9 100644 --- a/c/docs/eng/software-updates.md +++ b/c/docs/eng/software-updates.md @@ -1124,7 +1124,7 @@ updates core owns all orchestration**: | Responsibility | Owner | |----------------|-------| -| JWS compact parsing, base64url decode | **core** (uses `az::core` base64/JSON it already links) | +| JWS compact parsing, base64url decode | **core** (uses `az::core` base64/JSON it already links; base64 through `az_iot_base64_decode()`) | | SJWK extraction, JWK `n`/`e` parsing | **core** | | Root-key store, `kid` resolution | **core** (see §7) | | Revocation enforcement (disabled kids) | **core** | @@ -1463,6 +1463,7 @@ This avoids duplicating the well-tested JSON parsing logic and keeps us aligned | Workflow struct with `action`, `id`, `retry_timestamp` | Only `id` is used (from `workflowId`) | | File hash parsing (`hash_type` + `hash_value` as `az_span`) | ✅ Sufficient | | `az_json_string_unescape()` for manifest string unescaping | Not used: it stops at `\u` escapes. The SDK decodes JSON strings with `az_iot_json_string_decode()` (`src/core/json_string.c`) | +| `az_base64_url_decode()` / `az_base64_decode()` for JWS segments | Used through `az_iot_base64_decode()` (`src/core/base64.c`), which first rejects characters outside the alphabet (az_core left-shifts a negative value on them), misplaced padding and nonzero trailing bits, and decodes in blocks so the output may overwrite the input | | `az_iot_hub_client_properties_writer_*` for PnP component wrapping | Not used (device twin only) | ### Configurable Limits (No Source Change Needed) diff --git a/c/docs/eng/test-coverage.md b/c/docs/eng/test-coverage.md index 10e8b3c33..826a65cb1 100644 --- a/c/docs/eng/test-coverage.md +++ b/c/docs/eng/test-coverage.md @@ -955,7 +955,7 @@ RS256 signature verification) through the crypto backends in `c/adapters/crypto_ | | Manifest signed by an unknown root key is rejected | End-to-end through `az_iot_su_parse_update_request()`. | unit | Done | [manifest_signed_by_an_unknown_root_key_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1428) | | | Malformed jws is rejected | Wrong segment count, bad base64url, missing header. | unit | Done | [malformed_jws_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1449) | | | Near-limit nested signing key is verified | An offer near `AZ_IOT_SU_REQUEST_BUFFER_SIZE`, almost all signing JWK, verifies in the managed client. | unit | Done | [near_limit_nested_signing_key_is_verified](../../tests/unit/su_client_test.c) | -| | Large signature is verified | JWS header and signing key over 2 KiB in an offer over 4 KiB; modulus in standard base64 with escaped slashes (managed and public parser). | unit | Done | [large_signature_is_verified](../../tests/unit/su_client_test.c) | +| | Large signature is verified | JWS header and signing key over 2 KiB in an offer over 4 KiB; modulus in standard base64 with escaped slashes, asserted as decoded with the exponent and signature (managed and public parser). | unit | Done | [large_signature_is_verified](../../tests/unit/su_client_test.c) | | | Oversized signature is reported as too large | A header that does not fit `AZ_IOT_SU_VERIFY_SCRATCH_SIZE` is logged as too large; a malformed one as not valid. | unit | Done | [oversized_signature_is_reported_as_too_large](../../tests/unit/su_client_test.c) | | | Malformed manifest JSON is rejected | — | unit | Done | [malformed_manifest_json_is_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1481) | | | Verify file hash rejects an unsupported algorithm | — | unit | Done | [verify_file_hash_rejects_an_unsupported_algorithm](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/su_client_test.c#L1534) | @@ -1154,6 +1154,12 @@ own, so they are unit-tested only. | | A short buffer fails and is wiped | One byte short, or too short for the string to sign: `AZ_IOT_ERR_NOT_ENOUGH_SPACE`, buffer zeroed. | unit | Done | [a_short_buffer_fails_and_is_wiped](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/sas_token_test.c) | | | Invalid arguments are rejected | NULLs, empty URI, 0 expiry, unusable crypto, empty key; `&`, space, newline, `=`, `/` in the URI or key name. | unit | Done | [invalid_arguments_are_rejected](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/sas_token_test.c) | | | Derive rejects bad arguments and zeroes its output | Empty or NULL ID or key, no crypto: `AZ_IOT_ERR_INVALID_ARG`, output zeroed. | unit | Done | [derive_rejects_bad_arguments_and_zeroes_its_output](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/sas_token_test.c) | +| Base64 decode | base64url decodes every tail length | 0, 2 and 3 trailing characters; `-`/`_`. | unit | Done | [base64url_decodes_every_tail_length](../../tests/unit/base64_test.c) | +| | Standard base64 is accepted only when allowed | Padded `+`/`/` text. | unit | Done | [standard_base64_is_accepted_only_when_allowed](../../tests/unit/base64_test.c) | +| | Malformed input is rejected | Empty, bad length, out-of-alphabet, mixed alphabets, misplaced or excess padding. | unit | Done | [malformed_input_is_rejected](../../tests/unit/base64_test.c) | +| | Nonzero trailing bits are rejected | Non-canonical text that az_core would decode to the same bytes. | unit | Done | [nonzero_trailing_bits_are_rejected](../../tests/unit/base64_test.c) | +| | Destination bounds are checked | One byte short fails; exact fits; NULL output rejected. | unit | Done | [destination_bounds_are_checked](../../tests/unit/base64_test.c) | +| | Block boundaries decode in place | Lengths around the 64-character block, separate and in place. | unit | Done | [block_boundaries_decode_in_place](../../tests/unit/base64_test.c) | | JSON string decode | Plain text is copied | — | unit | Done | [plain_text_is_copied](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/json_string_test.c) | | | Every two-character escape decodes | — | unit | Done | [every_two_character_escape_decodes](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/json_string_test.c) | | | Unicode escapes cover each UTF-8 width | 1-, 2- and 3-byte bounds. | unit | Done | [unicode_escapes_cover_each_utf8_width](https://github.com/Azure/azure-iot-sdk/blob/main/c/tests/unit/json_string_test.c) | diff --git a/c/samples/software_update/esp32/components/azure-iot-sdk/CMakeLists.txt b/c/samples/software_update/esp32/components/azure-iot-sdk/CMakeLists.txt index 94cbdd976..d16e08c8d 100644 --- a/c/samples/software_update/esp32/components/azure-iot-sdk/CMakeLists.txt +++ b/c/samples/software_update/esp32/components/azure-iot-sdk/CMakeLists.txt @@ -30,6 +30,7 @@ idf_component_register( "${AZ_IOT_ROOT}/src/core/proto3.c" "${AZ_IOT_ROOT}/src/core/span_writer.c" "${AZ_IOT_ROOT}/src/core/json_string.c" + "${AZ_IOT_ROOT}/src/core/base64.c" "${AZ_IOT_ROOT}/src/core/message.c" "${AZ_IOT_ROOT}/src/core/certificate_provider_pem.c" "${AZ_IOT_ROOT}/src/core/crypto.c" diff --git a/c/src/CMakeLists.txt b/c/src/CMakeLists.txt index facd8d196..db04b3d50 100644 --- a/c/src/CMakeLists.txt +++ b/c/src/CMakeLists.txt @@ -16,6 +16,7 @@ add_library(az_iot_core core/proto3.c core/span_writer.c core/json_string.c + core/base64.c core/message.c core/certificate_provider_pem.c core/crypto.c diff --git a/c/src/core/base64.c b/c/src/core/base64.c new file mode 100644 index 000000000..cebdb7b34 --- /dev/null +++ b/c/src/core/base64.c @@ -0,0 +1,118 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license +// information. + +/* SPDX-License-Identifier: MIT */ +#include +#include + +#include + +#include "internal/base64.h" + +/** @brief Characters decoded per az_core call; a multiple of 4. */ +#define AZ_IOT_BASE64_BLOCK 64 + +/** + * @brief 6-bit value of a base64 character. + * + * @param c The character. + * @param std Standard alphabet ('+', '/') rather than base64url ('-', '_'). + * @return The value; -1 outside the alphabet, including '='. + */ +static int32_t b64_value(uint8_t c, bool std) +{ + if (c >= 'A' && c <= 'Z') + { + return c - 'A'; + } + if (c >= 'a' && c <= 'z') + { + return c - 'a' + 26; + } + if (c >= '0' && c <= '9') + { + return c - '0' + 52; + } + if (c == (std ? '+' : '-')) + { + return 62; + } + if (c == (std ? '/' : '_')) + { + return 63; + } + return -1; +} + +az_iot_result az_iot_base64_decode(az_span in, bool allow_std, az_span out, az_span* out_decoded) +{ + const uint8_t* p = az_span_ptr(in); + int32_t len = az_span_size(in); + if (out_decoded == NULL || len <= 0) + { + return AZ_IOT_ERR_INVALID_ARG; + } + + bool std = false; + for (int32_t i = 0; allow_std && i < len; ++i) + { + std = std || p[i] == '+' || p[i] == '/' || p[i] == '='; + } + int32_t data_len = len; + if (std) + { + if (len % 4 != 0) + { + return AZ_IOT_ERR_INVALID_ARG; + } + while (data_len > len - 2 && p[data_len - 1] == '=') + { + --data_len; + } + } + int32_t rem = data_len % 4; + if (data_len <= 0 || rem == 1 || (std && len - data_len != (4 - rem) % 4)) + { + return AZ_IOT_ERR_INVALID_ARG; + } + for (int32_t i = 0; i < data_len; ++i) + { + if (b64_value(p[i], std) < 0) + { + return AZ_IOT_ERR_INVALID_ARG; + } + } + /* Canonical form: the bits past the last whole byte are zero. */ + int32_t last = b64_value(p[data_len - 1], std); + if ((rem == 2 && (last & 0x0F) != 0) || (rem == 3 && (last & 0x03) != 0)) + { + return AZ_IOT_ERR_INVALID_ARG; + } + if ((data_len / 4) * 3 + (rem * 3) / 4 > az_span_size(out)) + { + return AZ_IOT_ERR_NOT_ENOUGH_SPACE; + } + + /* Each block is read before its bytes are written, and output never passes + * input, so decoding in place is safe. */ + uint8_t block[AZ_IOT_BASE64_BLOCK / 4 * 3]; + uint8_t* dst = az_span_ptr(out); + int32_t w = 0; + for (int32_t i = 0; i < len; i += AZ_IOT_BASE64_BLOCK) + { + int32_t end = (len - i < AZ_IOT_BASE64_BLOCK) ? len : i + AZ_IOT_BASE64_BLOCK; + az_span src = az_span_slice(in, i, end); + int32_t written = 0; + az_result r = std ? az_base64_decode(AZ_SPAN_FROM_BUFFER(block), src, &written) + : az_base64_url_decode(AZ_SPAN_FROM_BUFFER(block), src, &written); + if (az_result_failed(r)) + { + return AZ_IOT_ERR_INVALID_ARG; + } + memmove(dst + w, block, (size_t)written); + w += written; + } + *out_decoded = az_span_create(dst, w); + return AZ_IOT_OK; +} diff --git a/c/src/core/internal/base64.h b/c/src/core/internal/base64.h new file mode 100644 index 000000000..00f5c3cbd --- /dev/null +++ b/c/src/core/internal/base64.h @@ -0,0 +1,55 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license +// information. + +/* SPDX-License-Identifier: MIT */ +/** + * @file base64.h + * @brief Canonical base64 decoding, in place or not. + * + * Validates the input before handing it to az_core, which left-shifts a + * negative value on a character outside the alphabet and accepts nonzero + * trailing bits. Decodes in blocks through a small buffer, so the output may + * overwrite the input. + */ +#ifndef AZ_IOT_BASE64_H +#define AZ_IOT_BASE64_H + +#include + +#include + +#include "azure/iot/az_iot_result.h" + +#ifdef __cplusplus +extern "C" +{ +#endif + + /** + * @brief Decode canonical unpadded base64url or, if @p allow_std, padded standard base64. + * + * Standard base64 is chosen when @p in has '+', '/' or '='. Its length must be a multiple + * of 4, with at most two trailing '='. Unused trailing bits must be zero. + * + * @param in Encoded text. + * @param allow_std Also accept standard base64. + * @param out Destination. May start at az_span_ptr(@p in) (in place); must not + * otherwise overlap @p in. + * @param out_decoded The decoded slice of @p out. Untouched on error; @p out may then hold + * a partial result. + * @return AZ_IOT_OK; AZ_IOT_ERR_INVALID_ARG if @p in is empty, has a character outside the + * alphabet, misplaced padding, a bad length or nonzero trailing bits; + * AZ_IOT_ERR_NOT_ENOUGH_SPACE when @p out is too small. + */ + az_iot_result az_iot_base64_decode( + az_span in, + bool allow_std, + az_span out, + az_span* out_decoded); + +#ifdef __cplusplus +} +#endif + +#endif /* AZ_IOT_BASE64_H */ diff --git a/c/src/features/su/su_client.c b/c/src/features/su/su_client.c index 66ff678e0..edfe9537d 100644 --- a/c/src/features/su/su_client.c +++ b/c/src/features/su/su_client.c @@ -32,6 +32,7 @@ #include "azure/iot/az_iot_su.h" #include "internal/su_internal.h" +#include "internal/base64.h" #include "internal/crypto.h" #include "internal/json_string.h" #include "internal/mono_time.h" @@ -306,130 +307,34 @@ typedef struct int32_t need; } su_scratch; -/** @brief 6-bit value of base64url (or, if @p std, standard base64) character @p c; -1 if none. */ -static int32_t b64_value(uint8_t c, bool std) -{ - if (c >= 'A' && c <= 'Z') - { - return c - 'A'; - } - if (c >= 'a' && c <= 'z') - { - return c - 'a' + 26; - } - if (c >= '0' && c <= '9') - { - return c - '0' + 52; - } - if (c == (std ? '+' : '-')) - { - return 62; - } - if (c == (std ? '/' : '_')) - { - return 63; - } - return -1; -} - -/** - * @brief Decode unpadded base64url, or padded standard base64, from @p src into @p dst. - * - * Writes never pass the input read so far, so @p dst may be az_span_ptr(@p src) (in place). - * - * @param src Encoded text. - * @param allow_std Also accept standard base64: the service encodes the signing-key modulus that - * way. Chosen when @p src has '+', '/' or '='. - * @param dst Destination of at least (size of @p src) * 3 / 4 bytes. - * @param out_len Bytes written. - * @return AZ_IOT_OK; AZ_IOT_ERR_INVALID_ARG if @p src is empty or malformed. - */ -static az_iot_result b64_decode(az_span src, bool allow_std, uint8_t* dst, int32_t* out_len) -{ - const uint8_t* p = az_span_ptr(src); - int32_t len = az_span_size(src); - bool std = false; - for (int32_t i = 0; allow_std && i < len; ++i) - { - std = std || p[i] == '+' || p[i] == '/' || p[i] == '='; - } - if (std) - { - if (len % 4 != 0) - { - return AZ_IOT_ERR_INVALID_ARG; - } - for (int32_t pad = 0; pad < 2 && len > 0 && p[len - 1] == '='; ++pad) - { - --len; - } - } - if (len <= 0 || len % 4 == 1) - { - return AZ_IOT_ERR_INVALID_ARG; - } - uint32_t acc = 0; - int32_t bits = 0; - int32_t w = 0; - for (int32_t i = 0; i < len; ++i) - { - int32_t v = b64_value(p[i], std); - if (v < 0) - { - return AZ_IOT_ERR_INVALID_ARG; - } - acc = (acc << 6) | (uint32_t)v; - bits += 6; - if (bits >= 8) - { - bits -= 8; - dst[w++] = (uint8_t)(acc >> bits); - acc &= (1u << bits) - 1u; - } - } - *out_len = w; - return AZ_IOT_OK; -} - /** - * @brief Decode @p src (see b64_decode()) into the free part of @p s and claim the bytes written. + * @brief Decode @p src (see az_iot_base64_decode()) into the free part of @p s and claim it. * - * @return AZ_IOT_OK with @p out in @p s; AZ_IOT_ERR_NOT_ENOUGH_SPACE if they do not fit; + * @return AZ_IOT_OK with @p out in @p s; AZ_IOT_ERR_NOT_ENOUGH_SPACE if it does not fit; * AZ_IOT_ERR_INVALID_ARG if @p src is empty or malformed. */ static az_iot_result scratch_b64_decode(su_scratch* s, az_span src, bool allow_std, az_span* out) { int32_t len = az_span_size(src); s->need = (len / 4) * 3 + ((len % 4) * 3) / 4; - if (s->need > s->cap - s->used) - { - return AZ_IOT_ERR_NOT_ENOUGH_SPACE; - } - int32_t written = 0; - if (b64_decode(src, allow_std, s->buf + s->used, &written) != AZ_IOT_OK) + az_iot_result r = az_iot_base64_decode( + src, allow_std, az_span_create(s->buf + s->used, s->cap - s->used), out); + if (r == AZ_IOT_OK) { - return AZ_IOT_ERR_INVALID_ARG; + s->used += az_span_size(*out); } - *out = az_span_create(s->buf + s->used, written); - s->used += written; - return AZ_IOT_OK; + return r; } /** - * @brief Decode writable @p src (see b64_decode()) in place. + * @brief Decode writable @p src (see az_iot_base64_decode()) in place. * * @return AZ_IOT_OK with @p out at the start of @p src; AZ_IOT_ERR_INVALID_ARG if empty or * malformed. */ static az_iot_result b64_decode_in_place(az_span src, bool allow_std, az_span* out) { - int32_t written = 0; - if (b64_decode(src, allow_std, az_span_ptr(src), &written) != AZ_IOT_OK) - { - return AZ_IOT_ERR_INVALID_ARG; - } - *out = az_span_create(az_span_ptr(src), written); - return AZ_IOT_OK; + return az_iot_base64_decode(src, allow_std, src, out); } /** diff --git a/c/tests/CMakeLists.txt b/c/tests/CMakeLists.txt index 47f78448d..500c857b6 100644 --- a/c/tests/CMakeLists.txt +++ b/c/tests/CMakeLists.txt @@ -163,6 +163,9 @@ target_include_directories(az_iot_tests_retry_policy PRIVATE az_iot_add_cmocka_test(az_iot_tests_span_writer unit/span_writer_test.c) target_include_directories(az_iot_tests_span_writer PRIVATE ${PROJECT_SOURCE_DIR}/src/core) +az_iot_add_cmocka_test(az_iot_tests_base64 unit/base64_test.c) +target_include_directories(az_iot_tests_base64 PRIVATE + ${PROJECT_SOURCE_DIR}/src/core) az_iot_add_cmocka_test(az_iot_tests_json_string unit/json_string_test.c) target_include_directories(az_iot_tests_json_string PRIVATE ${PROJECT_SOURCE_DIR}/src/core) diff --git a/c/tests/unit/base64_test.c b/c/tests/unit/base64_test.c new file mode 100644 index 000000000..978c6aa43 --- /dev/null +++ b/c/tests/unit/base64_test.c @@ -0,0 +1,168 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license +// information. + +/* SPDX-License-Identifier: MIT */ +/* Unit tests for az_iot_base64_decode(): both alphabets, padding, trailing + * bits, block boundaries, in-place decoding and destination bounds. */ +#include +#include +#include +#include +#include + +#include + +#include + +#include "internal/base64.h" + +static az_span span_of(const char* s) +{ + return az_span_create((uint8_t*)(uintptr_t)s, (int32_t)strlen(s)); +} + +static az_iot_result decode(const char* in, bool allow_std, uint8_t* out, int32_t cap, az_span* got) +{ + return az_iot_base64_decode(span_of(in), allow_std, az_span_create(out, cap), got); +} + +static void expect_decoded(const char* in, bool allow_std, const uint8_t* want, int32_t want_len) +{ + uint8_t out[64]; + az_span got = AZ_SPAN_EMPTY; + assert_int_equal(decode(in, allow_std, out, (int32_t)sizeof(out), &got), AZ_IOT_OK); + assert_int_equal(az_span_size(got), want_len); + assert_memory_equal(az_span_ptr(got), want, (size_t)want_len); +} + +static void expect_invalid(const char* in, bool allow_std) +{ + uint8_t out[64]; + az_span got = AZ_SPAN_EMPTY; + assert_int_equal(decode(in, allow_std, out, (int32_t)sizeof(out), &got), AZ_IOT_ERR_INVALID_ARG); +} + +/* Unpadded base64url of `n` bytes (i * 7 + 3) into `dst`. */ +static int32_t encode_url(uint8_t* bytes, int32_t n, char* dst, int32_t cap) +{ + for (int32_t i = 0; i < n; ++i) + { + bytes[i] = (uint8_t)(i * 7 + 3); + } + int32_t w = 0; + assert_true(az_result_succeeded( + az_base64_encode(az_span_create((uint8_t*)dst, cap), az_span_create(bytes, n), &w))); + int32_t o = 0; + for (int32_t i = 0; i < w; ++i) + { + char c = dst[i]; + if (c != '=') + { + dst[o++] = c == '+' ? '-' : (c == '/' ? '_' : c); + } + } + dst[o] = '\0'; + return o; +} + +static void base64url_decodes_every_tail_length(void** state) +{ + (void)state; + static const uint8_t k[] = { 0x01, 0x02, 0x03 }; + expect_decoded("AQID", false, k, 3); + expect_decoded("AQI", false, k, 2); + expect_decoded("AQ", false, k, 1); + static const uint8_t ff[] = { 0xFB, 0xFF, 0xFF }; + expect_decoded("-___", false, ff, 3); +} + +static void standard_base64_is_accepted_only_when_allowed(void** state) +{ + (void)state; + static const uint8_t ff[] = { 0xFF, 0xFF, 0xFF }; + static const uint8_t k[] = { 0x01, 0x02, 0x03 }; + expect_decoded("////", true, ff, 3); + expect_decoded("AQI=", true, k, 2); + expect_decoded("AQ==", true, k, 1); + expect_decoded("AQID", true, k, 3); + expect_invalid("////", false); + expect_invalid("AQ==", false); +} + +static void malformed_input_is_rejected(void** state) +{ + (void)state; + expect_invalid("", true); + expect_invalid("A", false); + expect_invalid("AQIDB", false); + expect_invalid("AQ!D", false); + expect_invalid("AQ D", false); + expect_invalid("-+AA", true); /* mixed alphabets */ + expect_invalid("AQ=D", true); /* '=' inside */ + expect_invalid("A===", true); /* three pads */ + expect_invalid("AQ=", true); /* length not a multiple of 4 */ + expect_invalid("AQI==", true); + expect_invalid("AQ=", false); +} + +static void nonzero_trailing_bits_are_rejected(void** state) +{ + (void)state; + expect_invalid("AR", false); /* would also decode to 0x01 */ + expect_invalid("AQJ", false); + expect_invalid("AR==", true); + expect_invalid("AQJ=", true); +} + +static void destination_bounds_are_checked(void** state) +{ + (void)state; + uint8_t out[3]; + az_span got = AZ_SPAN_EMPTY; + assert_int_equal(decode("AQID", false, out, 2, &got), AZ_IOT_ERR_NOT_ENOUGH_SPACE); + assert_int_equal(decode("AQID", false, out, 3, &got), AZ_IOT_OK); + assert_int_equal(decode("AQ", false, out, 1, &got), AZ_IOT_OK); + assert_int_equal(decode("AQ==", true, out, 1, &got), AZ_IOT_OK); + assert_int_equal(az_iot_base64_decode(span_of("AQID"), false, AZ_SPAN_FROM_BUFFER(out), NULL), + AZ_IOT_ERR_INVALID_ARG); +} + +/* Lengths around the 64-character block, decoded into a separate buffer and in place. */ +static void block_boundaries_decode_in_place(void** state) +{ + (void)state; + static const int32_t lengths[] = { 1, 46, 47, 48, 49, 50, 95, 96, 97, 200 }; + for (size_t t = 0; t < sizeof(lengths) / sizeof(lengths[0]); ++t) + { + uint8_t bytes[256]; + char text[512]; + int32_t n = lengths[t]; + int32_t text_len = encode_url(bytes, n, text, (int32_t)sizeof(text)); + + uint8_t out[256]; + az_span got = AZ_SPAN_EMPTY; + assert_int_equal(decode(text, false, out, (int32_t)sizeof(out), &got), AZ_IOT_OK); + assert_int_equal(az_span_size(got), n); + assert_memory_equal(az_span_ptr(got), bytes, (size_t)n); + + az_span in_place = az_span_create((uint8_t*)text, text_len); + assert_int_equal(az_iot_base64_decode(in_place, false, in_place, &got), AZ_IOT_OK); + assert_ptr_equal(az_span_ptr(got), (uint8_t*)text); + assert_int_equal(az_span_size(got), n); + assert_memory_equal(az_span_ptr(got), bytes, (size_t)n); + } +} + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(base64url_decodes_every_tail_length), + cmocka_unit_test(standard_base64_is_accepted_only_when_allowed), + cmocka_unit_test(malformed_input_is_rejected), + cmocka_unit_test(nonzero_trailing_bits_are_rejected), + cmocka_unit_test(destination_bounds_are_checked), + cmocka_unit_test(block_boundaries_decode_in_place), + }; + return cmocka_run_group_tests(tests, NULL, NULL); +} diff --git a/c/tests/unit/su_client_test.c b/c/tests/unit/su_client_test.c index 9a856c2cf..221ee39ec 100644 --- a/c/tests/unit/su_client_test.c +++ b/c/tests/unit/su_client_test.c @@ -270,6 +270,14 @@ typedef struct int download_calls; char download_file_ids[MAX_OPS][32]; char download_urls[MAX_OPS][64]; + + /* Key, exponent and signature of the last verify_rs256 call. */ + uint8_t verify_mod[8]; + size_t verify_mod_len; + uint8_t verify_exp[8]; + size_t verify_exp_len; + uint8_t verify_sig[8]; + size_t verify_sig_len; } hook_log; static void span_to_cstr(az_span s, char* out, size_t cap) @@ -391,15 +399,16 @@ static az_iot_result mock_verify_rs256( const uint8_t* sig, size_t sig_len) { - (void)mod; - (void)mod_len; - (void)exp; - (void)exp_len; (void)signed_data; (void)signed_len; - (void)sig; - (void)sig_len; hook_log* l = mock_log(self); + /* Truncated copies; the length is kept whole so a wrong decode still shows. */ + memcpy(l->verify_mod, mod, mod_len < sizeof(l->verify_mod) ? mod_len : sizeof(l->verify_mod)); + l->verify_mod_len = mod_len; + memcpy(l->verify_exp, exp, exp_len < sizeof(l->verify_exp) ? exp_len : sizeof(l->verify_exp)); + l->verify_exp_len = exp_len; + memcpy(l->verify_sig, sig, sig_len < sizeof(l->verify_sig) ? sig_len : sizeof(l->verify_sig)); + l->verify_sig_len = sig_len; log_op(l, OP_VERIFY, 0); return l->verify_result == AZ_IOT_SU_RESULT_SUCCESS ? AZ_IOT_OK : AZ_IOT_ERR_AUTH; } @@ -4659,6 +4668,20 @@ static const char* large_signature_patch(void) return patch; } +/* The last verify_rs256 (the manifest's) got the escaped standard-base64 + * modulus "\/\/\/\/" as FF FF FF, the exponent and the signature decoded. */ +static void assert_signing_key_decoded(const hook_log* l) +{ + static const uint8_t ff[] = { 0xFF, 0xFF, 0xFF }; + static const uint8_t sig[] = { 0xDE, 0xAD, 0xBE, 0xEF }; + assert_int_equal(l->verify_mod_len, sizeof(ff)); + assert_memory_equal(l->verify_mod, ff, sizeof(ff)); + assert_int_equal(l->verify_exp_len, sizeof(k_root_exp)); + assert_memory_equal(l->verify_exp, k_root_exp, sizeof(k_root_exp)); + assert_int_equal(l->verify_sig_len, sizeof(sig)); + assert_memory_equal(l->verify_sig, sig, sizeof(sig)); +} + static void large_signature_is_verified(void** state) { fixture* fx = (fixture*)*state; @@ -4668,12 +4691,15 @@ static void large_signature_is_verified(void** state) pump(fx, 40); assert_int_equal(az_iot_su_client_get_state(&fx->su), AZ_IOT_SU_STATE_IDLE); assert_int_equal(fx->chan.last_report.outcome, AZ_IOT_SU_OUTCOME_SUCCEEDED); + assert_signing_key_decoded(&fx->log); az_iot_su_client_update_request req; az_iot_su_client_update_manifest manifest; + memset(&fx->log.verify_mod, 0, sizeof(fx->log.verify_mod)); assert_int_equal( parse_with_roots(&fx->log, large_signature_patch(), k_root_keys, 1, &req, &manifest), AZ_IOT_OK); + assert_signing_key_decoded(&fx->log); } /* An offer near AZ_IOT_SU_REQUEST_BUFFER_SIZE whose size is almost all the @@ -4695,6 +4721,8 @@ static void near_limit_nested_signing_key_is_verified(void** state) pump(fx, 40); assert_int_equal(az_iot_su_client_get_state(&fx->su), AZ_IOT_SU_STATE_IDLE); assert_int_equal(fx->chan.last_report.outcome, AZ_IOT_SU_OUTCOME_SUCCEEDED); + assert_int_equal(fx->log.verify_mod_len, sizeof(k_root_mod)); + assert_memory_equal(fx->log.verify_mod, k_root_mod, sizeof(k_root_mod)); } /* A signature part that does not fit the scratch is logged as too large; a From 79249777361d454003733bf137e3356504903f2b Mon Sep 17 00:00:00 2001 From: ewertons <12401690+ewertons@users.noreply.github.com> Date: Fri, 9 Oct 2026 19:04:02 +0000 Subject: [PATCH 4/4] [C] Format base64 helper; count its test suite in the coverage sentinel --- .github/workflows/ci-c.yml | 2 +- c/src/core/internal/base64.h | 6 +----- c/tests/unit/base64_test.c | 5 +++-- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci-c.yml b/.github/workflows/ci-c.yml index 493357e90..e04406384 100644 --- a/.github/workflows/ci-c.yml +++ b/.github/workflows/ci-c.yml @@ -952,7 +952,7 @@ jobs: # actually had -- coverage computed over fewer tests than anyone thought. # Now that no suite decides at run time whether to participate, the count # is deterministic, so assert it exactly. Bump this when adding a suite. - EXPECTED_TESTS: '51' + EXPECTED_TESTS: '52' services: mosquitto: image: eclipse-mosquitto:2 diff --git a/c/src/core/internal/base64.h b/c/src/core/internal/base64.h index 00f5c3cbd..97121a000 100644 --- a/c/src/core/internal/base64.h +++ b/c/src/core/internal/base64.h @@ -42,11 +42,7 @@ extern "C" * alphabet, misplaced padding, a bad length or nonzero trailing bits; * AZ_IOT_ERR_NOT_ENOUGH_SPACE when @p out is too small. */ - az_iot_result az_iot_base64_decode( - az_span in, - bool allow_std, - az_span out, - az_span* out_decoded); + az_iot_result az_iot_base64_decode(az_span in, bool allow_std, az_span out, az_span* out_decoded); #ifdef __cplusplus } diff --git a/c/tests/unit/base64_test.c b/c/tests/unit/base64_test.c index 978c6aa43..6631177e0 100644 --- a/c/tests/unit/base64_test.c +++ b/c/tests/unit/base64_test.c @@ -124,8 +124,9 @@ static void destination_bounds_are_checked(void** state) assert_int_equal(decode("AQID", false, out, 3, &got), AZ_IOT_OK); assert_int_equal(decode("AQ", false, out, 1, &got), AZ_IOT_OK); assert_int_equal(decode("AQ==", true, out, 1, &got), AZ_IOT_OK); - assert_int_equal(az_iot_base64_decode(span_of("AQID"), false, AZ_SPAN_FROM_BUFFER(out), NULL), - AZ_IOT_ERR_INVALID_ARG); + assert_int_equal( + az_iot_base64_decode(span_of("AQID"), false, AZ_SPAN_FROM_BUFFER(out), NULL), + AZ_IOT_ERR_INVALID_ARG); } /* Lengths around the 64-character block, decoded into a separate buffer and in place. */