Skip to content

Take the ITS explicit certificate signature choice from the content signer's curve - #2463

Open
Arpan0995 wants to merge 1 commit into
bcgit:mainfrom
Arpan0995:its-explicit-cert-brainpool-signature-choice
Open

Arpan0995 wants to merge 1 commit into
bcgit:mainfrom
Arpan0995:its-explicit-cert-brainpool-signature-choice

Conversation

@Arpan0995

Copy link
Copy Markdown
Contributor

ITSExplicitCertificateBuilder.build picks the certificate's Signature CHOICE by switching a VerificationKeyIndicator CHOICE, the certificate's own when self-signing and otherwise the issuer's, against the PublicVerificationKey curve constants (ITSExplicitCertificateBuilder.java:65-75, :91-104). The two CHOICE spaces are unrelated: verificationKey = 0 and reconstructionValue = 1 (VerificationKeyIndicator.java:25-26) index ecdsaNistP256 and ecdsaBrainpoolP256r1 (PublicVerificationKey.java:24-26), so the signing key's curve never enters the decision:

  • a self-signed brainpoolP256r1 certificate is tagged ecdsaNistP256Signature, and the verifier picked by that tag (ITSCertificate.java:57) refuses it with OperatorCreationException: wrong verifier for algorithm: 0 (BcITSContentVerifierProvider.java:83-86);
  • no certificate can be signed with a brainpoolP384r1 key: the NIST arm applies the 32-byte encoding at ECDSAEncoder.java:60 to a 384-bit r, and 25 of 25 self-signed attempts threw IllegalArgumentException: standard length exceeded for value. A brainpoolP384r1 subject key under a P-256 issuer is unaffected;
  • an explicit certificate under an implicit issuer takes tag 1 from reconstructionValue = 1: a secp256r1 signature is silently mislabelled ecdsaBrainpoolP256r1Signature, a brainpoolP256r1 one happens to be right, and a brainpoolP384r1 signer throws as above.

Reproduced on the released bcpkix-jdk18on-1.86.jar, on current origin/main (94270ff1cb) and on the byte-identical 1.87-SNAPSHOT beta (1.87.0.20719), with the same output and the re-decoded OER carrying the same tag. build takes no curve argument, so there is no workaround; every path is fail-closed. ETSISignedDataBuilder reads the curve from the signer already (ETSISignedDataBuilder.java:104, :131, :153).

This change:

  • takes the curve from the content signer, ECDSAEncoder.toITS(signer.getCurveID(), signer.getSignature()), dropping two locals and three imports. Both shipped signers reject a fourth curve at construction with IllegalArgumentException: unknown key type (BcITSContentSigner.java:64, JcaITSContentSigner.java:101); a third-party ITSContentSigner reporting one now gets IllegalArgumentException: unknown curveID (ECDSAEncoder.java:76) rather than the removed arm's IllegalStateException. JcaITSExplicitCertificateBuilder inherits the corrected build; and
  • adds testBuildSelfSignedCurves and testBuildUnderImplicitIssuer to ITSBasicTest: the first builds a self-signed CA and an end-entity certificate under it on all three curves, checking the emitted CHOICE and that the verifier accepts each; the second pins the implicit-issuer case. Both fail without the change (ITSBasicTest.java:366, :452) and pass with it; the package's 21 pre-existing tests pass either way.

Base tree only, no overlays and no module-info change (org.bouncycastle.its is already exported). A release-note entry is included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant