test: exercise SPHINCS+ OID round-trip for -simple variants - #2345
Open
major wants to merge 4047 commits into
Open
test: exercise SPHINCS+ OID round-trip for -simple variants#2345major wants to merge 4047 commits into
major wants to merge 4047 commits into
Conversation
… with an IOException instead of throwing a NullPointerException, closing a denial-of-service when verifying a malformed signature
…lenge (parser robustness)
Fix StringIndexOutOfBoundsException parsing a malformed EST auth challenge (parser robustness) See merge request root/bc-java!222
Use constant-time comparison for secret-bearing private-key `equals()` See merge request root/bc-java!223
…signed byte arrays BigInteger.toByteArray() prepends a two's-complement sign byte when the MSB is set, so two values of the same bit-length can serialise to different lengths (256 vs 257 bytes for a 2048-bit value) purely on that bit, which the variable-time Arrays.constantTimeAreEqual then leaks. Size both operands from their own unsigned byte lengths (the larger of the two) and compare the unsigned fixed-length forms; this drops the sign byte and makes the comparison length-uniform. Not perfectly constant-time -- toByteArray() in the conversion still scales with magnitude -- but it removes the high-bit length signal. Boolean result unchanged (checked against BigInteger.equals over 50k random pairs); no caller changes.
…arse time Features, TrustSignature, SignatureTarget, RevocationKey and RevocationReason index a fixed offset of their body from their accessors (e.g. Features.getFeatures() reads data[0], TrustSignature.getTrustAmount() reads data[1]) but their (boolean, boolean, byte[]) wire-parse constructor did not validate the body length. SignatureSubpacketInputStream accepts a subpacket whose length field is 1 (an empty body), so a truncated subpacket decoded cleanly and then threw ArrayIndexOutOfBoundsException when an accessor was read. Each parse constructor now runs verifyData(data), rejecting a body too short for its accessors with an IllegalArgumentException (surfaced as a MalformedPacketException by the parser), matching the existing IssuerFingerprint and IntendedRecipientFingerprint guards. The value-based constructors are unchanged.
Document the constant-time contract of BigIntegers.constantTimeAreEqual See merge request root/bc-java!224
…rusting revocation status
…ON2 and rename the calculator to JcePGPS2KCalculator
…qual and complete its javadoc
…mary HQC algorithm name with no-dash aliases
… and BCPQC providers as announced in the 1.84 release notes, the low-level implementations remain available
…info converter test sweeping all 36 MQOM parameter sets, added AIMerParameterSpec to the fromName round-trip sweep
…ine classes, the public API surface is the key pair generator and signer
… moved the FAEST Owf cross-check helper into the test tree where its only caller lives
…s, a leftover of the expired draft-zauner-tls-aes-ocb, together with its module exports
…QC packages and added the missing package-info files for the aimer, qruov, sdith and xwing packages
…ferences to oid is provider code
…TRU+ decapsulation, DHAgreement peer validation, exception cause-chaining, the fixed-arity ASN.1 SEQUENCE guard batch, DSTU4145/OpenPGP/CRMF/TLS/DANE/TSP/ERS/SSH/EST robustness fixes, CMS RFC 5084 GCM parameters, X509CertificateReviewer, PKCS12 default iteration count, Mayo OQS OIDs, Ed25519 expanded keys, ECDHRawAgreement, ML-DSA and DH performance work, name finder and ASN.1 limit constants
…phaning the continuation of an entry) and specifications.html (stray bold closers in the cipher tables, unmatched paragraph closes, XSalsa20 IV size corrected to 192 bits)
…the provider module descriptor, matching the compositesignatures package
…m set: ARIA, GIFT-COFB, Romulus, RFC 5649 wrap, ChaCha7539/XChaCha20, VMPC-KSA3, LEA, Shacal-2, GOST3412-2015, additional Blake2 variants, SHA-512/t, KMAC/KGMac/Blake3Mac, X25519/X448 and PAKE agreement, SAKKE, NTRU+, X-Wing, the ISO 18033-2 Classic McEliece sets, the provider KEM and PQC signature listings, HASH-ML-DSA and Argon2
…leset after the github bcgit#2176 consolidation into core, and replaced System.clearProperty with getProperties().remove in PKIXNameConstraintsTest for the 1.4 API floor.
…vate tests MayoRetryTest and HttpUtilTest which sit in signed main packages. Moved them out of the shared pqc.crypto.test/est.test AllTests into package-local AllTests (run by Gradle) and dropped those from the signed-jar test staging, matching the lms/keybox pattern.
…qcMalformedInputTest so the -source 1.5 compile accepts the anonymous Runnable captures (Java 8 effectively-final let the Gradle build through).
…to a pre-2024 fork, reintroducing the AES-CCM tag bug (CCM params routed through GCMParameters, rejected as 'Invalid ICV length: 8' on JRE 5 BCFKS/PKCS AEAD paths). Regenerated from base keeping the sole JRE-5 adaptation - avoiding the Java 8 PBEParameterSpec.getParameterSpec().
…atching jdk18+.xml: keeps the package-private tests in signed main packages out of the unsigned test jar (the bc+-build.xml AllTests deletes already prevented the clash; this stops them being compiled at all).
…ata version by default; add opt-in org.bouncycastle.cms.signeddata.preserve_version to carry the original version over verbatim for Authenticode interop (version 1 with a non-id-data eContentType). Adds a SignedData ctor taking an explicit version, relates to github bcgit#2344.
…silently dropping key blobs that follow a free/deleted slot; empty blobs are now skipped and parsing continues, relates to github bcgit#2343.
…non-tagged unsignedAttrs with IllegalArgumentException instead of leaking a ClassCastException, decoding via getInstance to match asn1.pkcs.SignerInfo, relates to github bcgit#2342.
…tension parsers CertificatePolicies, PolicyMappings, ExtendedKeyUsage, CRLDistPoint and SubjectDirectoryAttributes, matching AuthorityInformationAccess/NameConstraints, relates to github bcgit#2331.
… KMIPInputStream XXE (DTD/external-entity) hardening, relates to github bcgit#2315.
…ibution point so it survives the empty-SEQUENCE CRLDistPoint hardening, relates to github bcgit#2331.
…rdMaxTime <= elapsed) to match the javadoc and hard-fail at exactly maxTime; removes the same-millisecond flakiness that RevocationTest.testRevokedEndEntityWithSoftFailure papered over with a 1s Thread.sleep, now dropped.
…ignedData.asVersion(int) method that returns a copy with the version field forced, so a producer (e.g. Authenticode needing version 1) can pin it explicitly per-instance while replaceSigners/addDigestAlgorithm keep recomputing the RFC 5652 version by default, relates to github bcgit#2344.
The OID encode/decode round-trip in SphincsPlusTest was skipped for -simple hash-mode variants behind a stale FIXME guard. OID mappings for these variants were already wired correctly in commit 729a3aa (interop updates for SPHINCS+, Nov 2023), aligning with FIPS 205 SLH-DSA finalizing on the simple hashing mode, so the guard was never needed after that change. - remove the stale FIXME/guard so the round-trip runs for all variants - add parameter-identity assertions so decode(encode(params)) is verified to return the exact same parameter set, not just matching raw key bytes - apply the same fix to the jdk1.3 mirror of this test Verified locally: all 9 SphincsPlusTest cases pass, including every -simple, -robust, and haraka variant. Investigation and fix drafted with AI assistance (Claude); reviewed and verified by the author before submission. Signed-off-by: Major Hayden <major@redhat.com>
Contributor
|
Thanks for the patch. I've merged this, but please note SPHINCS+ will be getting deleted in the next release or two in favor of SLH-DSA. The new PQC algorithms take a considerable amount of space and with the new prototype signature algorithms around as well, it's becoming quite a challenge to stop the provider size exploding. |
hubot
pushed a commit
that referenced
this pull request
Jul 9, 2026
…riants in SphincsPlusTest (and its jdk1.3 mirror): the OIDs were already wired, so the FIXME skip was stale; add parameter-identity assertions to catch future OID/parameter mapping regressions, relates to github #2345.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This removes a stale
FIXMEinSphincsPlusTestthat was skipping the SubjectPublicKeyInfo/PrivateKeyInfo (OID) encode/decode round-trip for SPHINCS+ "-simple" hash-mode variants.Why the FIXME was stale:
sphincsPlusOids/sphincsPlusParamsinpqc/crypto/util/Utils.javaalready have correct forward and reverse OID mappings for all "-simple" variants (haraka has dedicated_simple-suffixed constants; sha2/shake reuse the unsuffixed constant, which now maps to the final SLH-DSA standard OID rather than the round-3-draft one). This was wired up in729a3aa83("interop updates for SPHINCS+", Nov 2023), which aligned BC's naming with FIPS 205 finalizing on the "simple" hashing mode and dropping "robust". The test guard just never got removed after that change landed.What changed:
// FIXME No OIDs for simple variants of SPHINCS+comment and theif (name.indexOf("-simple") < 0)guard, so the OID round-trip now runs unconditionally for every variant.assertTrue(... parameters == pubParams.getParameters())/ same for private key, so the test verifiesdecode(encode(params))returns the exact same parameter-set instance, not just that raw key bytes happen to match afterward.jdk1.3mirror of this test.Verification: ran
SphincsPlusTestlocally (viajunit.textui.TestRunner, since the gradle module restricts--teststoAllTest*), all 9 cases pass, including every-simple,-robust, and haraka variant now exercising the full ASN.1 round-trip plus the new parameter-identity checks.Investigation and fix drafted with AI assistance (Claude); I reviewed the git history, verified the OID mapping tables, and ran the tests myself before submitting.