(D)TLS: Remove redundant verification of self-generated RSA signatures

peterdettman · peterdettman · commit 11877fcf4967 · 2024-09-18T19:28:55.000+07:00
- see #1722
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcDefaultTlsCredentialedSigner.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcDefaultTlsCredentialedSigner.java
@@ -1,7 +1,5 @@
 package org.bouncycastle.tls.crypto.impl.bc;
 
-import java.io.IOException;
-
 import org.bouncycastle.crypto.params.AsymmetricKeyParameter;
 import org.bouncycastle.crypto.params.DSAPrivateKeyParameters;
 import org.bouncycastle.crypto.params.ECPrivateKeyParameters;
@@ -21,16 +19,6 @@
 public class BcDefaultTlsCredentialedSigner
     extends DefaultTlsCredentialedSigner
 {
-    private static BcTlsCertificate getEndEntity(BcTlsCrypto crypto, Certificate certificate) throws IOException
-    {
-        if (certificate == null || certificate.isEmpty())
-        {
-            throw new IllegalArgumentException("No certificate");
-        }
-
-        return BcTlsCertificate.convert(crypto, certificate.getCertificateAt(0));
-    }
-
     private static TlsSigner makeSigner(BcTlsCrypto crypto, AsymmetricKeyParameter privateKey, Certificate certificate,
         SignatureAndHashAlgorithm signatureAndHashAlgorithm)
     {
@@ -48,17 +36,7 @@ private static TlsSigner makeSigner(BcTlsCrypto crypto, AsymmetricKeyParameter p
                 }
             }
 
-            RSAKeyParameters pubKeyRSA;
-            try
-            {
-                pubKeyRSA = getEndEntity(crypto, certificate).getPubKeyRSA();
-            }
-            catch (Exception e)
-            {
-                throw new RuntimeException(e);
-            }
-
-            signer = new BcTlsRSASigner(crypto, privKeyRSA, pubKeyRSA);
+            signer = new BcTlsRSASigner(crypto, privKeyRSA);
         }
         else if (privateKey instanceof DSAPrivateKeyParameters)
         {
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcTlsRSASigner.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcTlsRSASigner.java
@@ -24,13 +24,17 @@
 public class BcTlsRSASigner
     extends BcTlsSigner
 {
-    private final RSAKeyParameters publicKey;
-
+    /**
+     * @deprecated Use constructor without 'publicKey' parameter.
+     */
     public BcTlsRSASigner(BcTlsCrypto crypto, RSAKeyParameters privateKey, RSAKeyParameters publicKey)
     {
-        super(crypto, privateKey);
+        this(crypto, privateKey);
+    }
 
-        this.publicKey = publicKey;
+    public BcTlsRSASigner(BcTlsCrypto crypto, RSAKeyParameters privateKey)
+    {
+        super(crypto, privateKey);
     }
 
     public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] hash) throws IOException
@@ -63,21 +67,11 @@ public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] h
         signer.update(hash, 0, hash.length);
         try
         {
-            byte[] signature = signer.generateSignature();
-
-            signer.init(false, publicKey);
-            signer.update(hash, 0, hash.length);
-
-            if (signer.verifySignature(signature))
-            {
-                return signature;
-            }
+            return signer.generateSignature();
         }
         catch (CryptoException e)
         {
             throw new TlsFatalAlert(AlertDescription.internal_error, e);
         }
-
-        throw new TlsFatalAlert(AlertDescription.internal_error);
     }
 }
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcVerifyingStreamSigner.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/bc/BcVerifyingStreamSigner.java
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaDefaultTlsCredentialedSigner.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaDefaultTlsCredentialedSigner.java
@@ -1,8 +1,6 @@
 package org.bouncycastle.tls.crypto.impl.jcajce;
 
-import java.io.IOException;
 import java.security.PrivateKey;
-import java.security.PublicKey;
 import java.security.interfaces.DSAPrivateKey;
 import java.security.interfaces.RSAPrivateKey;
 
@@ -19,16 +17,6 @@
 public class JcaDefaultTlsCredentialedSigner
     extends DefaultTlsCredentialedSigner
 {
-    private static JcaTlsCertificate getEndEntity(JcaTlsCrypto crypto, Certificate certificate) throws IOException
-    {
-        if (certificate == null || certificate.isEmpty())
-        {
-            throw new IllegalArgumentException("No certificate");
-        }
-
-        return JcaTlsCertificate.convert(crypto, certificate.getCertificateAt(0));
-    }
-
     private static TlsSigner makeSigner(JcaTlsCrypto crypto, PrivateKey privateKey, Certificate certificate,
         SignatureAndHashAlgorithm signatureAndHashAlgorithm)
     {
@@ -50,17 +38,7 @@ private static TlsSigner makeSigner(JcaTlsCrypto crypto, PrivateKey privateKey,
                 }
             }
 
-            PublicKey publicKey;
-            try
-            {
-                publicKey = getEndEntity(crypto, certificate).getPubKeyRSA();
-            }
-            catch (Exception e)
-            {
-                throw new RuntimeException(e);
-            }
-
-            signer = new JcaTlsRSASigner(crypto, privateKey, publicKey);
+            signer = new JcaTlsRSASigner(crypto, privateKey);
         }
         else if (privateKey instanceof DSAPrivateKey
             || "DSA".equalsIgnoreCase(algorithm))
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaTlsCrypto.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaTlsCrypto.java
@@ -1057,39 +1057,6 @@ protected Tls13Verifier createTls13Verifier(String algorithmName, AlgorithmParam
         }
     }
 
-    protected TlsStreamSigner createVerifyingStreamSigner(SignatureAndHashAlgorithm algorithm, PrivateKey privateKey,
-        boolean needsRandom, PublicKey publicKey) throws IOException
-    {
-        String algorithmName = JcaUtils.getJcaAlgorithmName(algorithm);
-
-        return createVerifyingStreamSigner(algorithmName, null, privateKey, needsRandom, publicKey);
-    }
-
-    protected TlsStreamSigner createVerifyingStreamSigner(String algorithmName, AlgorithmParameterSpec parameter,
-        PrivateKey privateKey, boolean needsRandom, PublicKey publicKey) throws IOException
-    {
-        try
-        {
-            Signature signer = getHelper().createSignature(algorithmName);
-            Signature verifier = getHelper().createSignature(algorithmName);
-
-            if (null != parameter)
-            {
-                signer.setParameter(parameter);
-                verifier.setParameter(parameter);
-            }
-
-            signer.initSign(privateKey, needsRandom ? getSecureRandom() : null);
-            verifier.initVerify(publicKey);
-
-            return new JcaVerifyingStreamSigner(signer, verifier);
-        }
-        catch (GeneralSecurityException e)
-        {
-            throw new TlsFatalAlert(AlertDescription.internal_error, e);
-        }
-    }
-
     protected Boolean isSupportedEncryptionAlgorithm(int encryptionAlgorithm)
     {
         switch (encryptionAlgorithm)
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaTlsRSASigner.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaTlsRSASigner.java
@@ -25,11 +25,18 @@ public class JcaTlsRSASigner
 {
     private final JcaTlsCrypto crypto;
     private final PrivateKey privateKey;
-    private final PublicKey publicKey;
 
     private Signature rawSigner = null;
 
+    /**
+     * @deprecated Use constructor without 'publicKey' parameter.
+     */
     public JcaTlsRSASigner(JcaTlsCrypto crypto, PrivateKey privateKey, PublicKey publicKey)
+    {
+        this(crypto, privateKey);
+    }
+
+    public JcaTlsRSASigner(JcaTlsCrypto crypto, PrivateKey privateKey)
     {
         if (null == crypto)
         {
@@ -42,7 +49,6 @@ public JcaTlsRSASigner(JcaTlsCrypto crypto, PrivateKey privateKey, PublicKey pub
 
         this.crypto = crypto;
         this.privateKey = privateKey;
-        this.publicKey = publicKey;
     }
 
     public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] hash) throws IOException
@@ -78,15 +84,7 @@ public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] h
 
             signer.update(input, 0, input.length);
 
-            byte[] signature = signer.sign();
-
-            signer.initVerify(publicKey);
-            signer.update(input, 0, input.length);
-
-            if (signer.verify(signature))
-            {
-                return signature;
-            }
+            return signer.sign();
         }
         catch (GeneralSecurityException e)
         {
@@ -96,8 +94,6 @@ public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] h
         {
             this.rawSigner = null;
         }
-
-        throw new TlsFatalAlert(AlertDescription.internal_error);
     }
 
     public TlsStreamSigner getStreamSigner(SignatureAndHashAlgorithm algorithm) throws IOException
@@ -110,7 +106,7 @@ public TlsStreamSigner getStreamSigner(SignatureAndHashAlgorithm algorithm) thro
             && JcaUtils.isSunMSCAPIProviderActive()
             && isSunMSCAPIRawSigner())
         {
-            return crypto.createVerifyingStreamSigner(algorithm, privateKey, true, publicKey);
+            return crypto.createStreamSigner(algorithm, privateKey, true);
         }
 
         return null;
diff --git a/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaVerifyingStreamSigner.java b/tls/src/main/java/org/bouncycastle/tls/crypto/impl/jcajce/JcaVerifyingStreamSigner.java

Original file line number	Diff line number	Diff line change
`@@ -24,13 +24,17 @@`
`24`	`24`	`public class BcTlsRSASigner`
`25`	`25`	`extends BcTlsSigner`
`26`	`26`	`{`
`27`		`- private final RSAKeyParameters publicKey;`
`28`		`-`
	`27`	`+ /**`
	`28`	`+ * @deprecated Use constructor without 'publicKey' parameter.`
	`29`	`+ */`
`29`	`30`	`public BcTlsRSASigner(BcTlsCrypto crypto, RSAKeyParameters privateKey, RSAKeyParameters publicKey)`
`30`	`31`	`{`
`31`		`- super(crypto, privateKey);`
	`32`	`+ this(crypto, privateKey);`
	`33`	`+ }`
`32`	`34`
`33`		`- this.publicKey = publicKey;`
	`35`	`+ public BcTlsRSASigner(BcTlsCrypto crypto, RSAKeyParameters privateKey)`
	`36`	`+ {`
	`37`	`+ super(crypto, privateKey);`
`34`	`38`	`}`
`35`	`39`
`36`	`40`	`public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] hash) throws IOException`
`@@ -63,21 +67,11 @@ public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] h`
`63`	`67`	`signer.update(hash, 0, hash.length);`
`64`	`68`	`try`
`65`	`69`	`{`
`66`		`- byte[] signature = signer.generateSignature();`
`67`		`-`
`68`		`- signer.init(false, publicKey);`
`69`		`- signer.update(hash, 0, hash.length);`
`70`		`-`
`71`		`- if (signer.verifySignature(signature))`
`72`		`- {`
`73`		`- return signature;`
`74`		`- }`
	`70`	`+ return signer.generateSignature();`
`75`	`71`	`}`
`76`	`72`	`catch (CryptoException e)`
`77`	`73`	`{`
`78`	`74`	`throw new TlsFatalAlert(AlertDescription.internal_error, e);`
`79`	`75`	`}`
`80`		`-`
`81`		`- throw new TlsFatalAlert(AlertDescription.internal_error);`
`82`	`76`	`}`
`83`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -25,11 +25,18 @@ public class JcaTlsRSASigner`
`25`	`25`	`{`
`26`	`26`	`private final JcaTlsCrypto crypto;`
`27`	`27`	`private final PrivateKey privateKey;`
`28`		`- private final PublicKey publicKey;`
`29`	`28`
`30`	`29`	`private Signature rawSigner = null;`
`31`	`30`
	`31`	`+ /**`
	`32`	`+ * @deprecated Use constructor without 'publicKey' parameter.`
	`33`	`+ */`
`32`	`34`	`public JcaTlsRSASigner(JcaTlsCrypto crypto, PrivateKey privateKey, PublicKey publicKey)`
	`35`	`+ {`
	`36`	`+ this(crypto, privateKey);`
	`37`	`+ }`
	`38`	`+`
	`39`	`+ public JcaTlsRSASigner(JcaTlsCrypto crypto, PrivateKey privateKey)`
`33`	`40`	`{`
`34`	`41`	`if (null == crypto)`
`35`	`42`	`{`
`@@ -42,7 +49,6 @@ public JcaTlsRSASigner(JcaTlsCrypto crypto, PrivateKey privateKey, PublicKey pub`
`42`	`49`
`43`	`50`	`this.crypto = crypto;`
`44`	`51`	`this.privateKey = privateKey;`
`45`		`- this.publicKey = publicKey;`
`46`	`52`	`}`
`47`	`53`
`48`	`54`	`public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] hash) throws IOException`
`@@ -78,15 +84,7 @@ public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] h`
`78`	`84`
`79`	`85`	`signer.update(input, 0, input.length);`
`80`	`86`
`81`		`- byte[] signature = signer.sign();`
`82`		`-`
`83`		`- signer.initVerify(publicKey);`
`84`		`- signer.update(input, 0, input.length);`
`85`		`-`
`86`		`- if (signer.verify(signature))`
`87`		`- {`
`88`		`- return signature;`
`89`		`- }`
	`87`	`+ return signer.sign();`
`90`	`88`	`}`
`91`	`89`	`catch (GeneralSecurityException e)`
`92`	`90`	`{`
`@@ -96,8 +94,6 @@ public byte[] generateRawSignature(SignatureAndHashAlgorithm algorithm, byte[] h`
`96`	`94`	`{`
`97`	`95`	`this.rawSigner = null;`
`98`	`96`	`}`
`99`		`-`
`100`		`- throw new TlsFatalAlert(AlertDescription.internal_error);`
`101`	`97`	`}`
`102`	`98`
`103`	`99`	`public TlsStreamSigner getStreamSigner(SignatureAndHashAlgorithm algorithm) throws IOException`
`@@ -110,7 +106,7 @@ public TlsStreamSigner getStreamSigner(SignatureAndHashAlgorithm algorithm) thro`
`110`	`106`	`&& JcaUtils.isSunMSCAPIProviderActive()`
`111`	`107`	`&& isSunMSCAPIRawSigner())`
`112`	`108`	`{`
`113`		`- return crypto.createVerifyingStreamSigner(algorithm, privateKey, true, publicKey);`
	`109`	`+ return crypto.createStreamSigner(algorithm, privateKey, true);`
`114`	`110`	`}`
`115`	`111`
`116`	`112`	`return null;`