KeyAgreement now reports the IllegalStateException javax.crypto.KeyAgreement specifies when a shared secret is asked for before doPhase has completed the agreement, rather than returning whatever each SPI's result field held, and the DH SPI no longer holds the private value in that field.

Author: dghgit

docs/releasenotes.md

@@ -15,6 +15,8 @@ Date: 2026, TBD
 
 ### 2.1.2 Defects Fixed
 
+- A KeyAgreement asked for its shared secret before doPhase returned data rather than refusing. javax.crypto.KeyAgreement specifies IllegalStateException for that state, but nothing in the provider tracked it, so each SPI handed back whatever its result field held: for Diffie-Hellman that was the private value itself - engineInit seeded result with x, so generateSecret() returned the private exponent padded to the prime's length and generateSecret("AES") an all-zero key taken from that padding - while ECDH returned null and its named-algorithm overload raised NullPointerException. BaseAgreementSpi now records whether a doPhase has completed the agreement since the last init and refuses the request with an IllegalStateException naming the algorithm, so every family in the provider - DH, ECDH and ECMQV, the SM2 exchange, both ECGOST families, XDH, SM9 and NewHope - answers the same way, and the DH SPI no longer holds the private value in that field at all.
+
 ### 2.1.3 Additional Features and Functionality
 
 ### 2.1.4 Additional Notes

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/dh/KeyAgreementSpi.java

@@ -153,6 +153,8 @@ protected Key engineDoPhase(
 
             result = unifiedAgreement.calculateAgreement(pKey);
 
+            agreementCompleted();
+
             return null;
         }
         else if (mqvAgreement != null)
@@ -169,6 +171,8 @@ else if (mqvAgreement != null)
 
             result = bigIntToBytes(mqvAgreement.calculateAgreement(pKey));
 
+            agreementCompleted();
+
             return null;
         }
         else
@@ -191,6 +195,8 @@ else if (mqvAgreement != null)
 
             if (lastPhase)
             {
+                agreementCompleted();
+
                 return null;
             }
 
@@ -337,14 +343,15 @@ else if (params instanceof UserKeyingMaterialSpec)
         }
 
         this.x = privKey.getX();
-        this.result = bigIntToBytes(x);
     }
 
     protected void engineInit(
         Key             key,
         SecureRandom    random) 
         throws InvalidKeyException
     {
+        resetAgreement();
+
         if (!(key instanceof DHPrivateKey))
         {
             throw new InvalidKeyException("DHKeyAgreement requires DHPrivateKey");
@@ -356,7 +363,6 @@ protected void engineInit(
         this.g = privKey.getParams().getG();
         this.x = privKey.getX();
         this.random = CryptoServicesRegistrar.getSecureRandom(random);
-        this.result = bigIntToBytes(x);
     }
 
     protected byte[] doCalcSecret()

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/GMKeyExchangeSpi.java

@@ -63,6 +63,8 @@ protected Key engineDoPhase(
 
         result = engine.calculateKey(128, parameters);
 
+        agreementCompleted();
+
         return null;
     }
 

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ec/KeyAgreementSpi.java

@@ -158,6 +158,8 @@ public Throwable getCause()
             };
         }
 
+        agreementCompleted();
+
         return null;
     }
 

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ecgost/KeyAgreementSpi.java

@@ -88,6 +88,8 @@ public Throwable getCause()
             };
         }
 
+        agreementCompleted();
+
         return null;
     }
 

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/ecgost12/KeyAgreementSpi.java

@@ -84,6 +84,8 @@ public Throwable getCause()
             };
         }
 
+        agreementCompleted();
+
         return null;
     }
 

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/edec/KeyAgreementSpi.java

@@ -151,6 +151,8 @@ protected Key engineDoPhase(Key key, boolean lastPhase)
             agreement.calculateAgreement(pub, result, 0);
         }
 
+        agreementCompleted();
+
         return null;
     }
 

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/sm9/KeyAgreementSpi.java

@@ -118,6 +118,8 @@ protected Key engineDoPhase(Key key, boolean lastPhase)
             throw SecurityExceptions.invalidKeyException(e.getMessage(), e);
         }
 
+        agreementCompleted();
+
         return null;
     }
 

prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/util/BaseAgreementSpi.java

@@ -166,6 +166,7 @@ public abstract class BaseAgreementSpi
     protected byte[] ukmParameters;
     protected byte[] ukmParametersSalt;
     private HybridValueParameterSpec hybridSpec;
+    private boolean completed;
 
     public BaseAgreementSpi(String kaAlgorithm, DerivationFunction kdf)
     {
@@ -268,6 +269,8 @@ protected void engineInit(
         SecureRandom random)
         throws InvalidKeyException
     {
+        resetAgreement();
+
         try
         {
             doInitFromKey(key, null, random);
@@ -285,6 +288,8 @@ protected void engineInit(
         SecureRandom random)
         throws InvalidKeyException, InvalidAlgorithmParameterException
     {
+        resetAgreement();
+
         if (params instanceof HybridValueParameterSpec)
         {
             this.hybridSpec = (HybridValueParameterSpec)params;
@@ -298,6 +303,39 @@ protected void engineInit(
         }
     }
 
+    /**
+     * Record that the agreement has run to completion and its shared secret may now be generated.
+     * A subclass calls this on the successful return of the doPhase that completes the agreement.
+     */
+    protected void agreementCompleted()
+    {
+        this.completed = true;
+    }
+
+    /**
+     * Require a fresh agreement before the next shared secret. Called by the engineInit methods
+     * here; a subclass that overrides one of those rather than doInitFromKey must call this itself.
+     */
+    protected void resetAgreement()
+    {
+        this.completed = false;
+    }
+
+    /**
+     * Refuse a shared secret asked for before the agreement has completed. javax.crypto.KeyAgreement
+     * specifies IllegalStateException for that state; without this each SPI would hand back whatever
+     * its result field happened to hold, which for DH was the private value itself.
+     *
+     * @throws IllegalStateException if no doPhase has completed the agreement since the last init.
+     */
+    protected void checkAgreementCompleted()
+    {
+        if (!completed)
+        {
+            throw new IllegalStateException(kaAlgorithm + " key agreement: doPhase must be called before generateSecret");
+        }
+    }
+
     protected byte[] engineGenerateSecret()
         throws IllegalStateException
     {
@@ -437,6 +475,8 @@ else if (kdf instanceof HKDFBytesGenerator)
 
     private byte[] calcSecret()
     {
+        checkAgreementCompleted();
+
         if (hybridSpec != null)
         {
             // Set Z' to Z || T

prov/src/main/java/org/bouncycastle/pqc/jcajce/provider/newhope/KeyAgreementSpi.java

@@ -33,6 +33,8 @@ public KeyAgreementSpi()
     protected void engineInit(Key key, SecureRandom secureRandom)
         throws InvalidKeyException
     {
+        resetAgreement();
+
         if (key != null)
         {
             agreement = new NHAgreement();
@@ -67,19 +69,25 @@ protected Key engineDoPhase(Key key, boolean lastPhase)
 
             shared = exchPair.getSharedValue();
 
+            agreementCompleted();
+
             return new BCNHPublicKey((NHPublicKeyParameters)exchPair.getPublicKey());
         }
         else
         {
             shared = agreement.calculateAgreement(otherPartyKey.getKeyParams());
 
+            agreementCompleted();
+
             return null;
         }
     }
 
     protected byte[] engineGenerateSecret()
         throws IllegalStateException
     {
+        checkAgreementCompleted();
+
         byte[] rv = Arrays.clone(shared);
 
         Arrays.fill(shared, (byte)0);
@@ -90,6 +98,8 @@ protected byte[] engineGenerateSecret()
     protected int engineGenerateSecret(byte[] bytes, int offset)
         throws IllegalStateException, ShortBufferException
     {
+        checkAgreementCompleted();
+
         System.arraycopy(shared, 0, bytes, offset, shared.length);
 
         Arrays.fill(shared, (byte)0);