fix(keyshare): keep the private key of every classical key share

key_schedule.go

@@ -57,6 +57,11 @@ type keySharePrivateKeys struct {
 	mlkem crypto.Decapsulator
 
 	mlkemEcdhe *ecdh.PrivateKey // [uTLS] seperate ecdhe key for pq keyshare in line with Chrome, instead of reusing ecdhe key like stdlib
+
+	// ecdhes holds the key of every classical share a custom ClientHello
+	// offered, by group, when it offered more than one; ecdhe is the first.
+	// Firefox offers X25519 and P-256, and the server may select either. [uTLS]
+	ecdhes map[CurveID]*ecdh.PrivateKey
 }
 
 // A keyExchange implements a TLS 1.3 KEM.
@@ -196,6 +201,8 @@ func (ke *ecdhKeyExchange) clientSharedSecretChooseEcdhe(priv *keySharePrivateKe
 			return nil, errors.New("tls: missing separate hybrid ECDHE private key")
 		}
 		ecdhe = priv.mlkemEcdhe
+	} else if key := priv.ecdhes[ke.id]; key != nil {
+		ecdhe = key
 	}
 	if ecdhe == nil {
 		return nil, errors.New("tls: missing ECDHE private key")

u_key_shares_test.go

@@ -0,0 +1,69 @@
+package tls
+
+import (
+	"fmt"
+	"testing"
+)
+
+// classicalKeyShareGroups lists the groups of a spec's classical key shares,
+// the ones a server may select without a HelloRetryRequest.
+func classicalKeyShareGroups(spec *ClientHelloSpec) []CurveID {
+	var groups []CurveID
+	for _, ext := range spec.Extensions {
+		if ks, ok := ext.(*KeyShareExtension); ok {
+			for _, share := range ks.KeyShares {
+				switch share.Group {
+				case X25519, CurveP256, CurveP384, CurveP521:
+					groups = append(groups, share.Group)
+				}
+			}
+		}
+	}
+	return groups
+}
+
+// TestUTLSEveryClassicalKeyShareIsUsable checks that a server may select any
+// of the classical key shares a spec offers, not only the first: Firefox
+// offers X25519 and P-256 shares, and a server preferring P-256 completes the
+// handshake with it, without a HelloRetryRequest.
+func TestUTLSEveryClassicalKeyShareIsUsable(t *testing.T) {
+	specs := map[string]*ClientHelloSpec{}
+	for _, id := range []ClientHelloID{HelloFirefox_105, HelloFirefox_120, HelloFirefox_148} {
+		spec, err := utlsIdToSpec(id)
+		if err != nil {
+			t.Fatal(err)
+		}
+		specs[id.Str()] = &spec
+	}
+	specs["three classical shares"] = &ClientHelloSpec{
+		CipherSuites: []uint16{TLS_AES_128_GCM_SHA256},
+		Extensions: []TLSExtension{
+			&SNIExtension{},
+			&SupportedVersionsExtension{Versions: []uint16{VersionTLS13}},
+			&SupportedCurvesExtension{Curves: []CurveID{X25519, CurveP256, CurveP384}},
+			&SignatureAlgorithmsExtension{SupportedSignatureAlgorithms: []SignatureScheme{ECDSAWithP256AndSHA256}},
+			&KeyShareExtension{KeyShares: []KeyShare{{Group: X25519}, {Group: CurveP256}, {Group: CurveP384}}},
+		},
+	}
+
+	for name, spec := range specs {
+		groups := classicalKeyShareGroups(spec)
+		if len(groups) < 2 {
+			t.Fatalf("%s: offers %v classical key shares, want at least two", name, groups)
+		}
+		for _, group := range groups {
+			t.Run(fmt.Sprintf("%s/%v", name, group), func(t *testing.T) {
+				clientConfig := testConfigClient.Clone()
+				serverConfig := testConfigServer.Clone()
+				serverConfig.CurvePreferences = []CurveID{group}
+				_, state, err := testUtlsHandshake(t, clientConfig, serverConfig, spec)
+				if err != nil {
+					t.Fatalf("server selecting %v: %v", group, err)
+				}
+				if state.CurveID != group || state.HelloRetryRequest {
+					t.Fatalf("negotiated %v with HelloRetryRequest=%v, want %v without a retry", state.CurveID, state.HelloRetryRequest, group)
+				}
+			})
+		}
+	}
+}

u_parrots.go

@@ -3084,6 +3084,21 @@ func (uconn *UConn) applyPresetByID(id ClientHelloID) (err error) {
 
 // ApplyPreset should only be used in conjunction with HelloCustom to apply custom specs.
 // The provided ClientHelloSpec is cloned before per-connection handshake state is applied.
+// keepClassicalKeyShareKey records the private key of a classical key share
+// the ClientHello offers: the first one as KeyShareKeys.Ecdhe, as before, and
+// every one by group in KeyShareKeys.Ecdhes, so that the server may select
+// any of them.
+func (uconn *UConn) keepClassicalKeyShareKey(group CurveID, key *ecdh.PrivateKey) {
+	keys := uconn.HandshakeState.State13.KeyShareKeys
+	if keys.Ecdhe == nil {
+		keys.Ecdhe = key
+	}
+	if keys.Ecdhes == nil {
+		keys.Ecdhes = make(map[CurveID]*ecdh.PrivateKey)
+	}
+	keys.Ecdhes[group] = key
+}
+
 func (uconn *UConn) ApplyPreset(p *ClientHelloSpec) error {
 	var err error
 	p = p.clone()
@@ -3216,7 +3231,6 @@ func (uconn *UConn) ApplyPreset(p *ClientHelloSpec) error {
 				}
 			}
 		case *KeyShareExtension:
-			preferredCurveIsSet := false
 			reusableClassicalKeys := make(map[CurveID][]*ecdh.PrivateKey)
 			for i := range ext.KeyShares {
 				curveID := ext.KeyShares[i].Group
@@ -3293,11 +3307,7 @@ func (uconn *UConn) ApplyPreset(p *ClientHelloSpec) error {
 						reusableClassicalKeys[curveID] = keysForCurve[1:]
 
 						ext.KeyShares[i].Data = reusedKey.PublicKey().Bytes()
-						if !preferredCurveIsSet {
-							// only do this once for the first non-grease curve
-							uconn.HandshakeState.State13.KeyShareKeys.Ecdhe = reusedKey
-							preferredCurveIsSet = true
-						}
+						uconn.keepClassicalKeyShareKey(curveID, reusedKey)
 						continue
 					}
 
@@ -3308,11 +3318,7 @@ func (uconn *UConn) ApplyPreset(p *ClientHelloSpec) error {
 					}
 
 					ext.KeyShares[i].Data = ecdheKey.PublicKey().Bytes()
-					if !preferredCurveIsSet {
-						// only do this once for the first non-grease curve
-						uconn.HandshakeState.State13.KeyShareKeys.Ecdhe = ecdheKey
-						preferredCurveIsSet = true
-					}
+					uconn.keepClassicalKeyShareKey(curveID, ecdheKey)
 				}
 			}
 		case *SupportedVersionsExtension:

u_public.go

@@ -928,6 +928,10 @@ type KeySharePrivateKeys struct {
 	Ecdhe      *ecdh.PrivateKey
 	Mlkem      crypto.Decapsulator
 	MlkemEcdhe *ecdh.PrivateKey
+	// Ecdhes holds the key of every classical share the ClientHello offers,
+	// by group, so that the server may select any of them; Ecdhe is the
+	// first one.
+	Ecdhes map[CurveID]*ecdh.PrivateKey
 }
 
 func (ksp *KeySharePrivateKeys) ToPrivate() *keySharePrivateKeys {
@@ -939,6 +943,7 @@ func (ksp *KeySharePrivateKeys) ToPrivate() *keySharePrivateKeys {
 		ecdhe:      ksp.Ecdhe,
 		mlkem:      ksp.Mlkem,
 		mlkemEcdhe: ksp.MlkemEcdhe,
+		ecdhes:     ksp.Ecdhes,
 	}
 }
 
@@ -951,5 +956,6 @@ func (ksp *keySharePrivateKeys) ToPublic() *KeySharePrivateKeys {
 		Ecdhe:      ksp.ecdhe,
 		Mlkem:      ksp.mlkem,
 		MlkemEcdhe: ksp.mlkemEcdhe,
+		Ecdhes:     ksp.ecdhes,
 	}
 }