Fix nits

Author: jhoyla

src/crypto/tls/common.go

@@ -479,10 +479,18 @@ const (
 	ECDSAWithSHA1 SignatureScheme = 0x0203
 )
 
+// A TLSFlag is the index of the bit in the TLSFlags bit array that should be
+// set to send the flag. Because TLSFlags can have so many different uses, many
+// yet to be defined, this extension should not be copied into the ECH
+// ClientHelloOuter
 type TLSFlag uint16
 
 const (
-	FlagSupportMTLS TLSFlag = 0x50
+	// ExperimentalFlagSupportMTLS is a flag that signals that a client
+	// supports mTLS, and will be able to respond to CertificateRequest
+	// messages appropriately.
+	// https://datatracker.ietf.org/doc/draft-jhoyla-req-mtls-flag/
+	ExperimentalFlagSupportMTLS TLSFlag = 0x50
 )
 
 // ClientHelloInfo contains information from a ClientHello message in order to
@@ -921,6 +929,10 @@ type Config struct {
 	// See https://tools.ietf.org/html/draft-ietf-tls-subcerts.
 	SupportDelegatedCredential bool
 
+	// TLSFlagsSupported is the list of flags that the client or server is
+	// willing to support. This is currently limited to the set of flags set in
+	// the ClientHello, although the draft specifies various other messages
+	// where they can appear.
 	TLSFlagsSupported []TLSFlag
 
 	// mutex protects sessionTicketKeys and autoSessionTicketKeys.

src/crypto/tls/handshake_messages.go

@@ -591,13 +591,7 @@ func (m *clientHelloMsg) unmarshal(data []byte) bool {
 			if !extData.ReadUint8LengthPrefixed(&flagsList) || flagsList.Empty() {
 				return false
 			}
-			for !flagsList.Empty() {
-				var flagByte uint8
-				if !flagsList.ReadUint8(&flagByte) {
-					return false
-				}
-				m.tlsFlags = append(m.tlsFlags, flagByte)
-			}
+			m.tlsFlags = flagsList
 		case extensionCookie:
 			// RFC 8446, Section 4.2.2
 			if !readUint16LengthPrefixed(&extData, &m.cookie) ||

src/crypto/tls/handshake_server_test.go

@@ -539,6 +539,41 @@ func testCrossVersionResume(t *testing.T, version uint16) {
 	}
 }
 
+func TestTLSFlags(t *testing.T) {
+	serverConfig := &Config{
+		Certificates: []Certificate{{
+			Certificate: [][]byte{testRSACertificate},
+			PrivateKey:  testRSAPrivateKey,
+		}},
+		TLSFlagsSupported: []TLSFlag{0x50},
+	}
+	clientCert, err := X509KeyPair([]byte(clientECDSACertificatePEM), []byte(clientECDSAKeyPEM))
+	if err != nil {
+		t.Fatalf("couldn't load client certs")
+	}
+	clientConfig := &Config{
+		TLSFlagsSupported:  []TLSFlag{0x50},
+		InsecureSkipVerify: true,
+		Certificates:       []Certificate{clientCert},
+	}
+	state, _, err := testHandshake(t, clientConfig, serverConfig)
+	if err != nil {
+		t.Fatalf("handshake failed: %s", err)
+	}
+	if state.PeerTLSFlags[0] != TLSFlag(0x50) {
+		t.Fatalf("Received wrong flags")
+	}
+	if state.AgreedTLSFlags[0] != TLSFlag(0x50) {
+		t.Fatalf("Failed to agree correct flags")
+	}
+	if !state.RequestClientCert {
+		t.Fatalf("Failed to request client cert")
+	}
+	if len(state.PeerCertificates) == 0 {
+		t.Fatalf("Didn't receive correct client certs")
+	}
+}
+
 // Note: see comment in handshake_test.go for details of how the reference
 // tests work.
 

src/crypto/tls/handshake_server_tls13.go

@@ -322,8 +322,13 @@ GroupSelection:
 		return errors.New("tls: invalid client key share")
 	}
 	if len(hs.clientHello.tlsFlags) != 0 {
+		if hs.clientHello.tlsFlags[len(hs.clientHello.tlsFlags)-1] == 0 {
+			c.sendAlert(alertIllegalParameter)
+			return errors.New("tls: invalid client TLS flags")
+		}
 		supportedFlags, err := encodeFlags(hs.c.config.TLSFlagsSupported)
 		if err != nil {
+			c.sendAlert(alertInternalError)
 			return errors.New("tls: invalid server flags")
 		}
 		var mutuallySupportedFlags []byte
@@ -385,13 +390,14 @@ GroupSelection:
 
 func decodeFlags(flagBytes []byte) ([]TLSFlag, error) {
 	var flags []TLSFlag
+	if len(flagBytes) > int(maxTLSFlag>>3) {
+		return nil, fmt.Errorf("TLS flags extension malformed (too long)")
+	}
+
 	for byteIndex, b := range flagBytes {
 		for i := 0; !(b == 0); i++ {
 			if (b & 1) == 1 {
 				flagNo := byteIndex*8 + i
-				if flagNo >= int(maxTLSFlag) {
-					return nil, fmt.Errorf("TLS flag is out of range: %d", flagNo)
-				}
 				flags = append(flags, TLSFlag(flagNo))
 			}
 			b >>= 1
@@ -937,7 +943,7 @@ func (hs *serverHandshakeStateTLS13) sendServerParameters() error {
 
 func (hs *serverHandshakeStateTLS13) requestClientCert() bool {
 	for _, flag := range hs.tlsFlags {
-		if flag == FlagSupportMTLS {
+		if flag == ExperimentalFlagSupportMTLS {
 			return true
 		}
 	}