Support certificate chains in SNI auth (Azure#18832)

Author: chlowell

sdk/azidentity/CHANGELOG.md

@@ -7,6 +7,7 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+* `ClientCertificateCredential` sends only the leaf cert for SNI authentication
 
 ### Other Changes
 

sdk/azidentity/azidentity_test.go

@@ -8,6 +8,7 @@ package azidentity
 
 import (
 	"context"
+	"crypto/x509"
 	"errors"
 	"fmt"
 	"io"
@@ -122,7 +123,7 @@ const (
 	testHost = "https://localhost"
 )
 
-func validateJWTRequestContainsHeader(t *testing.T, headerName string) mock.ResponsePredicate {
+func validateX5C(t *testing.T, certs []*x509.Certificate) mock.ResponsePredicate {
 	return func(req *http.Request) bool {
 		body, err := io.ReadAll(req.Body)
 		if err != nil {
@@ -135,8 +136,10 @@ func validateJWTRequestContainsHeader(t *testing.T, headerName string) mock.Resp
 		if token == nil {
 			t.Fatalf("Failed to parse the JWT token: %s.", assertion[1])
 		}
-		if _, ok := token.Header[headerName]; !ok {
-			t.Fatalf("JWT did not contain the %s header", headerName)
+		if v, ok := token.Header["x5c"].([]any); !ok {
+			t.Fatal("missing x5c header")
+		} else if actual := len(v); actual != len(certs) {
+			t.Fatalf("expected %d certs, got %d", len(certs), actual)
 		}
 		return true
 	}

sdk/azidentity/client_certificate_credential.go

@@ -9,10 +9,7 @@ package azidentity
 import (
 	"context"
 	"crypto"
-	"crypto/rsa"
-	"crypto/sha1"
 	"crypto/x509"
-	"encoding/base64"
 	"encoding/pem"
 	"errors"
 	"os"
@@ -46,10 +43,6 @@ func NewClientCertificateCredential(tenantID string, clientID string, certs []*x
 	if len(certs) == 0 {
 		return nil, errors.New("at least one certificate is required")
 	}
-	pk, ok := key.(*rsa.PrivateKey)
-	if !ok {
-		return nil, errors.New("'key' must be an *rsa.PrivateKey")
-	}
 	if !validTenantID(tenantID) {
 		return nil, errors.New(tenantIDValidationErr)
 	}
@@ -60,11 +53,7 @@ func NewClientCertificateCredential(tenantID string, clientID string, certs []*x
 	if err != nil {
 		return nil, err
 	}
-	cert, err := newCertContents(certs, pk, options.SendCertificateChain)
-	if err != nil {
-		return nil, err
-	}
-	cred := confidential.NewCredFromCert(cert.c, key) // TODO: NewCredFromCert should take a slice
+	cred, err := confidential.NewCredFromCertChain(certs, key)
 	if err != nil {
 		return nil, err
 	}
@@ -156,36 +145,6 @@ func ParseCertificates(certData []byte, password []byte) ([]*x509.Certificate, c
 	return certs, pk, nil
 }
 
-type certContents struct {
-	c   *x509.Certificate // the signing cert
-	fp  []byte            // the signing cert's fingerprint, a SHA-1 digest
-	pk  *rsa.PrivateKey   // the signing key
-	x5c []string          // concatenation of every provided cert, base64 encoded
-}
-
-func newCertContents(certs []*x509.Certificate, key *rsa.PrivateKey, sendCertificateChain bool) (*certContents, error) {
-	cc := certContents{pk: key}
-	// need the the signing cert's fingerprint: identify that cert by matching its public key to the private key
-	for _, cert := range certs {
-		certKey, ok := cert.PublicKey.(*rsa.PublicKey)
-		if ok && key.E == certKey.E && key.N.Cmp(certKey.N) == 0 {
-			fp := sha1.Sum(cert.Raw)
-			cc.fp = fp[:]
-			cc.c = cert
-			if sendCertificateChain {
-				// signing cert must be first in x5c
-				cc.x5c = append([]string{base64.StdEncoding.EncodeToString(cert.Raw)}, cc.x5c...)
-			}
-		} else if sendCertificateChain {
-			cc.x5c = append(cc.x5c, base64.StdEncoding.EncodeToString(cert.Raw))
-		}
-	}
-	if len(cc.fp) == 0 || cc.c == nil {
-		return nil, errors.New("found no certificate matching 'key'")
-	}
-	return &cc, nil
-}
-
 func loadPEMCert(certData []byte) ([]*pem.Block, error) {
 	blocks := []*pem.Block{}
 	for {

sdk/azidentity/client_certificate_credential_test.go

@@ -39,6 +39,7 @@ func newCertTest(name, certPath string, password string) certTest {
 var allCertTests = []certTest{
 	newCertTest("pem", "testdata/certificate.pem", ""),
 	newCertTest("pemB", "testdata/certificate_formatB.pem", ""),
+	newCertTest("pemChain", "testdata/certificate-with-chain.pem", ""),
 	newCertTest("pkcs12", "testdata/certificate.pfx", ""),
 	newCertTest("pkcs12Encrypted", "testdata/certificate_encrypted_key.pfx", "password"),
 }
@@ -107,26 +108,29 @@ func TestClientCertificateCredential_GetTokenSuccess_withCertificateChain(t *tes
 	}
 }
 
-func TestClientCertificateCredential_GetTokenSuccess_withCertificateChain_mock(t *testing.T) {
-	test := allCertTests[0]
-	srv, close := mock.NewServer(mock.WithTransformAllRequestsToTestServerUrl())
-	defer close()
-	srv.AppendResponse(mock.WithBody(instanceDiscoveryResponse))
-	srv.AppendResponse(mock.WithBody([]byte(tenantDiscoveryResponse)))
-	srv.AppendResponse(mock.WithPredicate(validateJWTRequestContainsHeader(t, "x5c")), mock.WithBody([]byte(accessTokenRespSuccess)))
-	srv.AppendResponse()
+func TestClientCertificateCredential_SendCertificateChain(t *testing.T) {
+	for _, test := range allCertTests {
+		t.Run(test.name, func(t *testing.T) {
+			srv, close := mock.NewServer(mock.WithTransformAllRequestsToTestServerUrl())
+			defer close()
+			srv.AppendResponse(mock.WithBody(instanceDiscoveryResponse))
+			srv.AppendResponse(mock.WithBody([]byte(tenantDiscoveryResponse)))
+			srv.AppendResponse(mock.WithPredicate(validateX5C(t, test.certs)), mock.WithBody([]byte(accessTokenRespSuccess)))
+			srv.AppendResponse()
 
-	options := ClientCertificateCredentialOptions{ClientOptions: azcore.ClientOptions{Transport: srv}, SendCertificateChain: true}
-	cred, err := NewClientCertificateCredential(fakeTenantID, fakeClientID, test.certs, test.key, &options)
-	if err != nil {
-		t.Fatal(err)
-	}
-	tk, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{liveTestScope}})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if tk.Token != tokenValue {
-		t.Fatalf("unexpected token: %s", tk.Token)
+			options := ClientCertificateCredentialOptions{ClientOptions: azcore.ClientOptions{Transport: srv}, SendCertificateChain: true}
+			cred, err := NewClientCertificateCredential(fakeTenantID, fakeClientID, test.certs, test.key, &options)
+			if err != nil {
+				t.Fatal(err)
+			}
+			tk, err := cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{liveTestScope}})
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tk.Token != tokenValue {
+				t.Fatalf("unexpected token: %s", tk.Token)
+			}
+		})
 	}
 }
 

sdk/azidentity/environment_credential_test.go

@@ -195,12 +195,20 @@ func TestEnvironmentCredential_UsernamePasswordSet(t *testing.T) {
 }
 
 func TestEnvironmentCredential_SendCertificateChain(t *testing.T) {
+	certData, err := os.ReadFile(liveSP.pfxPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	certs, _, err := ParseCertificates(certData, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
 	resetEnvironmentVarsForTest()
 	srv, close := mock.NewServer(mock.WithTransformAllRequestsToTestServerUrl())
 	defer close()
 	srv.AppendResponse(mock.WithBody(instanceDiscoveryResponse))
 	srv.AppendResponse(mock.WithBody([]byte(tenantDiscoveryResponse)))
-	srv.AppendResponse(mock.WithPredicate(validateJWTRequestContainsHeader(t, "x5c")), mock.WithBody([]byte(accessTokenRespSuccess)))
+	srv.AppendResponse(mock.WithPredicate(validateX5C(t, certs)), mock.WithBody([]byte(accessTokenRespSuccess)))
 	srv.AppendResponse()
 
 	vars := map[string]string{