diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c4e1ba4..45abe02 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,6 +74,7 @@ jobs: tests/test_block_type_confusion.sh \ tests/test_disk_device_capacity.sh \ tests/test_f09_preface.sh \ + tests/test_key_files.sh \ tests/test_legacy_disk_5_2_1.sh \ tests/test_path_traversal.sh \ tests/test_pqbox.sh \ diff --git a/AUDIT.md b/AUDIT.md index 7f46f35..dfe887f 100644 --- a/AUDIT.md +++ b/AUDIT.md @@ -225,6 +225,14 @@ regression injects a directory symlink and verifies that its external sentinel survives. These are reviewed fixes and regression coverage, not independent certification or proof that a 5.2.8 hosted gate passed. +The C/C++ default-branch analysis of commit `69fc26b` closed #5, #6, and #7, +then reported High #8, #9, and #10 solely in the newly added SDK regression: +its sentinel and mode checks used `stat`/`lstat` before later path operations. +Each individual content or metadata check now opens without following links +and uses `fstat` or reads through that already-open descriptor. A static guard +rejects a return to path-level `stat`/`lstat` in this test. A fresh +default-branch scan remains the authoritative closure evidence. + The exact 5.2.8 candidate must repeat the required suite. Native Windows and macOS gates, hosted GitHub CI and release promotion, authenticated OBS validation, and resolution of the openSUSE automatic `debugsource` rpmlint @@ -270,6 +278,7 @@ should be rerun, but the historical resolution does not itself constitute a | 5.2.8 | High | CodeQL #5: SDK key copies changed permissions through a re-resolved destination path | Publish through the core atomic output object and apply permissions to its open descriptor; run link-target/mode regressions through `sdk-test` | | 5.2.8 | High | CodeQL #6: POSIX disk restore classified a pathname before reopening it destructively | Open without truncation or symlink following, classify with `fstat`, and retain the same device descriptor through write | | 5.2.8 | High | CodeQL #7: benchmark cleanup classified entries before recursively resolving their path | Traverse pinned descriptors/handles, refuse link/reparse traversal, remove entries relative to pinned parents, and verify Windows directory identity before handle deletion | +| 5.2.8 | High (test-only) | CodeQL #8/#9/#10: the new SDK regression inspected paths before later reads or cleanup | Open each fixture without following links, inspect/read through `fstat` and the same descriptor, and reject path-level `stat`/`lstat` in the static gate | See `CHANGELOG.md` for the complete per-release history and compatibility notes. Old tags remain immutable and may contain artifacts or build assumptions removed diff --git a/CHANGELOG.md b/CHANGELOG.md index c6cdbef..b36e681 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,12 +26,23 @@ unchanged. `unlinkat`, and using pinned, reparse-point-aware handles for Windows traversal. The regression injects a directory symlink into a live workspace and verifies that cleanup does not visit its target. +- The first default-branch rescan closed #5, #6, and #7 and then identified + test-only path checks as High #8, #9, and #10 in the new SDK regression. + Replace every test-side `stat`/`lstat` sequence with one no-follow `open` + followed by `fstat` and descriptor reads, and retain that boundary in the + static regression gate. - Treat inability to create the raw-C1 scanner filename as an explicit fixture skip on filesystems that reject the byte; when creation succeeds, the unsafe - diagnostic-escaping assertions still run unchanged. + diagnostic-escaping assertions still run unchanged. Normalize Bash 3.2's + sign-extended `%d` character conversion to an unsigned octet so raw and + UTF-8 C1 diagnostics retain their canonical `\\xNN`/`\\uNNNN` form. - Reject redirected or otherwise non-console Windows password prompts before entering `_getch`, handle console EOF as an error, and cover the native redirected-input path so it cannot hang a release gate. +- On Windows, validate private-key confinement as the protected, + current-user-only DACL that the implementation creates; do not treat MSYS's + synthetic `stat` mode as a POSIX `0600` result. POSIX continues to require + the real descriptor mode `0600` under multiple umasks. - Run `sdk-test` from both `release-check` and the hosted GCC/Clang Linux job so the atomic key-save regression cannot silently fall outside release gates. - Carry the v5.2.7 archive format, cryptography, bundled codec release, and SDK diff --git a/README.md b/README.md index 252a1c4..a845804 100644 --- a/README.md +++ b/README.md @@ -10,9 +10,15 @@ publish atomically through an already-open private object, POSIX disk restore classifies and retains the descriptor it actually opened, and benchmark cleanup traverses only pinned descriptors or handles without following links or Windows reparse points. It also makes the raw-C1 scanner fixture explicitly skip a -filesystem that rejects creation with `EILSEQ`, and brings `sdk-test` into the +filesystem that rejects creation with `EILSEQ`, normalizes Bash 3.2 signed-byte +diagnostics, and brings `sdk-test` into the release and hosted Linux gates. Windows password prompts now reject redirected -input before entering `_getch` and treat console EOF as an error. These +input before entering `_getch` and treat console EOF as an error; its key-file +regression validates the protected current-user-only DACL rather than an MSYS +POSIX-mode projection. The C/C++ default-branch scan of commit `69fc26b` +closed #5, #6, and #7 and exposed test-only #8, #9, and #10 in the new SDK +regression. Their follow-up uses no-follow descriptors plus `fstat`/descriptor +reads and requires a fresh default-branch scan for closure. These corrections do not change archive format v1.6, cryptography, the bundled codec release, or the SDK ABI. @@ -36,7 +42,8 @@ opens a POSIX target once before its type, identity, and device-capacity decisions, and benchmark cleanup is descriptor-relative on POSIX and handle/reparse-point aware on Windows. The live-workspace symlink regression, SDK link-target/mode regression, static path-race guards, portable raw-C1 -fixture, native redirected-prompt regression, and `sdk-test` CI step cover +fixture with Bash 3.2 unsigned-byte normalization, native redirected-prompt +and protected-DACL regressions, and `sdk-test` CI step cover these boundaries. All current release paths move to 5.2.8 and require fresh exact-tag hosted CI, package, native-platform, source-only, checksum, OBS, and promotion evidence. diff --git a/SECURITY.md b/SECURITY.md index e227e39..44c52ff 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -228,6 +228,13 @@ publication, #6 at POSIX disk-target classification/use, and #7 at benchmark workspace cleanup. The regressions and source review are project evidence, not an independent certification or a claim that exact-v5.2.8 CI has passed. +The C/C++ default-branch analysis of commit `69fc26b` closed #5, #6, and #7, +then opened test-only High #8, #9, and #10 because the new SDK regression used +path-level `stat`/`lstat` before later reads or cleanup. Each individual test +check now uses a no-follow descriptor with `fstat` or descriptor reads; the +static gate rejects reintroduction of path-level metadata checks there. A +subsequent default-branch scan is required as closure evidence. + The Windows handle-relative implementation is scoped to normal local Win32 paths. Win32 extended-length and device-namespace paths, raw UNC output roots, and mapped/network-drive output are not supported in 5.2.8. Cross-build and diff --git a/scripts/check-source-only.sh b/scripts/check-source-only.sh index 723cb4c..62c933f 100755 --- a/scripts/check-source-only.sh +++ b/scripts/check-source-only.sh @@ -165,6 +165,10 @@ safe_path_for_output() { for ((index = 0; index < length; index++)); do character=${path:index:1} printf -v byte '%d' "'$character" + # Bash 3.2 can sign-extend bytes >= 0x80 when converting a character + # with %d. Normalize to an unsigned octet before UTF-8 validation and + # diagnostic escaping. + byte=$((byte & 0xff)) if ((byte < 0x20 || byte == 0x7f)); then printf -v escaped '\\x%02x' "$byte" @@ -185,6 +189,7 @@ safe_path_for_output() { if ((byte >= 0xc2 && byte <= 0xdf && index + 1 < length)); then character=${path:index+1:1} printf -v byte2 '%d' "'$character" + byte2=$((byte2 & 0xff)) if ((byte2 >= 0x80 && byte2 <= 0xbf)); then codepoint=$(((byte & 0x1f) << 6 | (byte2 & 0x3f))) sequence=${path:index:2} @@ -192,8 +197,10 @@ safe_path_for_output() { elif ((byte >= 0xe0 && byte <= 0xef && index + 2 < length)); then character=${path:index+1:1} printf -v byte2 '%d' "'$character" + byte2=$((byte2 & 0xff)) character=${path:index+2:1} printf -v byte3 '%d' "'$character" + byte3=$((byte3 & 0xff)) if ((byte3 >= 0x80 && byte3 <= 0xbf && ((byte == 0xe0 && byte2 >= 0xa0 && byte2 <= 0xbf) || (byte >= 0xe1 && byte <= 0xec && byte2 >= 0x80 && byte2 <= 0xbf) || @@ -206,10 +213,13 @@ safe_path_for_output() { elif ((byte >= 0xf0 && byte <= 0xf4 && index + 3 < length)); then character=${path:index+1:1} printf -v byte2 '%d' "'$character" + byte2=$((byte2 & 0xff)) character=${path:index+2:1} printf -v byte3 '%d' "'$character" + byte3=$((byte3 & 0xff)) character=${path:index+3:1} printf -v byte4 '%d' "'$character" + byte4=$((byte4 & 0xff)) if ((byte3 >= 0x80 && byte3 <= 0xbf && byte4 >= 0x80 && byte4 <= 0xbf && ((byte == 0xf0 && byte2 >= 0x90 && byte2 <= 0xbf) || diff --git a/sdk/tests/test_sdk_roundtrip.c b/sdk/tests/test_sdk_roundtrip.c index 8ef87a2..f855bde 100644 --- a/sdk/tests/test_sdk_roundtrip.c +++ b/sdk/tests/test_sdk_roundtrip.c @@ -13,6 +13,7 @@ #include #include #ifndef _WIN32 +#include #include #endif #include @@ -55,18 +56,35 @@ static int file_matches(const char *path, const void *expected, size_t expected_size) { struct stat info; char observed[128]; - if (expected_size > sizeof(observed) || stat(path, &info) != 0 || - info.st_size < 0 || (uint64_t)info.st_size != (uint64_t)expected_size) + if (expected_size > sizeof(observed)) return 0; - FILE *stream = fopen(path, "rb"); - if (!stream) return 0; - size_t got = fread(observed, 1, expected_size, stream); - int read_error = ferror(stream); - int close_rc = fclose(stream); - return got == expected_size && !read_error && close_rc == 0 && + int fd = open(path, O_RDONLY | O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK); + if (fd < 0) return 0; + int ok = fstat(fd, &info) == 0 && S_ISREG(info.st_mode) && + info.st_size >= 0 && + (uint64_t)info.st_size == (uint64_t)expected_size; + size_t got = 0; + while (ok && got < expected_size) { + ssize_t count = read(fd, observed + got, expected_size - got); + if (count <= 0) { + ok = 0; + break; + } + got += (size_t)count; + } + if (close(fd) != 0) ok = 0; + return ok && got == expected_size && memcmp(observed, expected, expected_size) == 0; } +static int regular_file_info(const char *path, struct stat *info) { + int fd = open(path, O_RDONLY | O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK); + if (fd < 0) return 0; + int ok = fstat(fd, info) == 0 && S_ISREG(info->st_mode); + if (close(fd) != 0) ok = 0; + return ok; +} + static int private_key_save_avoids_link_targets(const zuptsdk_keypair_t *kp) { static const char sentinel[] = "do not replace through a symlink\n"; char workspace[] = "/tmp/zupt-sdk-link-save.XXXXXX"; @@ -95,20 +113,22 @@ static int private_key_save_avoids_link_targets(const zuptsdk_keypair_t *kp) { if (symlink(target, symlink_path) != 0 || zuptsdk_keypair_save_private(kp, symlink_path) != ZUPTSDK_OK || !file_matches(target, sentinel, sizeof(sentinel) - 1) || - stat(target, &target_st) != 0 || lstat(symlink_path, &output_st) != 0 || + !regular_file_info(target, &target_st) || + !regular_file_info(symlink_path, &output_st) || (target_st.st_dev == output_st.st_dev && target_st.st_ino == output_st.st_ino) || - !S_ISREG(output_st.st_mode) || output_st.st_size <= 0 || + output_st.st_size <= 0 || (output_st.st_mode & 0777) != 0600) goto cleanup; if (link(target, hardlink_path) != 0 || zuptsdk_keypair_save_private(kp, hardlink_path) != ZUPTSDK_OK || !file_matches(target, sentinel, sizeof(sentinel) - 1) || - stat(target, &target_st) != 0 || stat(hardlink_path, &output_st) != 0 || + !regular_file_info(target, &target_st) || + !regular_file_info(hardlink_path, &output_st) || (target_st.st_dev == output_st.st_dev && target_st.st_ino == output_st.st_ino) || - !S_ISREG(output_st.st_mode) || output_st.st_size <= 0 || + output_st.st_size <= 0 || (output_st.st_mode & 0777) != 0600) goto cleanup; @@ -328,12 +348,16 @@ static void test_keypair_pq(void) { TEST("keypair_generate + compress_pq + extract_pq"); char saved_priv[160]; char saved_pub[160]; +#ifdef _WIN32 snprintf(saved_priv, sizeof(saved_priv), "/tmp/_zsdk_priv_%ld.key", (long)getpid()); snprintf(saved_pub, sizeof(saved_pub), "/tmp/_zsdk_pub_%ld.key", (long)getpid()); unlink(saved_priv); unlink(saved_pub); +#else + char saved_workspace[] = "/tmp/zupt-sdk-roundtrip.XXXXXX"; +#endif zuptsdk_ctx_t *ctx = NULL; CHECK(zuptsdk_ctx_create(&ctx), "ctx"); @@ -343,6 +367,16 @@ static void test_keypair_pq(void) { if (rc != ZUPTSDK_OK) { FAIL("keygen"); zuptsdk_ctx_destroy(ctx); return; } #ifndef _WIN32 + if (!mkdtemp(saved_workspace)) { + FAIL("private temporary workspace"); + zuptsdk_keypair_destroy(kp); + zuptsdk_ctx_destroy(ctx); + return; + } + snprintf(saved_priv, sizeof(saved_priv), "%s/private.key", + saved_workspace); + snprintf(saved_pub, sizeof(saved_pub), "%s/public.key", + saved_workspace); if (!private_key_save_avoids_link_targets(kp)) { FAIL("private key save followed a symlink or hardlink target"); goto err; @@ -357,8 +391,8 @@ static void test_keypair_pq(void) { #ifndef _WIN32 struct stat private_st; struct stat public_st; - if (stat(saved_priv, &private_st) != 0 || - stat(saved_pub, &public_st) != 0 || + if (!regular_file_info(saved_priv, &private_st) || + !regular_file_info(saved_pub, &public_st) || (private_st.st_mode & 0777) != 0600 || (public_st.st_mode & 0777) != 0644) { FAIL("saved key permissions do not match the requested modes"); @@ -398,8 +432,10 @@ static void test_keypair_pq(void) { zuptsdk_privkey_destroy(priv); zuptsdk_options_destroy(opts); - unlink(saved_priv); - unlink(saved_pub); + if (unlink(saved_priv) != 0 || unlink(saved_pub) != 0) ok = 0; +#ifndef _WIN32 + if (rmdir(saved_workspace) != 0) ok = 0; +#endif if (!ok) { FAIL("byte mismatch or rc != OK"); zuptsdk_keypair_destroy(kp); zuptsdk_ctx_destroy(ctx); return; } zuptsdk_keypair_destroy(kp); @@ -410,6 +446,9 @@ static void test_keypair_pq(void) { err: unlink(saved_priv); unlink(saved_pub); +#ifndef _WIN32 + rmdir(saved_workspace); +#endif zuptsdk_keypair_destroy(kp); zuptsdk_ctx_destroy(ctx); } diff --git a/tests/test_key_files.sh b/tests/test_key_files.sh index ec4be02..8c232ff 100644 --- a/tests/test_key_files.sh +++ b/tests/test_key_files.sh @@ -41,17 +41,76 @@ file_mode() { fi } +windows_private_acl() { + local output=$1 windows_path + command -v cygpath >/dev/null 2>&1 || return 1 + command -v powershell.exe >/dev/null 2>&1 || return 1 + windows_path=$(cygpath -aw -- "$output") || return 1 + # PowerShell variables must remain literal until powershell.exe evaluates + # this single-quoted Bash argument. + # shellcheck disable=SC2016 + ZUPT_KEY_ACL_PATH=$windows_path powershell.exe -NoLogo -NoProfile \ + -NonInteractive -Command ' + $ErrorActionPreference = "Stop" + $acl = Get-Acl -LiteralPath $env:ZUPT_KEY_ACL_PATH + $sidType = [System.Security.Principal.SecurityIdentifier] + $rules = @($acl.GetAccessRules($true, $true, $sidType)) + $currentSid = + [System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value + if (-not $acl.AreAccessRulesProtected) { + throw "private-key DACL permits inheritance" + } + if ($rules.Count -ne 1) { + throw "private-key DACL does not contain exactly one ACE" + } + $rule = $rules[0] + if ($rule.IsInherited) { + throw "private-key ACE is inherited" + } + if ($rule.AccessControlType -ne + [System.Security.AccessControl.AccessControlType]::Allow) { + throw "private-key ACE is not an allow rule" + } + if ($rule.IdentityReference.Value -ne $currentSid) { + throw "private-key ACE is not restricted to the current user" + } + if ($rule.InheritanceFlags -ne + [System.Security.AccessControl.InheritanceFlags]::None -or + $rule.PropagationFlags -ne + [System.Security.AccessControl.PropagationFlags]::None) { + throw "private-key ACE unexpectedly propagates" + } + $fullControl = [int64]( + [System.Security.AccessControl.FileSystemRights]::FullControl) + $actualRights = [int64]($rule.FileSystemRights) + if (($actualRights -band $fullControl) -ne $fullControl) { + throw "private-key ACE does not grant current-user full control" + } + ' /dev/null +} + generate_with_mode() { local label=$1 mask=$2 output=$3 shift 3 if (umask "$mask"; "$zupt_bin" keygen "$@" -o "$output" >/dev/null 2>&1); then - local mode - mode=$(file_mode "$output") - if [[ $mode == 600 ]]; then - pass "$label is mode 0600 under umask $mask" - else - fail "$label mode under umask $mask is $mode, expected 600" - fi + case $(uname -s 2>/dev/null || printf unknown) in + MINGW*|MSYS*|CYGWIN*) + if windows_private_acl "$output"; then + pass "$label has a protected current-user-only DACL under umask $mask" + else + fail "$label lacks a protected current-user-only DACL under umask $mask" + fi + ;; + *) + local mode + mode=$(file_mode "$output") + if [[ $mode == 600 ]]; then + pass "$label is mode 0600 under umask $mask" + else + fail "$label mode under umask $mask is $mode, expected 600" + fi + ;; + esac else fail "$label generation failed under umask $mask" fi diff --git a/tests/test_static_analysis.sh b/tests/test_static_analysis.sh index 690898e..4c13747 100755 --- a/tests/test_static_analysis.sh +++ b/tests/test_static_analysis.sh @@ -212,6 +212,19 @@ else F "SDK key save atomic publication guard is missing" fi +# The SDK regression must not recreate the same check/use pattern while +# inspecting its sentinels and key modes. Open once, then classify/read via +# that descriptor; this also keeps CodeQL evidence free of test-only races. +if grep -Eq '(^|[^[:alnum:]_])(stat|lstat)[[:space:]]*\(' \ + sdk/tests/test_sdk_roundtrip.c; then + F "SDK regression uses path-level stat/lstat before later path operations" +elif grep -Fq 'fstat(fd, &info)' sdk/tests/test_sdk_roundtrip.c && + grep -Fq 'fstat(fd, info)' sdk/tests/test_sdk_roundtrip.c; then + P "SDK regression inspects already-open file descriptors" +else + F "SDK regression descriptor-based inspection guard is missing" +fi + echo "" echo " ───────────────────────────────────────" echo " Static analysis: $PASS passed, $FAIL failed"