diff --git a/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Client.java b/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Client.java index 0d9bdb4e30..838363d9a3 100644 --- a/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Client.java +++ b/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Client.java @@ -7,6 +7,7 @@ import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.bouncycastle.crypto.Digest; import org.bouncycastle.crypto.params.SRP6GroupParameters; +import org.bouncycastle.util.Arrays; import org.bouncycastle.util.BigIntegers; /** @@ -154,7 +155,7 @@ public boolean verifyServerEvidenceMessage(BigInteger serverM2) throws CryptoExc // Compute the own server evidence message 'M2' BigInteger computedM2 = SRP6Util.calculateM2(digest, N, A, M1, S); - if (computedM2.equals(serverM2)) + if (Arrays.constantTimeAreEqual(computedM2.toByteArray(), serverM2.toByteArray())) { this.M2 = serverM2; return true; diff --git a/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Server.java b/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Server.java index 6a13675893..d560deca0d 100644 --- a/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Server.java +++ b/core/src/main/java/org/bouncycastle/crypto/agreement/srp/SRP6Server.java @@ -7,6 +7,7 @@ import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.bouncycastle.crypto.Digest; import org.bouncycastle.crypto.params.SRP6GroupParameters; +import org.bouncycastle.util.Arrays; import org.bouncycastle.util.BigIntegers; /** @@ -133,7 +134,7 @@ public boolean verifyClientEvidenceMessage(BigInteger clientM1) throws CryptoExc // Compute the own client evidence message 'M1' BigInteger computedM1 = SRP6Util.calculateM1(digest, N, A, B, S); - if (computedM1.equals(clientM1)) + if (Arrays.constantTimeAreEqual(computedM1.toByteArray(), clientM1.toByteArray())) { this.M1 = clientM1; return true; diff --git a/core/src/test/java/org/bouncycastle/crypto/test/SRP6Test.java b/core/src/test/java/org/bouncycastle/crypto/test/SRP6Test.java index 9cf6d444c8..1b0e0dd9c7 100644 --- a/core/src/test/java/org/bouncycastle/crypto/test/SRP6Test.java +++ b/core/src/test/java/org/bouncycastle/crypto/test/SRP6Test.java @@ -39,6 +39,7 @@ public void performTest() throws Exception rfc5054AppendixBTestVectors(); testMutualVerification(SRP6StandardGroups.rfc5054_1024); + testEvidenceMessageVerification(SRP6StandardGroups.rfc5054_1024); testClientCatchesBadB(SRP6StandardGroups.rfc5054_1024); testServerCatchesBadA(SRP6StandardGroups.rfc5054_1024); @@ -288,6 +289,65 @@ private void testMutualVerification(SRP6GroupParameters group) throws CryptoExce } } + /** + * Exercises the evidence-message (M1/M2) verification path, which is where the received + * keyed authenticators are checked. A correct M1/M2 must be accepted and a tampered one + * rejected, whether the tampering leaves the encoded length the same or shortens it. + */ + private void testEvidenceMessageVerification(SRP6GroupParameters group) throws CryptoException + { + byte[] I = "username".getBytes(); + byte[] P = "password".getBytes(); + byte[] s = new byte[16]; + random.nextBytes(s); + + SRP6VerifierGenerator gen = new SRP6VerifierGenerator(); + gen.init(group, SHA256Digest.newInstance()); + BigInteger v = gen.generateVerifier(s, I, P); + + SRP6Client client = new SRP6Client(); + client.init(group, SHA256Digest.newInstance(), random); + + SRP6Server server = new SRP6Server(); + server.init(group, v, SHA256Digest.newInstance(), random); + + BigInteger A = client.generateClientCredentials(s, I, P); + BigInteger B = server.generateServerCredentials(); + + client.calculateSecret(B); + server.calculateSecret(A); + + BigInteger clientM1 = client.calculateClientEvidenceMessage(); + + if (!server.verifyClientEvidenceMessage(clientM1)) + { + fail("server rejected the correct client evidence message M1"); + } + if (server.verifyClientEvidenceMessage(clientM1.add(BigInteger.valueOf(1)))) + { + fail("server accepted a tampered client evidence message M1"); + } + if (server.verifyClientEvidenceMessage(clientM1.shiftRight(8))) + { + fail("server accepted a truncated client evidence message M1"); + } + + BigInteger serverM2 = server.calculateServerEvidenceMessage(); + + if (!client.verifyServerEvidenceMessage(serverM2)) + { + fail("client rejected the correct server evidence message M2"); + } + if (client.verifyServerEvidenceMessage(serverM2.add(BigInteger.valueOf(1)))) + { + fail("client accepted a tampered server evidence message M2"); + } + if (client.verifyServerEvidenceMessage(serverM2.shiftRight(8))) + { + fail("client accepted a truncated server evidence message M2"); + } + } + private void testClientCatchesBadB(SRP6GroupParameters group) { byte[] I = "username".getBytes(); diff --git a/docs/releasenotes.html b/docs/releasenotes.html index da3c482a5d..764c5e8edf 100644 --- a/docs/releasenotes.html +++ b/docs/releasenotes.html @@ -51,6 +51,7 @@

2.1.2 Defects Fixed

  • GOST R 34.10-94 signing (org.bouncycastle.crypto.signers.GOST3410Signer) raised the domain generator to the per-signature nonce k with a bare BigInteger.modPow, whose running time varies with the exponent. Recovering k from that timing yields the private key straight out of the signature equation s = k*m + x*r, so k is now randomised with a random multiple of q before it is raised, exactly as DSASigner already does with its own k. Since the domain parameter a has order q, raising it to a multiple of q gives 1 and the signature is unchanged - the RFC-style known-answer vectors in GOST3410Test still produce the same r and s. Those vectors drive signing from a FixedSecureRandom, so they now carry one further byte for the randomiser to consume, in the same way the DSA signing vectors already do.
  • KCCMBlockCipher (DSTU7624-128/256/512 CCM mode) returned the input length rather than 0 from getUpdateOutputSize(int), but like CCMBlockCipher/KGCMBlockCipher it buffers all input until doFinal and produces no output on an update. Through the JCA layer this made the caller-supplied-buffer Cipher.update(input, inOff, inLen, output, outOff) reject a correctly sized output buffer with "javax.crypto.ShortBufferException: output buffer too short for input." when decrypting. getUpdateOutputSize now returns 0, matching the sibling CCM/KGCM modes (github #2354).
  • J-PAKE raised values to exponents carrying private material with bare BigInteger.modPow calls, whose running time varies with the exponent: the private ephemerals x1 and x2 in round 1, x2*s and the negated form of it in round 2 and in the keying material - both of which carry the password - and the v behind each Schnorr zero-knowledge proof, which together with the published r would give up x. JPAKEParticipant itself notes that leaking x1 or x2 lets an attacker brute-force the password. All of these exponents are now randomised with a random multiple of q before they are raised. A multiple of q rather than of p-1 is sound here, and much cheaper since the exponents are the size of q: the generator is checked with g^q = 1 when the JPAKEPrimeOrderGroup is built, and each value received from the other participant is checked the same way by validateZeroKnowledgeProof before it is used as a base. JPAKEUtil.calculateA and JPAKEUtil.calculateKeyingMaterial gained overloads taking a SecureRandom, and there is a new JPAKEUtil.calculateGx taking q and a SecureRandom; the existing overloads still work, taking the default from CryptoServicesRegistrar. The three-argument calculateGx has no q to work with, so it blinds with a multiple of p-1 and is deprecated in favour of the new one. The three modPow calls in validateZeroKnowledgeProof are unchanged, since their exponents are all public. The elliptic-curve variant was already routing its private scalars through ECAlgorithms.multiplySecret and needed no change.
  • +
  • The SRP-6a evidence-message checks compared the authenticator received from the peer against the locally computed one with BigInteger.equals, whose word-by-word magnitude comparison returns as soon as it meets a difference and so runs in a value-dependent time. M1 and M2 are keyed authenticators derived from the shared secret S and arrive from the not-yet-authenticated peer, so that early-out is a MAC-comparison timing oracle against a value the peer is trying to guess. SRP6Server.verifyClientEvidenceMessage and SRP6Client.verifyServerEvidenceMessage - in both the lightweight org.bouncycastle.crypto.agreement.srp package and the org.bouncycastle.tls.crypto.impl.jcajce.srp copies BCTLS carries - now compare with Arrays.constantTimeAreEqual over the fixed encodings, as the sibling J-PAKE MacTag check already does. The accept/reject decision is unchanged for every input, so a correct M1/M2 still verifies and any other value is still rejected.
  • The PKIX CertPathBuilder ("PKIX"/"RFC5280"/"RFC3280") matched candidate issuers by subject name only during its depth-first search, so a CertStore containing many self-issued certificates that share a single subject name and never chain to a trust anchor could be explored as a large number of partial paths before the build concluded no chain exists. The builder now bounds the total number of nodes visited per build; the limit is configurable via the org.bouncycastle.x509.max_cert_path_build_nodes system property (default 262144, far above any legitimate build) and, when exceeded, the build fails with a CertPathBuilderException naming the property. This is the builder-side companion to the existing org.bouncycastle.x509.max_policy_nodes bound.
  • A group of parse and revocation-handling entry points let an unchecked runtime exception (NullPointerException, ArrayIndexOutOfBoundsException, IllegalStateException or ArithmeticException) escape on empty, content-less or out-of-range input instead of the checked exception each entry point declares - the malformed input was rejected either way, but the leaked type could escape a documented throws contract. Each now fails with its declared type, and well-formed input is unaffected: org.bouncycastle.tsp.cms.CMSTimeStampedData (an empty or truncated stream; the fix also covers its org.bouncycastle.asn1.cms.MetaData and TimeStampDataUtil helpers) and org.bouncycastle.cms.CMSEnvelopedData (an EnvelopedData carrying no encryptedContent) now throw IOException / CMSException rather than NullPointerException; org.bouncycastle.tsp.TimeStampToken rejects a token whose SignerInfo carries no signed attributes with TSPValidationException rather than NullPointerException; org.bouncycastle.cert.cmp.GeneralPKIMessage, org.bouncycastle.est.CSRAttributesResponse, org.bouncycastle.cmc.SimplePKIResponse, org.bouncycastle.openssl.X509TrustedCertificateBlock, org.bouncycastle.tsp.TimeStampRequest, org.bouncycastle.tsp.TimeStampResponse, org.bouncycastle.pkcs.PKCS12PfxPdu and org.bouncycastle.pkcs.PKCS8EncryptedPrivateKeyInfo reject empty / no-content / truncated input with their declared CertIOException / IOException / PKCSIOException rather than a leaked NullPointerException, and org.bouncycastle.cert.crmf.CertificateRequestMessage.hasSigningKeyProofOfPossessionWithPKMAC answers false for an absent or non-signing-key proof-of-possession rather than throwing NullPointerException; org.bouncycastle.crypto.util.OpenSSHPrivateKeyUtil.parsePrivateKeyBlob and org.bouncycastle.math.ec.ECCurve.decodePoint reject an empty (or null) blob / point encoding with IllegalArgumentException rather than ArrayIndexOutOfBoundsException, closing the point-decode path reached by every untrusted-point consumer (EC key parsing, ECDH/ECIES, TLS); and the PKIX revocation code no longer leaks a runtime exception on attacker-controlled CRL/OCSP fields - PKIXCertPathReviewer and X509RevocationChecker bound an out-of-range CRLReason code against their fixed reason table (reporting "unknown" instead of ArrayIndexOutOfBoundsException / ArithmeticException), RFC3280CertPathUtilities tolerates an absent reasons mask on a CRL distribution point, and ProvOcspRevocationChecker tolerates an OCSP response with no nonce extension.
  • java.security.AlgorithmParameters.init(byte[]) is contracted to throw IOException on a decoding error, but several BC AlgorithmParameters SPIs (RSA OAEP/PSS, EC, DSA, DH, ElGamal, IES, GOST, and the GCM/CCM parameters of AES, ARIA, LEA and SM4) could leak an unchecked exception. Each affected engineInit(byte[]) and both loadParameters helpers now convert a leaked runtime exception to IOException; well-formed parameters are unaffected.
  • diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Client.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Client.java index 3633e3da07..f08b92bd2d 100644 --- a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Client.java +++ b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Client.java @@ -6,6 +6,7 @@ import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.bouncycastle.tls.crypto.SRP6Group; import org.bouncycastle.tls.crypto.TlsHash; +import org.bouncycastle.util.Arrays; import org.bouncycastle.util.BigIntegers; /** @@ -152,7 +153,7 @@ public boolean verifyServerEvidenceMessage(BigInteger serverM2) throws IllegalSt // Compute the own server evidence message 'M2' BigInteger computedM2 = SRP6Util.calculateM2(digest, N, A, M1, S); - if (computedM2.equals(serverM2)) + if (Arrays.constantTimeAreEqual(computedM2.toByteArray(), serverM2.toByteArray())) { this.M2 = serverM2; return true; diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Server.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Server.java index 752ae1e857..fa76a1c7e1 100644 --- a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Server.java +++ b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/srp/SRP6Server.java @@ -6,6 +6,7 @@ import org.bouncycastle.crypto.CryptoServicesRegistrar; import org.bouncycastle.tls.crypto.SRP6Group; import org.bouncycastle.tls.crypto.TlsHash; +import org.bouncycastle.util.Arrays; import org.bouncycastle.util.BigIntegers; /** @@ -131,7 +132,7 @@ public boolean verifyClientEvidenceMessage(BigInteger clientM1) throws IllegalSt // Compute the own client evidence message 'M1' BigInteger computedM1 = SRP6Util.calculateM1(digest, N, A, B, S); - if (computedM1.equals(clientM1)) + if (Arrays.constantTimeAreEqual(computedM1.toByteArray(), clientM1.toByteArray())) { this.M1 = clientM1; return true;