TLS: negotiate group before looking for early share

Author: peterdettman

tls/src/main/java/org/bouncycastle/tls/TlsServerProtocol.java

@@ -10,9 +10,6 @@
 
 import org.bouncycastle.tls.crypto.TlsAgreement;
 import org.bouncycastle.tls.crypto.TlsCrypto;
-import org.bouncycastle.tls.crypto.TlsDHConfig;
-import org.bouncycastle.tls.crypto.TlsECConfig;
-import org.bouncycastle.tls.crypto.TlsKemConfig;
 import org.bouncycastle.tls.crypto.TlsSecret;
 import org.bouncycastle.util.Arrays;
 
@@ -219,7 +216,7 @@ protected ServerHello generate13ServerHello(ClientHello clientHello, HandshakeMe
             }
             this.retryCookie = null;
 
-            clientShare = TlsUtils.selectKeyShare(clientShares, retryGroup);
+            clientShare = TlsUtils.getRetryKeyShare(clientShares, retryGroup);
             if (null == clientShare)
             {
                 throw new TlsFatalAlert(AlertDescription.illegal_parameter);
@@ -297,17 +294,18 @@ protected ServerHello generate13ServerHello(ClientHello clientHello, HandshakeMe
             int[] clientSupportedGroups = securityParameters.getClientSupportedGroups();
             int[] serverSupportedGroups = securityParameters.getServerSupportedGroups();
 
-            clientShare = TlsUtils.selectKeyShare(crypto, serverVersion, clientShares, clientSupportedGroups,
+            int selectedGroup = TlsUtils.selectKeyShareGroup(crypto, serverVersion, clientSupportedGroups,
                 serverSupportedGroups);
+            if (selectedGroup < 0)
+            {
+                throw new TlsFatalAlert(AlertDescription.handshake_failure);
+            }
+
+            clientShare = TlsUtils.findEarlyKeyShare(clientShares, selectedGroup);
 
             if (null == clientShare)
             {
-                this.retryGroup = TlsUtils.selectKeyShareGroup(crypto, serverVersion, clientSupportedGroups,
-                    serverSupportedGroups);
-                if (retryGroup < 0)
-                {
-                    throw new TlsFatalAlert(AlertDescription.handshake_failure);
-                }
+                this.retryGroup = selectedGroup;
 
                 this.retryCookie = tlsServerContext.getNonceGenerator().generateNonce(16);
 

tls/src/main/java/org/bouncycastle/tls/TlsUtils.java

@@ -5449,37 +5449,30 @@ else if (NamedGroup.refersToASpecificKem(keyShareGroup))
         return null;
     }
 
-    static KeyShareEntry selectKeyShare(Vector clientShares, int keyShareGroup)
+    static KeyShareEntry findEarlyKeyShare(Vector clientShares, int keyShareGroup)
     {
-        if (null != clientShares && 1 == clientShares.size())
+        if (null != clientShares)
         {
-            KeyShareEntry clientShare = (KeyShareEntry)clientShares.elementAt(0);
-            if (null != clientShare && clientShare.getNamedGroup() == keyShareGroup)
+            for (int i = 0; i < clientShares.size(); ++i)
             {
-                return clientShare;
+                KeyShareEntry clientShare = (KeyShareEntry)clientShares.elementAt(i);
+                if (null != clientShare && clientShare.getNamedGroup() == keyShareGroup)
+                {
+                    return clientShare;
+                }
             }
         }
         return null;
     }
 
-    static KeyShareEntry selectKeyShare(TlsCrypto crypto, ProtocolVersion negotiatedVersion, Vector clientShares,
-        int[] clientSupportedGroups, int[] serverSupportedGroups)
+    static KeyShareEntry getRetryKeyShare(Vector clientShares, int keyShareGroup)
     {
-        if (null != clientShares && !isNullOrEmpty(clientSupportedGroups) && !isNullOrEmpty(serverSupportedGroups))
+        if (null != clientShares && 1 == clientShares.size())
         {
-            for (int i = 0; i < clientShares.size(); ++i)
+            KeyShareEntry clientShare = (KeyShareEntry)clientShares.elementAt(0);
+            if (null != clientShare && clientShare.getNamedGroup() == keyShareGroup)
             {
-                KeyShareEntry clientShare = (KeyShareEntry)clientShares.elementAt(i);
-
-                int group = clientShare.getNamedGroup();
-
-                if (NamedGroup.canBeNegotiated(group, negotiatedVersion) &&
-                    Arrays.contains(serverSupportedGroups, group) &&
-                    Arrays.contains(clientSupportedGroups, group) &&
-                    supportsKeyShareGroup(crypto, group))
-                {
-                    return clientShare;
-                }
+                return clientShare;
             }
         }
         return null;