feat: Extend ct.rs Condition to unsigned widths -- RSA groundwork

[laruizlo]

Extend ct.rs Condition to unsigned widths (u64 parity, new u32)

Why

Groundwork for the upcoming RSA implementation. The RSA bigint layer uses u64 limbs on 64-bit targets and u32 limbs elsewhere, and needs identical constant-time mask constructors for both widths so the dual-width code is written once. Before this PR Condition support was uneven: Condition<i64> had the full constructor set, Condition<u64> had only from_bool/select/is_true, and Condition<u32> did not exist.

This PR is deliberately separate from the RSA work so the shared security-critical utils code gets its own review, and the RSA PRs stay pure-RSA.

What

The PR has grown through review; the final state is:

• Four fully-populated widths. Condition<i64>, <i32>, <u64> and <u32>, each registered in the sealed SupportedMaskType scheme (registration alone gives every width the generic boolean operator impls & | ^ ! and their assign forms).
• The unsigned constructor set, identical at u64 and u32 where meaning is width-independent:
  • TRUE / FALSE
  • from_bool, from_bool_var: mask from a boolean (wrapping-sub construction)
  • from_lsb: mask from bit 0 (parity/oddness tests)
  • from_msb: mask from the top bit (adaptor for borrow/carry words coming out of wrapping subtraction chains: from_msb(borrow) is the lt mask, no post-processing)
  • is_bit_set, is_zero, is_not_zero, is_equal
  • select, mov, swap, to_bool
  • all const fn except mov
• The signed set at full i64/i32 parity, including a review-driven correctness fix: is_lt now uses the standard overflow-free signed-comparison identity. The previous is_negative(x - y) construction overflowed for operands more than half the range apart (debug panic, opposite answer in release). With overflow gone, the whole comparison family (is_lt, is_lte, is_gt, is_gte, is_within_range) is const fn.
• Every impl is written out by hand, one per width; ct.rs contains no macro_rules!. Earlier revisions macro-generated the per-width impls, but cargo mutants parses with syn and cannot see into macro bodies, so the macro form hid every mask identity from mutation testing: the Condition constructors produced 0 mutants. Per review feedback the widths are now individually defined, taking the package census from 75 to 301 mutants (226 on the constructors). See Verification below for the run results.
• The cost of that choice is drift between the copies, and it is a known, accepted trade: the four impls land byte-identical modulo the width token (produced by mechanical expansion of the former macros). The per-width tests differential every width against the native operators, so a functional regression applied to one width fails on any covered input; what no test catches is a method added to one width and not the other, a functionally equal but non-constant-time rewrite of a single width, or drift outside the tested boundary sets. Header comments and a CLAUDE.md note state that the widths in a group must be edited together. A small mechanical parity check (normalise the impls, fail on divergence) is planned as its own follow-up PR rather than growing this one.

IMPORTANT / API changes relative to the pre-PR Condition<u64>:

• Commit 0202982 removes Condition<u64>::is_true(). It duplicated the boolean accessor under a second name and was the API's only &self receiver. A workspace-wide search found no production callers (its only uses were this crate's own tests, migrated in the same commit). The removal is deliberately isolated in its own commit: if reviewers prefer to keep is_true, dropping that single commit restores it without affecting the rest of the PR.
• Commit bfc0b82 renames to_bool_var() to to_bool() on all widths. The _var suffix on from_bool_var distinguishes the runtime form from the const-generic from_bool::<VALUE>(); there is no const-generic counterpart on the output side, so the suffix distinguished nothing. No production callers; out-of-tree or in-flight code calling is_true() or to_bool_var() must switch to to_bool().

Design notes for review

• The unsigned constructions do NOT reuse the signed shapes. The signed constructors rely on two's-complement sign reasoning (is_negative via arithmetic shift, the sign-based is_lt identity) that has no meaning for full-range unsigned values. The unsigned versions use the standard mask identities: wrapping_sub from a 0/1 bit, and MSB-extraction of x | x.wrapping_neg() for the zero test.
• Ordering comparisons are deliberately omitted from the unsigned set. Multi-word callers (the RSA bigint) derive lt from their subtraction borrow chain and convert with from_msb; a word-level unsigned is_lt would invite exactly the overflow-style misuse the signed identity has to defend against.
• Constructors are const fn throughout; black_box hygiene stays where it already lives (the byte-level helpers), since black_box is not const-compatible and the mask constructors don't use it on any width.
• Six mutation survivors are OR/XOR equivalences by construction: in is_lt the two operands fire on disjoint sign cases, and in select the mask and its complement never both pass a bit, so | and ^ compute the same function and no test can separate them. This is the survivor class QUALITY_AND_STYLE.md names as acceptable. The | spelling is nevertheless the contract: because tests cannot distinguish the two, they also cannot catch a functionally-equal but non-constant-time rewrite of a single width, so the mask-based shapes must be preserved exactly.

Tests

One hand-written test module per width, mirroring the impls they exercise (the test macros were expanded for the same consistency reasons; .cargo/mutants.toml excludes tests/**, so no mutation coverage was at stake):

• Boundary coverage for every constructor: 0, 1, MAX, MAX-1, 1 << (BITS-1) unsigned and 0, +/-1, MIN, MIN+1, MAX, MAX-1 signed, the values where leaked cross-signedness reasoning or overflow would produce wrong masks.
• Mask-canonicality assert on every constructor result: select between a pattern and its complement, which differ in every bit, must return one of them exactly. This proves the mask is all-ones/all-zeros through the public API and kills the wrong-mask mutants a plain truthiness check passes (e.g. a constructor returning 1 instead of -1).
• Differential checks against the native operators (<, <=, ==, ranges) at every width over all boundary pairs, plus a far-apart-operand regression for the old is_lt overflow and a dense strided sweep of the full i32 range.
• A borrow-adaptor cross-check: from_msb of a widening-subtraction borrow word agrees with the < operator over all boundary pairs.
• The legacy i64_tests/u64_tests modules are folded into the per-width modules; deleting them was measured as a zero change in the mutation outcome. ct_tests runs 66 tests; the bouncycastle-utils package runs 83.

Verified additionally against a dual-width bigint prototype implementing the RSA phase-1 predicate/conditional set (ct_is_zero, ct_eq, ct_lt via sbb chain, is_odd, bit(i), select/assign/swap, conditional modular correction, masked table scan) — compiles and passes written-once against Condition<Word> in both the native-u64 and forced-u32 lanes.

Consumers

hex and base64 are the only production consumers of Condition masks (all via the signed is_within_range/is_in_list paths, untouched). The byte-level helpers (ct_eq_bytes, ct_eq_zero_bytes, conditional_copy_bytes) used by core, hmac, mlkem, mldsa and the lowmemory variants are behaviourally untouched.

Pre-push verification (final tree):

• cargo test --workspace: 71 suites ok, 0 failures
• cargo fmt --all --check clean; cargo doc -p bouncycastle-utils --no-deps clean under #![forbid(missing_docs)]
• cargo mutants -p bouncycastle-utils -j 2 --timeout 20: 301 mutants: 220 caught, 15 missed, 66 unviable (macro-form baseline: 75 mutants, constructors invisible). The 15 missed:
  • 6 OR/XOR equivalences in is_lt (i64, i32) and select (all four widths), triaged above; new only in the sense that they were previously invisible
  • 8 pre-existing OR/XOR equivalences in conditional_copy_bytes (disjoint masked operands), unchanged from before this PR
  • 1 pre-existing test gap: Secret::drop (the suite exercises explicit zeroize() but never observes drop-time scrubbing), unchanged from before this PR
  • the 66 unviable are Default::default() replacements on constructors; Condition has no Default
• Quality stats: unwrap/Err() counts flat (491/260 core-code baseline)

Co-authored with Claude Code

[ounsworth]

This is great groundwork!

The Condition class was clearly in need of some attention. The section "Observed during review, out of scope for this PR" lists things that are all good changes / cleanup. If you are willing to tackle those as part of this PR, that would be good improvements.

I also left a few comments below.

[ounsworth]

Claude Code likes to make these sorts of comments that explain what it's doing right now, but these comments seem like an odd thing to check in. Is this saying that there is unfinished work ("it has no inherent constructors yet: add them when a consumer needs them.")? Why not just add them now?

[jjkurczak]

I'll also chime in here - I think it keeps the cognitive load lower/reduces complexity to keep the implementation styles of the various Conditions consistent - either all via macro or all individually-defined. Would it be possible for i32 and i64 to be done together here in an equivalent signed_condition_impl macro?

[laruizlo]

Yeah, I didn't want to change much in the pre-existing code without consulting with you all, but fully agree that a single style would make it much cleaner. I would vote for going all macro, and can add the signed variant as well.

[ounsworth]

But I don't understand the comment. I think the comment is wrong.

I tried replacing this with

        fn assert_canonical(c: Condition<i64>, expected: bool) {
            // assert_eq!(c.select(-1, 0), if expected { -1 } else { 0 });
            assert_eq!(c.to_bool_var(), expected)
        }

and the test still passes. So it's not clear to me why this test needs to use the .select() function, or what that has to do with the raw bit mask (which I don't think is what select() is returning).

[ounsworth]

The naming of this feels weird to me. Could we call this .to_bool() instead of .to_bool_var()? Is there some significance in the _var() part of that?
(I know this is pre-existing before this PR, but if you agree, then we could clean it up at the same time)

[laruizlo]

Agree! It seems that Claude Code added the _var() to keep the symmetry with from_bool_var, but the reverse conversion doesn't have a const version, so it's more clear if we remove that, I think. I'll add that to the cleanup.