TLS: Add negotiatedGroup property to SecurityParameters (TLS 1.3+)

peterdettman · peterdettman · commit 8d339efe835b · 2025-12-03T23:31:58.000+07:00
- see #2117
diff --git a/tls/src/main/java/org/bouncycastle/tls/SecurityParameters.java b/tls/src/main/java/org/bouncycastle/tls/SecurityParameters.java
@@ -55,6 +55,7 @@ public class SecurityParameters
     Certificate localCertificate = null;
     Certificate peerCertificate = null;
     ProtocolVersion negotiatedVersion = null;
+    int negotiatedGroup = -1;
     int statusRequestVersion = 0;
     short clientCertificateType = CertificateType.X509;
     short serverCertificateType = CertificateType.X509;
@@ -372,6 +373,17 @@ public ProtocolVersion getNegotiatedVersion()
         return negotiatedVersion;
     }
 
+    /**
+     * The named group selected by the server for key exchange (if any), or -1 when not negotiated.
+     * <br/>
+     * See {@link NamedGroup} for group constants. Currently not set (i.e. -1) when pre-1.3 version negotiated. Not all
+     * TLS 1.3 key exchanges negotiate a group (e.g. PskKeyExchangeMode.psk_ke).
+     */
+    public int getNegotiatedGroup()
+    {
+        return negotiatedGroup;
+    }
+
     public int getStatusRequestVersion()
     {
         return statusRequestVersion;
diff --git a/tls/src/main/java/org/bouncycastle/tls/TlsClientProtocol.java b/tls/src/main/java/org/bouncycastle/tls/TlsClientProtocol.java
@@ -956,6 +956,8 @@ protected void process13HelloRetryRequest(ServerHello helloRetryRequest)
         TlsUtils.negotiatedCipherSuite(securityParameters, cipherSuite);
         tlsClient.notifySelectedCipherSuite(cipherSuite);
 
+        securityParameters.negotiatedGroup = selectedGroup;
+
         this.clientAgreements = null;
         this.retryCookie = cookie;
         this.retryGroup = selectedGroup;
@@ -1066,17 +1068,17 @@ protected void process13ServerHello(ServerHello serverHello, boolean afterHelloR
             KeyShareEntry serverShare = TlsExtensionsUtils.getKeyShareServerHello(extensions);
             if (null == serverShare)
             {
-                if (afterHelloRetryRequest
-                    || null == pskEarlySecret
-                    || !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_ke))
+                if (afterHelloRetryRequest ||
+                    pskEarlySecret == null ||
+                    !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_ke))
                 {
                     throw new TlsFatalAlert(AlertDescription.illegal_parameter);
                 }
             }
             else
             {
-                if (null != pskEarlySecret
-                    && !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_dhe_ke))
+                if (pskEarlySecret != null &&
+                    !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_dhe_ke))
                 {
                     throw new TlsFatalAlert(AlertDescription.illegal_parameter);
                 }
@@ -1091,6 +1093,11 @@ protected void process13ServerHello(ServerHello serverHello, boolean afterHelloR
 
                 agreement.receivePeerValue(serverShare.getKeyExchange());
                 sharedSecret = agreement.calculateSecret();
+
+                if (!afterHelloRetryRequest)
+                {
+                    securityParameters.negotiatedGroup = namedGroup;                    
+                }
             }
         }
 
diff --git a/tls/src/main/java/org/bouncycastle/tls/TlsProtocol.java b/tls/src/main/java/org/bouncycastle/tls/TlsProtocol.java
@@ -158,6 +158,7 @@ protected boolean isTLSv13ConnectionState()
     protected TlsSecret sessionMasterSecret = null;
 
     protected byte[] retryCookie = null;
+    // TODO[api] Remove and manage via SecurityParameters.negotiatedGroup    
     protected int retryGroup = -1;
     protected Hashtable clientExtensions = null;
     protected Hashtable serverExtensions = null;
diff --git a/tls/src/main/java/org/bouncycastle/tls/TlsServerProtocol.java b/tls/src/main/java/org/bouncycastle/tls/TlsServerProtocol.java
@@ -302,6 +302,8 @@ protected ServerHello generate13ServerHello(ClientHello clientHello, HandshakeMe
                 throw new TlsFatalAlert(AlertDescription.handshake_failure);
             }
 
+            securityParameters.negotiatedGroup = selectedGroup;
+
             clientShare = TlsUtils.findEarlyKeyShare(clientShares, selectedGroup);
 
             if (null == clientShare)
@@ -332,7 +334,7 @@ protected ServerHello generate13ServerHello(ClientHello clientHello, HandshakeMe
             int[] serverSupportedGroups = securityParameters.getServerSupportedGroups();
 
             if (!TlsUtils.isNullOrEmpty(serverSupportedGroups) &&
-                clientShare.getNamedGroup() != serverSupportedGroups[0] &&
+                serverSupportedGroups[0] != securityParameters.getNegotiatedGroup() &&
                 !serverEncryptedExtensions.containsKey(TlsExtensionsUtils.EXT_supported_groups))
             {
                 TlsExtensionsUtils.addSupportedGroupsExtension(serverEncryptedExtensions, serverSupportedGroups);
@@ -397,9 +399,14 @@ protected ServerHello generate13ServerHello(ClientHello clientHello, HandshakeMe
 
         TlsSecret sharedSecret;
         {
-            int namedGroup = clientShare.getNamedGroup();
+            int negotiatedGroup = securityParameters.getNegotiatedGroup();
+
+            if (clientShare.getNamedGroup() != negotiatedGroup)
+            {
+                throw new TlsFatalAlert(AlertDescription.illegal_parameter);
+            }
 
-            TlsAgreement agreement = TlsUtils.createKeyShare(crypto, namedGroup, true);
+            TlsAgreement agreement = TlsUtils.createKeyShare(crypto, negotiatedGroup, true);
             if (agreement == null)
             {
                 throw new TlsFatalAlert(AlertDescription.internal_error);
@@ -408,7 +415,7 @@ protected ServerHello generate13ServerHello(ClientHello clientHello, HandshakeMe
             agreement.receivePeerValue(clientShare.getKeyExchange());
 
             byte[] key_exchange = agreement.generateEphemeral();
-            KeyShareEntry serverShare = new KeyShareEntry(namedGroup, key_exchange);
+            KeyShareEntry serverShare = new KeyShareEntry(negotiatedGroup, key_exchange);
             TlsExtensionsUtils.addKeyShareServerHello(serverHelloExtensions, serverShare);
 
             sharedSecret = agreement.calculateSecret();
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/MockPSKTls13Client.java b/tls/src/test/java/org/bouncycastle/tls/test/MockPSKTls13Client.java
@@ -10,9 +10,11 @@
 import org.bouncycastle.tls.AlertLevel;
 import org.bouncycastle.tls.BasicTlsPSKExternal;
 import org.bouncycastle.tls.CipherSuite;
+import org.bouncycastle.tls.NamedGroup;
 import org.bouncycastle.tls.PRFAlgorithm;
 import org.bouncycastle.tls.ProtocolName;
 import org.bouncycastle.tls.ProtocolVersion;
+import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.TlsAuthentication;
 import org.bouncycastle.tls.TlsFatalAlert;
 import org.bouncycastle.tls.TlsPSK;
@@ -121,11 +123,19 @@ public void notifyHandshakeComplete() throws IOException
     {
         super.notifyHandshakeComplete();
 
-        ProtocolName protocolName = context.getSecurityParametersConnection().getApplicationProtocol();
+        SecurityParameters securityParameters = context.getSecurityParametersConnection();
+
+        ProtocolName protocolName = securityParameters.getApplicationProtocol();
         if (protocolName != null)
         {
             System.out.println("Client ALPN: " + protocolName.getUtf8Decoding());
         }
+
+        int negotiatedGroup = securityParameters.getNegotiatedGroup();
+        if (negotiatedGroup >= 0)
+        {
+            System.out.println("Client negotiated group: " + NamedGroup.getText(negotiatedGroup));
+        }
     }
 
     public Hashtable getClientExtensions() throws IOException
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/MockPSKTls13Server.java b/tls/src/test/java/org/bouncycastle/tls/test/MockPSKTls13Server.java
@@ -10,10 +10,12 @@
 import org.bouncycastle.tls.AlertLevel;
 import org.bouncycastle.tls.BasicTlsPSKExternal;
 import org.bouncycastle.tls.CipherSuite;
+import org.bouncycastle.tls.NamedGroup;
 import org.bouncycastle.tls.PRFAlgorithm;
 import org.bouncycastle.tls.ProtocolName;
 import org.bouncycastle.tls.ProtocolVersion;
 import org.bouncycastle.tls.PskIdentity;
+import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.TlsCredentials;
 import org.bouncycastle.tls.TlsFatalAlert;
 import org.bouncycastle.tls.TlsPSKExternal;
@@ -119,11 +121,19 @@ public void notifyHandshakeComplete() throws IOException
     {
         super.notifyHandshakeComplete();
 
-        ProtocolName protocolName = context.getSecurityParametersConnection().getApplicationProtocol();
+        SecurityParameters securityParameters = context.getSecurityParametersConnection();
+
+        ProtocolName protocolName = securityParameters.getApplicationProtocol();
         if (protocolName != null)
         {
             System.out.println("Server ALPN: " + protocolName.getUtf8Decoding());
         }
+
+        int negotiatedGroup = securityParameters.getNegotiatedGroup();
+        if (negotiatedGroup >= 0)
+        {
+            System.out.println("Server negotiated group: " + NamedGroup.getText(negotiatedGroup));
+        }
     }
 
     public void processClientExtensions(Hashtable clientExtensions) throws IOException
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/MockTlsHybridClient.java b/tls/src/test/java/org/bouncycastle/tls/test/MockTlsHybridClient.java
@@ -17,6 +17,7 @@
 import org.bouncycastle.tls.NamedGroupRole;
 import org.bouncycastle.tls.ProtocolName;
 import org.bouncycastle.tls.ProtocolVersion;
+import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.SignatureAlgorithm;
 import org.bouncycastle.tls.TlsAuthentication;
 import org.bouncycastle.tls.TlsCredentials;
@@ -187,12 +188,20 @@ public void notifyHandshakeComplete() throws IOException
     {
         super.notifyHandshakeComplete();
 
-        ProtocolName protocolName = context.getSecurityParametersConnection().getApplicationProtocol();
+        SecurityParameters securityParameters = context.getSecurityParametersConnection();
+
+        ProtocolName protocolName = securityParameters.getApplicationProtocol();
         if (protocolName != null)
         {
             System.out.println("Client ALPN: " + protocolName.getUtf8Decoding());
         }
 
+        int negotiatedGroup = securityParameters.getNegotiatedGroup();
+        if (negotiatedGroup >= 0)
+        {
+            System.out.println("Client negotiated group: " + NamedGroup.getText(negotiatedGroup));
+        }
+
         TlsSession newSession = context.getSession();
         if (newSession != null)
         {
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/MockTlsHybridServer.java b/tls/src/test/java/org/bouncycastle/tls/test/MockTlsHybridServer.java
@@ -16,6 +16,7 @@
 import org.bouncycastle.tls.NamedGroup;
 import org.bouncycastle.tls.ProtocolName;
 import org.bouncycastle.tls.ProtocolVersion;
+import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.SignatureAlgorithm;
 import org.bouncycastle.tls.TlsCredentialedDecryptor;
 import org.bouncycastle.tls.TlsCredentialedSigner;
@@ -181,12 +182,20 @@ public void notifyHandshakeComplete() throws IOException
     {
         super.notifyHandshakeComplete();
 
-        ProtocolName protocolName = context.getSecurityParametersConnection().getApplicationProtocol();
+        SecurityParameters securityParameters = context.getSecurityParametersConnection();
+
+        ProtocolName protocolName = securityParameters.getApplicationProtocol();
         if (protocolName != null)
         {
             System.out.println("Server ALPN: " + protocolName.getUtf8Decoding());
         }
 
+        int negotiatedGroup = securityParameters.getNegotiatedGroup();
+        if (negotiatedGroup >= 0)
+        {
+            System.out.println("Server negotiated group: " + NamedGroup.getText(negotiatedGroup));
+        }
+
         byte[] tlsServerEndPoint = context.exportChannelBinding(ChannelBinding.tls_server_end_point);
         System.out.println("Server 'tls-server-end-point': " + hex(tlsServerEndPoint));
 
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/MockTlsKemClient.java b/tls/src/test/java/org/bouncycastle/tls/test/MockTlsKemClient.java
@@ -17,6 +17,7 @@
 import org.bouncycastle.tls.NamedGroupRole;
 import org.bouncycastle.tls.ProtocolName;
 import org.bouncycastle.tls.ProtocolVersion;
+import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.SignatureAlgorithm;
 import org.bouncycastle.tls.TlsAuthentication;
 import org.bouncycastle.tls.TlsCredentials;
@@ -187,12 +188,20 @@ public void notifyHandshakeComplete() throws IOException
     {
         super.notifyHandshakeComplete();
 
-        ProtocolName protocolName = context.getSecurityParametersConnection().getApplicationProtocol();
+        SecurityParameters securityParameters = context.getSecurityParametersConnection();
+
+        ProtocolName protocolName = securityParameters.getApplicationProtocol();
         if (protocolName != null)
         {
             System.out.println("Client ALPN: " + protocolName.getUtf8Decoding());
         }
 
+        int negotiatedGroup = securityParameters.getNegotiatedGroup();
+        if (negotiatedGroup >= 0)
+        {
+            System.out.println("Client negotiated group: " + NamedGroup.getText(negotiatedGroup));
+        }
+        
         TlsSession newSession = context.getSession();
         if (newSession != null)
         {
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/MockTlsKemServer.java b/tls/src/test/java/org/bouncycastle/tls/test/MockTlsKemServer.java
@@ -16,6 +16,7 @@
 import org.bouncycastle.tls.NamedGroup;
 import org.bouncycastle.tls.ProtocolName;
 import org.bouncycastle.tls.ProtocolVersion;
+import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.SignatureAlgorithm;
 import org.bouncycastle.tls.TlsCredentialedDecryptor;
 import org.bouncycastle.tls.TlsCredentialedSigner;
@@ -181,12 +182,20 @@ public void notifyHandshakeComplete() throws IOException
     {
         super.notifyHandshakeComplete();
 
-        ProtocolName protocolName = context.getSecurityParametersConnection().getApplicationProtocol();
+        SecurityParameters securityParameters = context.getSecurityParametersConnection();
+
+        ProtocolName protocolName = securityParameters.getApplicationProtocol();
         if (protocolName != null)
         {
             System.out.println("Server ALPN: " + protocolName.getUtf8Decoding());
         }
 
+        int negotiatedGroup = securityParameters.getNegotiatedGroup();
+        if (negotiatedGroup >= 0)
+        {
+            System.out.println("Server negotiated group: " + NamedGroup.getText(negotiatedGroup));
+        }
+
         byte[] tlsServerEndPoint = context.exportChannelBinding(ChannelBinding.tls_server_end_point);
         System.out.println("Server 'tls-server-end-point': " + hex(tlsServerEndPoint));
 
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/TlsTestClientImpl.java b/tls/src/test/java/org/bouncycastle/tls/test/TlsTestClientImpl.java
@@ -21,6 +21,7 @@
 import org.bouncycastle.tls.ClientCertificateType;
 import org.bouncycastle.tls.ConnectionEnd;
 import org.bouncycastle.tls.DefaultTlsClient;
+import org.bouncycastle.tls.NamedGroup;
 import org.bouncycastle.tls.ProtocolVersion;
 import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.SignatureAlgorithm;
@@ -201,6 +202,12 @@ public void notifyHandshakeComplete() throws IOException
 
         if (TlsTestConfig.DEBUG)
         {
+            int negotiatedGroup = securityParameters.getNegotiatedGroup();
+            if (negotiatedGroup >= 0)
+            {
+                System.out.println("TLS client negotiated group: " + NamedGroup.getText(negotiatedGroup));
+            }
+
             System.out.println("TLS client reports 'tls-server-end-point' = " + hex(tlsServerEndPoint));
             System.out.println("TLS client reports 'tls-unique' = " + hex(tlsUnique));
         }
diff --git a/tls/src/test/java/org/bouncycastle/tls/test/TlsTestServerImpl.java b/tls/src/test/java/org/bouncycastle/tls/test/TlsTestServerImpl.java
@@ -15,6 +15,7 @@
 import org.bouncycastle.tls.ClientCertificateType;
 import org.bouncycastle.tls.ConnectionEnd;
 import org.bouncycastle.tls.DefaultTlsServer;
+import org.bouncycastle.tls.NamedGroup;
 import org.bouncycastle.tls.ProtocolVersion;
 import org.bouncycastle.tls.SecurityParameters;
 import org.bouncycastle.tls.SignatureAlgorithm;
@@ -160,6 +161,12 @@ public void notifyHandshakeComplete() throws IOException
 
         if (TlsTestConfig.DEBUG)
         {
+            int negotiatedGroup = securityParameters.getNegotiatedGroup();
+            if (negotiatedGroup >= 0)
+            {
+                System.out.println("TLS server negotiated group: " + NamedGroup.getText(negotiatedGroup));
+            }
+
             System.out.println("TLS server reports 'tls-server-end-point' = " + hex(tlsServerEndPoint));
             System.out.println("TLS server reports 'tls-unique' = " + hex(tlsUnique));
         }

Original file line number	Diff line number	Diff line change
`@@ -956,6 +956,8 @@ protected void process13HelloRetryRequest(ServerHello helloRetryRequest)`
`956`	`956`	`TlsUtils.negotiatedCipherSuite(securityParameters, cipherSuite);`
`957`	`957`	`tlsClient.notifySelectedCipherSuite(cipherSuite);`
`958`	`958`
	`959`	`+ securityParameters.negotiatedGroup = selectedGroup;`
	`960`	`+`
`959`	`961`	`this.clientAgreements = null;`
`960`	`962`	`this.retryCookie = cookie;`
`961`	`963`	`this.retryGroup = selectedGroup;`
`@@ -1066,17 +1068,17 @@ protected void process13ServerHello(ServerHello serverHello, boolean afterHelloR`
`1066`	`1068`	`KeyShareEntry serverShare = TlsExtensionsUtils.getKeyShareServerHello(extensions);`
`1067`	`1069`	`if (null == serverShare)`
`1068`	`1070`	`{`
`1069`		`- if (afterHelloRetryRequest`
`1070`		`- \|\| null == pskEarlySecret`
`1071`		`- \|\| !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_ke))`
	`1071`	`+ if (afterHelloRetryRequest \|\|`
	`1072`	`+ pskEarlySecret == null \|\|`
	`1073`	`+ !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_ke))`
`1072`	`1074`	`{`
`1073`	`1075`	`throw new TlsFatalAlert(AlertDescription.illegal_parameter);`
`1074`	`1076`	`}`
`1075`	`1077`	`}`
`1076`	`1078`	`else`
`1077`	`1079`	`{`
`1078`		`- if (null != pskEarlySecret`
`1079`		`- && !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_dhe_ke))`
	`1080`	`+ if (pskEarlySecret != null &&`
	`1081`	`+ !Arrays.contains(clientBinders.pskKeyExchangeModes, PskKeyExchangeMode.psk_dhe_ke))`
`1080`	`1082`	`{`
`1081`	`1083`	`throw new TlsFatalAlert(AlertDescription.illegal_parameter);`
`1082`	`1084`	`}`
`@@ -1091,6 +1093,11 @@ protected void process13ServerHello(ServerHello serverHello, boolean afterHelloR`
`1091`	`1093`
`1092`	`1094`	`agreement.receivePeerValue(serverShare.getKeyExchange());`
`1093`	`1095`	`sharedSecret = agreement.calculateSecret();`
	`1096`	`+`
	`1097`	`+ if (!afterHelloRetryRequest)`
	`1098`	`+ {`
	`1099`	`+ securityParameters.negotiatedGroup = namedGroup;`
	`1100`	`+ }`
`1094`	`1101`	`}`
`1095`	`1102`	`}`
`1096`	`1103`