crypto/tls: define api for delegated credentials so they are fetched using the same mechanisms used to fetch certificates #67 (#69)

Refactor new API

Address comments from review

Address comments from review 2

Address comments from review 3

Author: claucece

src/crypto/tls/auth.go

@@ -196,8 +196,6 @@ var rsaSignatureSchemes = []struct {
 // and optionally filtered by its explicit SupportedSignatureAlgorithms.
 //
 // This function must be kept in sync with supportedSignatureAlgorithms.
-// NOTE: for the PQ KEM exp or using Delegated Credentials, it must be kept in
-// sync with supportedSignatureAlgorithmsDC.
 func signatureSchemesForCertificate(version uint16, cert *Certificate) []SignatureScheme {
 	priv, ok := cert.PrivateKey.(crypto.Signer)
 	if !ok {

src/crypto/tls/common.go

@@ -497,7 +497,8 @@ type ClientHelloInfo struct {
 
 	// SignatureSchemesDC lists the signature schemes that the client
 	// is willing to verify when using Delegated Credentials.
-	// This can be different from SignatureSchemes.
+	// This is and can be different from SignatureSchemes. SignatureSchemesDC
+	// is set only if the DelegatedCredentials Extension is being used.
 	// If Delegated Credentials are supported, this list should not be nil.
 	SignatureSchemesDC []SignatureScheme
 
@@ -549,6 +550,9 @@ type CertificateRequestInfo struct {
 
 	// SignatureSchemesDC lists the signature schemes that the server
 	// is willing to verify when using Delegated Credentials.
+	// This is and can be different from SignatureSchemes. SignatureSchemesDC
+	// is set only if the DelegatedCredentials Extension is being used.
+	// If Delegated Credentials are supported, this list should not be nil.
 	SignatureSchemesDC []SignatureScheme
 
 	// Version is the TLS version that was negotiated for this connection.
@@ -820,13 +824,6 @@ type Config struct {
 	// See https://tools.ietf.org/html/draft-ietf-tls-subcerts.
 	SupportDelegatedCredential bool
 
-	// GetDelegatedCredential returns a DelegatedCredential for use with the
-	// delegated credential extension based on the ClientHello. It only works
-	// with TLS 1.3. If this is nil, then the server will not offer
-	// a DelegatedCredential. If the call returns nil, the server is also
-	// not offering a DelegatedCredential.
-	GetDelegatedCredential func(*ClientHelloInfo, *CertificateRequestInfo) (*DelegatedCredential, crypto.PrivateKey, error)
-
 	// mutex protects sessionTicketKeys and autoSessionTicketKeys.
 	mutex sync.RWMutex
 	// sessionTicketKeys contains zero or more ticket keys. If set, it means the
@@ -917,7 +914,6 @@ func (c *Config) Clone() *Config {
 		Renegotiation:               c.Renegotiation,
 		KeyLogWriter:                c.KeyLogWriter,
 		SupportDelegatedCredential:  c.SupportDelegatedCredential,
-		GetDelegatedCredential:      c.GetDelegatedCredential,
 		ECHEnabled:                  c.ECHEnabled,
 		ClientECHConfigs:            c.ClientECHConfigs,
 		ServerECHProvider:           c.ServerECHProvider,
@@ -1462,6 +1458,16 @@ func (c *Config) writeKeyLog(label string, clientRandom, secret []byte) error {
 // and is only for debugging, so a global mutex saves space.
 var writerMutex sync.Mutex
 
+// A DelegatedCredentialPair contains a Delegated Credential and its
+// associated private key.
+type DelegatedCredentialPair struct {
+	// DC is the delegated credential.
+	DC *DelegatedCredential
+	// PrivateKey is the private key used to derive the public key of
+	// contained in DC. PrivateKey must implement crypto.Signer.
+	PrivateKey crypto.PrivateKey
+}
+
 // A Certificate is a chain of one or more certificates, leaf first.
 type Certificate struct {
 	Certificate [][]byte
@@ -1479,9 +1485,15 @@ type Certificate struct {
 	// SignedCertificateTimestamps contains an optional list of Signed
 	// Certificate Timestamps which will be served to clients that request it.
 	SignedCertificateTimestamps [][]byte
-	// DelegatedCredential is a serialized Delegated Credential, signed by
-	// the leaf certificate.
-	// If not supported, this will be nil.
+	// DelegatedCredentials are a list of Delegated Credentials with their
+	// corresponding private keys, signed by the leaf certificate.
+	// If there are no delegated credentials, this field is nil.
+	DelegatedCredentials []DelegatedCredentialPair
+	// DelegatedCredential is the delegated credential to be used in the
+	// handshake.
+	// If there are no delegated credentials, this field is nil.
+	// NOTE: Do not fill this field, as it will be filled depending on
+	// the provided list of delegated credentials.
 	DelegatedCredential []byte
 	// Leaf is the parsed form of the leaf certificate, which may be initialized
 	// using x509.ParseCertificate to reduce per-handshake processing. If nil,

src/crypto/tls/delegated_credentials_test.go

@@ -17,13 +17,6 @@ import (
 	"time"
 )
 
-// dcAndPrivateKey stores a Delegated Credential and its corresponding private
-// key.
-type dcAndPrivateKey struct {
-	*DelegatedCredential
-	privateKey crypto.PrivateKey
-}
-
 // These test keys were generated with the following program, available in the
 // crypto/tls directory:
 //
@@ -137,12 +130,12 @@ ZyJKvc2KqjGeZh0Or5pq6ZJb0zR7WPdz5aJIzaZ5YcxLMSv0KwaAEPH2
 `
 
 var (
-	dcTestConfig   *Config
-	dcTestCerts    map[string]*Certificate
-	serverDC       []*dcAndPrivateKey
-	clientDC       []*dcAndPrivateKey
-	dcNow          time.Time
-	dcTestDCScheme = []SignatureScheme{ECDSAWithP256AndSHA256, ECDSAWithP384AndSHA384, ECDSAWithP521AndSHA512, Ed25519}
+	dcTestConfig            *Config
+	dcTestCerts             map[string]*Certificate
+	serverDC                []DelegatedCredentialPair
+	clientDC                []DelegatedCredentialPair
+	dcNow                   time.Time
+	dcTestDCSignatureScheme = []SignatureScheme{ECDSAWithP256AndSHA256, Ed25519, ECDSAWithP384AndSHA384, ECDSAWithP521AndSHA512}
 )
 
 func init() {
@@ -232,7 +225,7 @@ func initDCTest() {
 	}
 	dcTestCerts["no dc"] = noDcCert
 
-	// The root certificates for the client.
+	// The root certificates for the peer.
 	dcTestConfig.RootCAs = x509.NewCertPool()
 
 	for _, c := range dcTestCerts {
@@ -243,20 +236,19 @@ func initDCTest() {
 		dcTestConfig.RootCAs.AddCert(dcRoot)
 	}
 
-	for i := 0; i < len(dcTestDCScheme); i++ {
-		dc, sk, err := NewDelegatedCredential(dcCertP256, dcTestDCScheme[i], dcNow.Sub(dcCertP256.Leaf.NotBefore)+dcMaxTTL, false)
+	for i := 0; i < len(dcTestDCSignatureScheme); i++ {
+		dc, priv, err := NewDelegatedCredential(dcCertP256, dcTestDCSignatureScheme[i], dcNow.Sub(dcCertP256.Leaf.NotBefore)+dcMaxTTL, false)
 		if err != nil {
 			panic(err)
 		}
-		serverDC = append(serverDC, &dcAndPrivateKey{dc, sk})
+		serverDC = append(serverDC, DelegatedCredentialPair{dc, priv})
 
-		dc, sk, err = NewDelegatedCredential(dcCertP256, dcTestDCScheme[i], dcNow.Sub(dcCertP256.Leaf.NotBefore)+dcMaxTTL, true)
+		dc, priv, err = NewDelegatedCredential(dcCertP256, dcTestDCSignatureScheme[i], dcNow.Sub(dcCertP256.Leaf.NotBefore)+dcMaxTTL, true)
 		if err != nil {
 			panic(err)
 		}
-		clientDC = append(clientDC, &dcAndPrivateKey{dc, sk})
+		clientDC = append(clientDC, DelegatedCredentialPair{dc, priv})
 	}
-
 }
 
 func publicKeysEqual(publicKey, publicKey2 crypto.PublicKey, algo SignatureScheme) error {
@@ -387,7 +379,7 @@ func TestDelegatedCredentialMarshal(t *testing.T) {
 	cert := dcTestCerts["dcEd25519"]
 	time := dcNow.Sub(cert.Leaf.NotBefore) + dcMaxTTL
 
-	for _, sig := range dcTestDCScheme {
+	for _, sig := range dcTestDCSignatureScheme {
 		delegatedCred, _, err := NewDelegatedCredential(cert, sig, time, false)
 		if err != nil {
 			t.Fatal(err)
@@ -418,24 +410,44 @@ func TestDelegatedCredentialMarshal(t *testing.T) {
 	}
 }
 
-var dcTests = []struct {
+var dcServerTests = []struct {
 	clientDCSupport bool
+	clientMaxVers   uint16
+	serverMaxVers   uint16
+	expectSuccess   bool
+	expectDC        bool
+	name            string
+}{
+	{true, VersionTLS13, VersionTLS13, true, true, "tls13: DC client support"},
+	{false, VersionTLS13, VersionTLS13, true, false, "DC not client support"},
+	{true, VersionTLS12, VersionTLS13, true, false, "client using TLS 1.2. No DC is supported in that version."},
+	{true, VersionTLS13, VersionTLS12, true, false, "server using TLS 1.2. No DC is supported in that version."},
+	{true, VersionTLS11, VersionTLS13, true, false, "client using TLS 1.1. No DC is supported in that version."},
+	{true, VersionTLS13, VersionTLS10, false, false, "server using TLS 1.0. No DC is supported in that version."},
+}
+
+var dcClientTests = []struct {
 	serverDCSupport bool
 	clientMaxVers   uint16
 	serverMaxVers   uint16
 	expectSuccess   bool
 	expectDC        bool
 	name            string
 }{
-	{true, true, VersionTLS13, VersionTLS13, true, true, "tls13: DC server and client support"},
-	{true, false, VersionTLS13, VersionTLS13, true, false, "DC not server support"},
-	{false, true, VersionTLS13, VersionTLS13, true, false, "DC not client support"},
-	{true, true, VersionTLS12, VersionTLS13, true, false, "client using TLS 1.2. No DC is supported in that version."},
-	{true, true, VersionTLS13, VersionTLS12, true, false, "server using TLS 1.2. No DC is supported in that version."},
-	{true, true, VersionTLS11, VersionTLS13, true, false, "client using TLS 1.1. No DC is supported in that version."},
-	{true, true, VersionTLS13, VersionTLS10, false, false, "server using TLS 1.0. No DC is supported in that version."},
+	{true, VersionTLS13, VersionTLS13, true, true, "tls13: DC server support"},
+	{false, VersionTLS13, VersionTLS13, true, false, "DC not server support"},
+	{true, VersionTLS12, VersionTLS13, true, false, "client using TLS 1.2. No DC is supported in that version."},
+	{true, VersionTLS13, VersionTLS12, true, false, "server using TLS 1.2. No DC is supported in that version."},
+	{true, VersionTLS11, VersionTLS13, true, false, "client using TLS 1.1. No DC is supported in that version."},
+	{true, VersionTLS13, VersionTLS10, false, false, "server using TLS 1.0. No DC is supported in that version."},
 }
 
+// dcCount defines the delegated credential to be used as returned by the
+// getCertificate or getClientCertificate callback. This allows to use
+// delegated credentials with different algorithms at each run of the
+// tests.
+var dcCount int
+
 // Checks that the client suppports a version >= 1.3 and accepts Delegated
 // Credentials. If so, it returns the delegation certificate; otherwise it
 // returns a non-delegated certificate.
@@ -446,7 +458,9 @@ func testServerGetCertificate(ch *ClientHelloInfo) (*Certificate, error) {
 	}
 
 	if versOk && ch.SupportsDelegatedCredential {
-		return dcTestCerts["dcP256"], nil
+		serverCert := dcTestCerts["dcP256"]
+		serverCert.DelegatedCredentials = serverDC[dcCount:]
+		return serverCert, nil
 	}
 	return dcTestCerts["no dc"], nil
 
@@ -462,45 +476,17 @@ func testClientGetCertificate(cr *CertificateRequestInfo) (*Certificate, error)
 	}
 
 	if versOk && cr.SupportsDelegatedCredential {
-		return dcTestCerts["dcP256"], nil
+		clientCert := dcTestCerts["dcP256"]
+		clientCert.DelegatedCredentials = clientDC[dcCount:]
+		return clientCert, nil
 	}
 	return dcTestCerts["no dc"], nil
 
 }
 
-// sets the dc to use for testing
-var dcCounter = 0
-
-// Checks that the client supports the signature algorithm supported by the test
-// server, and that the server has a Delegated Credential.
-func testGetDelegatedCredential(ch *ClientHelloInfo, cr *CertificateRequestInfo) (*DelegatedCredential, crypto.PrivateKey, error) {
-	if ch != nil {
-		schemeOk := false
-		for _, scheme := range ch.SignatureSchemesDC {
-			schemeOk = schemeOk || (scheme == dcTestDCScheme[dcCounter])
-		}
-
-		if schemeOk && ch.SupportsDelegatedCredential {
-			return serverDC[dcCounter].DelegatedCredential, serverDC[dcCounter].privateKey, nil
-		}
-	} else if cr != nil {
-		schemeOk := false
-		for _, scheme := range cr.SignatureSchemesDC {
-			schemeOk = schemeOk || (scheme == dcTestDCScheme[dcCounter])
-		}
-
-		if schemeOk && cr.SupportsDelegatedCredential {
-			return clientDC[dcCounter].DelegatedCredential, clientDC[dcCounter].privateKey, nil
-		}
-	}
-
-	return nil, nil, nil
-}
-
 // Tests the handshake and one round of application data. Returns true if the
 // connection correctly used a Delegated Credential.
 func testConnWithDC(t *testing.T, clientMsg, serverMsg string, clientConfig, serverConfig *Config, peer string) (bool, error) {
-	initDCTest()
 	ln := newLocalListener(t)
 	defer ln.Close()
 
@@ -569,38 +555,30 @@ func TestDCHandshakeServerAuth(t *testing.T) {
 
 	clientConfig := dcTestConfig.Clone()
 	serverConfig := dcTestConfig.Clone()
-	serverConfig.GetCertificate = testServerGetCertificate
 	clientConfig.InsecureSkipVerify = true
 
-	for i, test := range dcTests {
+	for i, test := range dcServerTests {
 		clientConfig.SupportDelegatedCredential = test.clientDCSupport
 
-		for dcCounter < len(dcTestDCScheme)-1 {
-			if test.serverDCSupport {
-				serverConfig.GetDelegatedCredential = testGetDelegatedCredential
-				dcCounter++
-			} else {
-				serverConfig.GetDelegatedCredential = nil
-			}
-
+		initDCTest()
+		for dcCount = 0; dcCount < len(dcTestDCSignatureScheme); dcCount++ {
+			serverConfig.GetCertificate = testServerGetCertificate
 			clientConfig.MaxVersion = test.clientMaxVers
 			serverConfig.MaxVersion = test.serverMaxVers
 
 			usedDC, err := testConnWithDC(t, clientMsg, serverMsg, clientConfig, serverConfig, "client")
 
 			if err != nil && test.expectSuccess {
-				t.Errorf("test #%d (%s) fails: %s", i+1, test.name, err.Error())
+				t.Errorf("test #%d (%s) with signature algorithm #%d fails: %s", i, test.name, dcCount, err.Error())
 			} else if err == nil && !test.expectSuccess {
-				t.Errorf("test #%d (%s) succeeds; expected failure", i+1, test.name)
+				t.Errorf("test #%d (%s) with signature algorithm #%d succeeds; expected failure", i, test.name, dcCount)
 			}
 
 			if usedDC != test.expectDC {
-				t.Errorf("test #%d (%s) usedDC = %v; expected %v", i+1, test.name, usedDC, test.expectDC)
+				t.Errorf("test #%d (%s) with signature algorithm #%d usedDC = %v; expected %v", i, test.name, dcCount, usedDC, test.expectDC)
 			}
 		}
 	}
-
-	dcCounter = 0
 }
 
 // Test the client authentication with the Delegated Credential extension.
@@ -614,34 +592,27 @@ func TestDCHandshakeClientAuth(t *testing.T) {
 	clientConfig := dcTestConfig.Clone()
 	clientConfig.GetClientCertificate = testClientGetCertificate
 
-	for i, test := range dcTests {
+	for j, test := range dcClientTests {
 		serverConfig.SupportDelegatedCredential = test.serverDCSupport
 
-		for dcCounter < len(dcTestDCScheme)-1 {
-			if test.clientDCSupport {
-				clientConfig.GetDelegatedCredential = testGetDelegatedCredential
-				dcCounter++
-			} else {
-				clientConfig.GetDelegatedCredential = nil
-			}
-
+		initDCTest()
+		for dcCount = 0; dcCount < len(dcTestDCSignatureScheme); dcCount++ {
 			serverConfig.MaxVersion = test.serverMaxVers
 			clientConfig.MaxVersion = test.clientMaxVers
 
 			usedDC, err := testConnWithDC(t, clientMsg, serverMsg, clientConfig, serverConfig, "server")
 
 			if err != nil && test.expectSuccess {
-				t.Errorf("test #%d (%s) fails: %s", i+1, test.name, err.Error())
+				t.Errorf("test #%d (%s) with signature algorithm #%d fails: %s", j, test.name, dcCount, err.Error())
 			} else if err == nil && !test.expectSuccess {
-				t.Errorf("test #%d (%s) succeeds; expected failure", i+1, test.name)
+				t.Errorf("test #%d (%s) with signature algorithm #%d succeeds; expected failure", j, test.name, dcCount)
 			}
 
 			if usedDC != test.expectDC {
-				t.Errorf("test #%d (%s) usedDC = %v; expected %v", i+1, test.name, usedDC, test.expectDC)
+				t.Errorf("test #%d (%s) with signature algorithm #%d usedDC = %v; expected %v", j, test.name, dcCount, usedDC, test.expectDC)
 			}
 		}
 	}
-	dcCounter = 0
 }
 
 // Test server and client authentication with the Delegated Credential extension.
@@ -657,19 +628,19 @@ func TestDCHandshakeClientAndServerAuth(t *testing.T) {
 
 	serverConfig.SupportDelegatedCredential = true
 	clientConfig.SupportDelegatedCredential = true
-	clientConfig.GetDelegatedCredential = testGetDelegatedCredential
-	serverConfig.GetDelegatedCredential = testGetDelegatedCredential
 
 	serverConfig.MaxVersion = VersionTLS13
 	clientConfig.MaxVersion = VersionTLS13
 
+	initDCTest()
+
 	usedDC, err := testConnWithDC(t, clientMsg, serverMsg, clientConfig, serverConfig, "both")
 
 	if err != nil {
 		t.Errorf("test server and client auth fails: %s", err.Error())
 	}
 
 	if usedDC != true {
-		t.Errorf("test server and client auth does not succed")
+		t.Errorf("test server and client auth does not succeed")
 	}
 }