Skip to content

Commit 7e70e13

Browse files
committed
Address final pentest hardening findings
1 parent 4b44ef2 commit 7e70e13

9 files changed

Lines changed: 432 additions & 98 deletions

File tree

README.md

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -466,14 +466,16 @@ messages.
466466
`remaining_capacity()` for no-alloc sizing checks, redact the payload from
467467
`Debug`, clear their backing arrays when dropped as best-effort data-retention
468468
reduction, and provide explicit `constant_time_eq` helpers for equal-length
469-
values. They intentionally do not implement `PartialEq`/`==`: the helper is a
470-
dependency-free best-effort comparison, not a formal cryptographic token/MAC
471-
comparison primitive. Length mismatch returns immediately and must be treated
472-
as public protocol information. Applications that require a formally audited
473-
comparison should admit that dependency at the application boundary, for
474-
example by comparing exposed bytes with `subtle`. Do not use these helpers as
475-
the sole MAC, bearer-token, password-hash, or authentication-secret comparison
476-
primitive in high-assurance systems.
469+
reduction, and provide explicit equal-length comparison through
470+
`constant_time_eq_public_len`. They intentionally do not
471+
implement `PartialEq`/`==`: the helper is a dependency-free best-effort
472+
comparison, not a formal cryptographic token/MAC comparison primitive. Length
473+
mismatch returns immediately and must be treated as public protocol
474+
information. Applications that require a formally audited comparison should
475+
admit that dependency at the application boundary, for example by comparing
476+
exposed bytes with `subtle`. Do not use these helpers as the sole MAC,
477+
bearer-token, password-hash, or authentication-secret comparison primitive in
478+
high-assurance systems.
477479

478480
`into_exposed_array` is the explicit no-alloc ownership escape hatch for both
479481
stack-backed buffers. It returns the backing array and visible length, so
@@ -513,7 +515,7 @@ assert_eq!(format!("{encoded:?}"), r#"SecretBuffer { bytes: "<redacted>", len: 8
513515

514516
let decoded = STANDARD.decode_secret(encoded.expose_secret()).unwrap();
515517
assert_eq!(decoded.expose_secret(), b"hello");
516-
assert!(decoded.constant_time_eq(b"hello"));
518+
assert!(decoded.constant_time_eq_public_len(b"hello"));
517519
assert_eq!(format!("{decoded}"), "<redacted>");
518520

519521
let wrapped = STANDARD
@@ -537,12 +539,12 @@ assert_eq!(decoded.expose_secret(), b"hello");
537539
initialized bytes plus spare capacity when dropped. It does not claim formal
538540
zeroization and cannot clean historical copies outside the wrapper or make
539541
guarantees about allocator behavior. `SecretBuffer` intentionally does not
540-
implement `PartialEq`/`==`; use the explicit `constant_time_eq` helper only
541-
when its best-effort, public-length security contract is sufficient. Length
542-
mismatch returns immediately and must be treated as public protocol
543-
information. Applications that require a formally audited comparison should
544-
admit that dependency at the application boundary, for example by comparing
545-
exposed bytes with `subtle`.
542+
implement `PartialEq`/`==`; use the explicit
543+
`constant_time_eq_public_len` helper only when its best-effort, public-length
544+
security contract is sufficient. Length mismatch returns immediately and must
545+
be treated as public protocol information. Applications that require a
546+
formally audited comparison should admit that dependency at the application
547+
boundary, for example by comparing exposed bytes with `subtle`.
546548
On `wasm32`, the same compiler-fence-only wipe-barrier caveat applies to owned
547549
secret buffers. `wasm32` builds fail closed by default; enable
548550
`allow-wasm32-best-effort-wipe` only when the deployment explicitly accepts the

docs/API_AUDIT.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,8 @@ Decision rationale:
136136
- `into_exposed_array` is intentionally named as an ownership escape hatch
137137
where redaction and drop-time cleanup stop applying to the returned array.
138138
- Equality is intentionally not exposed through `PartialEq`/`==`. Callers must
139-
opt into the explicit `constant_time_eq` helper, whose equal-length scan is
140-
best-effort and whose length mismatch remains public.
139+
opt into the explicit `constant_time_eq_public_len` helper, whose equal-length
140+
scan is best-effort and whose length mismatch remains public.
141141

142142
Stable boundary:
143143

@@ -161,8 +161,8 @@ Decision rationale:
161161
- Drop-time cleanup uses the crate's volatile best-effort wipe helper for
162162
initialized bytes and vector spare capacity.
163163
- Equality is intentionally not exposed through `PartialEq`/`==`. Callers must
164-
opt into the explicit `constant_time_eq` helper, whose equal-length scan is
165-
best-effort and whose length mismatch remains public.
164+
opt into the explicit `constant_time_eq_public_len` helper, whose equal-length
165+
scan is best-effort and whose length mismatch remains public.
166166
- Strict standard padded `TryFrom` and `FromStr` implementations are kept only
167167
for native Rust ergonomics; non-standard profiles remain on explicit
168168
engine/profile methods.

docs/CONSTANT_TIME.md

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -262,10 +262,18 @@ partial plaintext before the final wipe.
262262

263263
Before reporting the opaque malformed-input result, the ct decoder passes the
264264
accumulated error mask through a non-inlined `ct_error_gate_barrier` that uses
265-
`core::hint::black_box` and a compiler fence. This is defense in depth against
266-
compiler reordering around the final public success/failure gate; it is not a
267-
hardware speculation barrier and does not change the transient-output window
268-
described above.
265+
`core::hint::black_box`, a compiler fence, and architecture-specific hardware
266+
speculation barriers where available (`lfence` on x86/x86_64, `isb sy` on ARM,
267+
and `isb sy; hint #20` on AArch64). This is defense in depth around the final
268+
public success/failure gate; it does not make the ct decoder a formally
269+
verified hardware side-channel resistant primitive and does not change the
270+
transient-output window described above.
271+
272+
For shared-memory or in-process sandbox threat models where even that transient
273+
output window is unacceptable, use
274+
`CtEngine::decode_slice_staged_clear_tail` with a private staging buffer. That
275+
API writes speculative decoded bytes into staging and copies into the caller's
276+
output only after validation succeeds.
269277

270278
The clear-tail APIs do not try to hide success, failure, or output length:
271279
those values are visible through the returned `Result` and decoded length. Any
@@ -280,13 +288,19 @@ cleanup steps before returning a protocol decision.
280288

281289
## Buffer Comparisons
282290

283-
`SecretBuffer::constant_time_eq`, `EncodedBuffer::constant_time_eq`, and
284-
`DecodedBuffer::constant_time_eq` provide dependency-free,
291+
`SecretBuffer::constant_time_eq_public_len`,
292+
`EncodedBuffer::constant_time_eq_public_len`, and
293+
`DecodedBuffer::constant_time_eq_public_len` provide dependency-free,
285294
constant-time-oriented comparison for equal-length buffers. These redacted
286295
buffer types intentionally do not implement `PartialEq`/`==`: the explicit
287296
method name is part of the security contract because this helper is best-effort
288297
and not a formal cryptographic comparison primitive.
289298

299+
The old `constant_time_eq` method name remains only as a deprecated migration
300+
alias during the `1.0.0-alpha` window. New code should use the
301+
`constant_time_eq_public_len` name so the public-length contract is visible at
302+
the call site.
303+
290304
Length mismatch returns immediately. Treat buffer length, the selected buffer
291305
type, and the final equality result as public. The helper scans every byte for
292306
equal-length inputs before returning. The per-byte difference is passed through

docs/SECURITY_CONTROLS.md

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,18 @@ The mapping is practical adoption guidance, not a certification claim.
1616
| CWE-226 Sensitive Information in Resource Not Removed Before Reuse | Caller-owned output buffers or crate-owned staging buffers retain partial sensitive data | Clear-tail APIs, stream cleanup, `EncodedBuffer`, `DecodedBuffer`, and `SecretBuffer` provide best-effort initialized-byte and spare-capacity cleanup. |
1717
| CWE-327 Use of Broken Or Risky Cryptographic Algorithm | Treating Base64 as encryption | Documentation describes Base64 as encoding, not encryption; secret wrappers are retention/logging helpers only. |
1818
| CWE-532 Insertion of Sensitive Information Into Log File | Accidentally logging sensitive encoded or decoded material | `SecretBuffer` redacts `Debug` and `Display` output and requires explicit reveal calls. |
19-
| CWE-208 Observable Timing Discrepancy | Sensitive comparison exits early on the first different byte | `SecretBuffer`, `EncodedBuffer`, and `DecodedBuffer` intentionally avoid `PartialEq`/`==` so best-effort comparison cannot be mistaken for a formal cryptographic primitive. Use explicit `constant_time_eq` for dependency-free equal-length scans; length mismatch returns immediately and is public. |
19+
| CWE-208 Observable Timing Discrepancy | Sensitive comparison exits early on the first different byte | `SecretBuffer`, `EncodedBuffer`, and `DecodedBuffer` intentionally avoid `PartialEq`/`==` so best-effort comparison cannot be mistaken for a formal cryptographic primitive. Use explicit `constant_time_eq_public_len` for dependency-free equal-length scans; length mismatch returns immediately and is public. |
2020
| CWE-829 Inclusion of Functionality From Untrusted Control Sphere | Runtime dependency compromise | Published crate has zero external runtime and default dev dependencies; dependency admission is documented and checked. |
2121

22-
The `constant_time_eq` helpers are dependency-free hardening aids, not audited
23-
MAC, bearer-token, password-hash, or authentication-secret comparison
24-
primitives. Their implementation is release-evidence-gated through generated
25-
assembly review, including LTO symbol-presence checks for
26-
`constant_time_eq_public_len`, but no formal cryptographic constant-time
27-
comparison guarantee is claimed. High-assurance applications that can admit a
28-
comparison dependency should use a reviewed primitive such as `subtle` at the
29-
protocol boundary.
22+
The `constant_time_eq_public_len` helpers are dependency-free hardening aids,
23+
not audited MAC, bearer-token, password-hash, or authentication-secret
24+
comparison primitives. Their implementation is release-evidence-gated through
25+
generated assembly review, including LTO symbol-presence checks for the helper,
26+
but no formal cryptographic constant-time comparison guarantee is claimed.
27+
High-assurance applications that can admit a comparison dependency should use a
28+
reviewed primitive such as `subtle` at the protocol boundary. The shorter
29+
`constant_time_eq` name remains only as a deprecated migration alias during the
30+
`1.0.0-alpha` window.
3031

3132
## Caller Responsibilities
3233

@@ -109,9 +110,10 @@ reduces post-return retention, but it is not an isolation boundary: code running
109110
in the same process with concurrent or unsafe access to the output buffer during
110111
the decode call could observe transient partial plaintext before the final wipe.
111112
Before the opaque malformed-input result is reported, the accumulated ct error
112-
mask passes through a non-inlined compiler barrier. This is defense in depth
113-
against compiler reordering around the public success/failure gate, not a
114-
hardware speculation barrier.
113+
mask passes through a non-inlined compiler barrier plus architecture-specific
114+
hardware speculation barriers where available. Shared-memory deployments that
115+
cannot tolerate transient writes to the caller output should use
116+
`CtEngine::decode_slice_staged_clear_tail` with a private staging buffer.
115117
For constant-time-oriented in-place decode, use
116118
`ct::CtEngine::decode_in_place_clear_tail`. The non-clear-tail CT in-place API
117119
was removed before the `1.0` stable boundary because it could partially destroy

docs/UNSAFE.md

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,20 @@
22

33
`base64-ng` keeps scalar encode/decode in safe Rust. The crate root uses
44
`#![deny(unsafe_code)]`, and reviewed `allow(unsafe_code)` exceptions are
5-
limited to volatile wipe helpers in `src/lib.rs` and the SIMD boundary in
6-
`src/simd.rs`.
5+
limited to volatile wipe helpers, the constant-time error gate barrier in
6+
`src/lib.rs`, and the SIMD boundary in `src/simd.rs`.
77

88
This inventory is intentionally small and release-gate enforced. Any new unsafe
99
block must be added here before an accelerated backend can be admitted.
1010

1111
## Policy
1212

13-
- Default builds compile audited unsafe volatile wipe helpers.
13+
- Default builds compile audited unsafe volatile wipe helpers and the
14+
constant-time error gate barrier.
1415
- Optional SIMD prototypes live only in `src/simd.rs`.
1516
- `scripts/validate-unsafe-boundary.sh` fails if `allow(unsafe_code)` appears
16-
outside the volatile wipe helpers or `src/simd.rs`.
17+
outside the volatile wipe helpers, the constant-time error gate barrier, or
18+
`src/simd.rs`.
1719
- `scripts/validate-unsafe-boundary.sh` fails if architecture intrinsics, CPU
1820
feature detection, or `target_feature` gates appear outside `src/simd.rs`.
1921
- Every unsafe function and unsafe block must have a local safety explanation.
@@ -110,9 +112,9 @@ Preconditions:
110112
Unsafe operation:
111113

112114
- `core::arch::asm!` emits `mfence` on non-Miri `x86`/`x86_64`,
113-
`dsb sy; isb sy` on non-Miri `arm`/`aarch64`, and `fence rw, rw` on non-Miri
114-
`riscv32`/`riscv64`. The pointer and length are also passed as opaque
115-
operands.
115+
`dsb sy; isb sy` on non-Miri `arm`, `dsb sy; isb sy; hint #20` on non-Miri
116+
`aarch64`, and `fence rw, rw` on non-Miri `riscv32`/`riscv64`. The pointer
117+
and length are also passed as opaque operands.
116118

117119
Safety argument:
118120

@@ -139,6 +141,43 @@ Limitations:
139141
enabled after reviewing this weaker cleanup posture and applying platform
140142
memory controls.
141143

144+
### `ct_error_gate_barrier`
145+
146+
Location: `src/lib.rs`
147+
148+
Status: active constant-time error-gate hardening primitive.
149+
150+
Purpose:
151+
152+
- Keep the accumulated constant-time decoder malformed-input mask visible
153+
across a non-inlined boundary before the public success/failure branch.
154+
- Emit an architecture-specific speculation barrier where stable Rust supports
155+
one locally.
156+
157+
Preconditions:
158+
159+
- Caller passes accumulated public error-mask bytes.
160+
161+
Unsafe operation:
162+
163+
- `core::arch::asm!` emits `lfence` on non-Miri `x86`/`x86_64`, `isb sy` on
164+
non-Miri `arm`, and `isb sy; hint #20` on non-Miri `aarch64`.
165+
166+
Safety argument:
167+
168+
- The assembly blocks do not access memory.
169+
- `options(nostack, preserves_flags)` states that the blocks do not use the
170+
stack or modify flags. The x86/x86_64 block also uses `nomem`.
171+
- The helper does not read or write through any pointer and cannot violate
172+
Rust aliasing or bounds rules.
173+
174+
Limitations:
175+
176+
- This is defense in depth against speculation around the final public
177+
malformed-input result. It does not make the ct decoder a formally verified
178+
hardware side-channel resistant primitive.
179+
- Unsupported architectures fall back to the compiler fence only.
180+
142181
### `wipe_vec_spare_capacity`
143182

144183
Location: `src/lib.rs`

scripts/validate-unsafe-boundary.sh

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,24 +20,24 @@ if [ "$matches" != "$allowed" ]; then
2020
fi
2121

2222
root_allow_count="$(grep -c '^#\[allow(unsafe_code)\]$' "$root_allowed" || true)"
23-
if [ "$root_allow_count" -ne 3 ]; then
24-
echo "unsafe boundary: src/lib.rs must have exactly three reviewed allow(unsafe_code) cleanup helpers"
23+
if [ "$root_allow_count" -ne 4 ]; then
24+
echo "unsafe boundary: src/lib.rs must have exactly four reviewed allow(unsafe_code) helpers"
2525
exit 1
2626
fi
2727

2828
if ! awk '
2929
/^#\[allow\(unsafe_code\)\]$/ {
3030
allow_line = NR
3131
}
32-
/^fn wipe_bytes\(/ || /^fn wipe_barrier\(/ || /^fn wipe_vec_spare_capacity\(/ {
32+
/^fn wipe_bytes\(/ || /^fn wipe_barrier\(/ || /^fn wipe_vec_spare_capacity\(/ || /^fn ct_error_gate_barrier\(/ {
3333
if (allow_line != NR - 1) {
3434
failed = 1
3535
}
3636
seen += 1
3737
}
38-
END { exit failed || seen != 3 }
38+
END { exit failed || seen != 4 }
3939
' "$root_allowed"; then
40-
echo "unsafe boundary: src/lib.rs allow(unsafe_code) must apply only to reviewed cleanup helpers"
40+
echo "unsafe boundary: src/lib.rs allow(unsafe_code) must apply only to reviewed cleanup and CT gate helpers"
4141
exit 1
4242
fi
4343

@@ -62,7 +62,7 @@ if [ ! -s docs/UNSAFE.md ]; then
6262
exit 1
6363
fi
6464

65-
unsafe_functions="$(sed -n 's/^[[:space:]]*pub(super)[[:space:]]*unsafe[[:space:]]*fn[[:space:]]*\([A-Za-z0-9_][A-Za-z0-9_]*\).*/\1/p' "$simd_allowed")"
65+
unsafe_functions="$(sed -n 's/^[[:space:]]*unsafe[[:space:]]*fn[[:space:]]*\([A-Za-z0-9_][A-Za-z0-9_]*\).*/\1/p' "$simd_allowed")"
6666

6767
if [ -z "$unsafe_functions" ]; then
6868
echo "unsafe boundary: expected documented prototype unsafe functions in $allowed"

0 commit comments

Comments
 (0)