fix first round of review

internal/credentials/spiffe_test.go

@@ -30,6 +30,8 @@ import (
 	"google.golang.org/grpc/testdata"
 )
 
+const wantURI = "spiffe://foo.bar.com/client/workload/1"
+
 type s struct {
 	grpctest.Tester
 }
@@ -183,23 +185,49 @@ func (s) TestSPIFFEIDFromState(t *testing.T) {
 }
 
 func (s) TestSPIFFEIDFromCert(t *testing.T) {
-	data, err := ioutil.ReadFile(testdata.Path("x509/spiffe_cert.pem"))
-	if err != nil {
-		t.Fatalf("Failed to load the test credentials: %v", err)
-	}
-	block, rest := pem.Decode(data)
-	if len(rest) != 0 {
-		t.Fatalf("Failed to parse the certificate: invalid format")
-	}
-	cert, err := x509.ParseCertificate(block.Bytes)
-	if err != nil {
-		t.Fatalf("Failed to load the test credentials: %v", err)
-	}
-	uri := SPIFFEIDFromCert(cert)
-	if uri == nil {
-		t.Fatalf("SPIFFE ID is nil")
+	tests := []struct {
+		name     string
+		dataPath string
+		// If we expect a SPIFFE ID to be returned.
+		expectID bool
+	}{
+		{
+			name:     "good certificate with SPIFFE ID",
+			dataPath: "x509/spiffe_cert.pem",
+			expectID: true,
+		},
+		{
+			name:     "bad certificate with SPIFFE ID and another URI",
+			dataPath: "x509/multiple_uri_cert.pem",
+			expectID: false,
+		},
+		{
+			name:     "certificate without SPIFFE ID",
+			dataPath: "x509/client1_cert.pem",
+			expectID: false,
+		},
 	}
-	if uri.String() != "spiffe://foo.bar.com/client/workload/1" {
-		t.Fatalf("SPIFFE ID not expected, got %s, want %s", uri.String(), "spiffe://foo.bar.com/client/workload/1")
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			data, err := ioutil.ReadFile(testdata.Path(tt.dataPath))
+			if err != nil {
+				t.Fatalf("ioutil.ReadFile(%s) failed: %v", testdata.Path(tt.dataPath), err)
+			}
+			block, _ := pem.Decode(data)
+			if block == nil {
+				t.Fatalf("Failed to parse the certificate: byte block is nil")
+			}
+			cert, err := x509.ParseCertificate(block.Bytes)
+			if err != nil {
+				t.Fatalf("x509.ParseCertificate(%b) failed: %v", block.Bytes, err)
+			}
+			uri := SPIFFEIDFromCert(cert)
+			if (uri != nil) != tt.expectID {
+				t.Fatalf("expectID got and want mismatch, got %t, want %t", uri != nil, tt.expectID)
+			}
+			if uri != nil && uri.String() != wantURI {
+				t.Fatalf("SPIFFE ID not expected, got %s, want %s", uri.String(), wantURI)
+			}
+		})
 	}
 }

testdata/x509/create.sh

@@ -110,5 +110,14 @@ openssl req -x509                                                         \
   -subj /C=US/ST=CA/L=SVL/O=gRPC/CN=test-client1/                         \
   -addext "subjectAltName = URI:spiffe://foo.bar.com/client/workload/1"
 
+# Generate a cert with SPIFFE ID and another SAN URI field(which doesn't meet SPIFFE specs).
+openssl req -x509                                                         \
+  -newkey rsa:4096                                                        \
+  -keyout multiple_uri_key.pem                                            \
+  -out multiple_uri_cert.pem                                              \
+  -nodes                                                                  \
+  -days 3650                                                              \
+  -subj /C=US/ST=CA/L=SVL/O=gRPC/CN=test-client1/                         \
+  -addext "subjectAltName = URI:spiffe://foo.bar.com/client/workload/1, URI:https://bar.baz.com/client"
 # Cleanup the CSRs.
 rm *_csr.pem

testdata/x509/multiple_uri_cert.pem

@@ -0,0 +1,34 @@
+-----BEGIN CERTIFICATE-----
+MIIFzjCCA7agAwIBAgIUI8r7b2hX9DRwEQGWuRdk32eU5kowDQYJKoZIhvcNAQEL
+BQAwTjELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNBMQwwCgYDVQQHDANTVkwxDTAL
+BgNVBAoMBGdSUEMxFTATBgNVBAMMDHRlc3QtY2xpZW50MTAeFw0yMDEwMDcwNjQx
+MTRaFw0zMDEwMDUwNjQxMTRaME4xCzAJBgNVBAYTAlVTMQswCQYDVQQIDAJDQTEM
+MAoGA1UEBwwDU1ZMMQ0wCwYDVQQKDARnUlBDMRUwEwYDVQQDDAx0ZXN0LWNsaWVu
+dDEwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQCm/zjNYkfCTcq7tnVf
+qkPEde1+M6s2z05iWDfoBeZfC2NwUxIBqAC6XTXTxqYSjEVRCQUzjVxyWQNiwuz7
+pK/xGZhP/Ih2uSQKTw8vkXay4HCOt9DR0S/XGcQNImdbawKgnGven8Jrg8UZDXrt
+9R9Z0nRajB1eXvXOsEEoEfOnYthc6P+MxWJc0lnfaTlowyEgv84Ha13y1h46W6yC
++WNBT/kWqp/mzDTv/Ima8xcqEft9VUZ82qJ1DVt1064x8KOzm2x7F7QSIcjxr39M
+fbASm8Vdnt10XfhdsDVkxTlBJs8WKGn0uw8MyPNjFG01OpYDHLAfJTL3XlvaUjfF
+yDMFsRDVjfkuYIkqAVWQ7eleFfOFBYzaVf2K+2OvCR+vGAPa5NQ59kwogJYLjV/O
+43axChBizcPM0p7gmRhhO7TQz7LLTea30rBJ/YtXdxFR11y9Jdq+i2KwWi8O50iO
+hzxUBkbcQD/W9Bcn7gOkD/pgEGynWvFSs+UHjLeyL0COk0NiuYIMlOgwtI5BGwzD
+bdLuTU/ZQm4BJBjEIGVHFqKyTqUXcw5t9fWxH8V0XNs8zqj9J7lvNKu9b88GnyaJ
+fKMdDO4rVTJHmvFDHP9MUJHC9SabW8+hK0nuU7n3+Pc07ToCAan+Ych5bQHsRMjI
+9EvxKVNfwIwNrmRr3mhbOU9xIQIDAQABo4GjMIGgMB0GA1UdDgQWBBS6jnt9IccJ
+SOuE1KwP68VCBPB4hTAfBgNVHSMEGDAWgBS6jnt9IccJSOuE1KwP68VCBPB4hTAP
+BgNVHRMBAf8EBTADAQH/ME0GA1UdEQRGMESGJnNwaWZmZTovL2Zvby5iYXIuY29t
+L2NsaWVudC93b3JrbG9hZC8xhhpodHRwczovL2Jhci5iYXouY29tL2NsaWVudDAN
+BgkqhkiG9w0BAQsFAAOCAgEAoR4LbmvtSXLiVg7BRilvSxIWgcG6AI75/afuaM20
+PUTpyDhnrPxEaytb5LP0w42BCMoIHXDLE0Jmbxqbi+ku/Qw1R6723J7gwRSUYIg1
+a2S5Gue4AFp7aSLDUZhl0jPphq7OMKozzH5TrDgjKljYjPURClc/ODSlGdzOqlif
+CbDHwrCorb+BFM3aFDE0pF06pnMDXcn/Ob9QCLIpvZEOWe/fJbPtTUiA5cY3knne
+regyhvfqfVZtU52qg+9o6q5QchVqOt19alAsISK9/H4iVE+S79AiYEAU4yM4S6p5
+VW44idy3KXmr5kyVwJhe3t9f5Ckuswmo6hL32ec6M52ElrS8Er0vFt4bjfNgq996
+lTm4/reL/Anko9chQiGBe7F8J82OfxjLoVH9CbZjIoS4LiZPkey3Ze9HUV1sHhM/
+umkL54jRsVjEwwSCIcF9onzmiD8D7FV3AQ9W/RbBF3wZvVBBs9ZKQCxek2pZX/eZ
+Q+BvXwG7NGArowpqbi+tSrW3O+XZzY7nXbbf23jCBwkBn3jvqn1Kwsr/T/HbXUaz
+dDUvkwgyrX7NfvvZ20svtKLlBZTO5D8P9fy0+cHsS0XkPhw6UbHk396hoOmVZ+OG
+E5uVb2sBy+vx+82IwVzWN0o7380AEmAA5nrA6fMaxTxmo07pOF7avAZ34LgHJIjr
+sTM=
+-----END CERTIFICATE-----

testdata/x509/multiple_uri_key.pem

@@ -0,0 +1,52 @@
+-----BEGIN PRIVATE KEY-----
+MIIJQQIBADANBgkqhkiG9w0BAQEFAASCCSswggknAgEAAoICAQCm/zjNYkfCTcq7
+tnVfqkPEde1+M6s2z05iWDfoBeZfC2NwUxIBqAC6XTXTxqYSjEVRCQUzjVxyWQNi
+wuz7pK/xGZhP/Ih2uSQKTw8vkXay4HCOt9DR0S/XGcQNImdbawKgnGven8Jrg8UZ
+DXrt9R9Z0nRajB1eXvXOsEEoEfOnYthc6P+MxWJc0lnfaTlowyEgv84Ha13y1h46
+W6yC+WNBT/kWqp/mzDTv/Ima8xcqEft9VUZ82qJ1DVt1064x8KOzm2x7F7QSIcjx
+r39MfbASm8Vdnt10XfhdsDVkxTlBJs8WKGn0uw8MyPNjFG01OpYDHLAfJTL3Xlva
+UjfFyDMFsRDVjfkuYIkqAVWQ7eleFfOFBYzaVf2K+2OvCR+vGAPa5NQ59kwogJYL
+jV/O43axChBizcPM0p7gmRhhO7TQz7LLTea30rBJ/YtXdxFR11y9Jdq+i2KwWi8O
+50iOhzxUBkbcQD/W9Bcn7gOkD/pgEGynWvFSs+UHjLeyL0COk0NiuYIMlOgwtI5B
+GwzDbdLuTU/ZQm4BJBjEIGVHFqKyTqUXcw5t9fWxH8V0XNs8zqj9J7lvNKu9b88G
+nyaJfKMdDO4rVTJHmvFDHP9MUJHC9SabW8+hK0nuU7n3+Pc07ToCAan+Ych5bQHs
+RMjI9EvxKVNfwIwNrmRr3mhbOU9xIQIDAQABAoICADn4UuGJAlwC4SN0XR5OXqPu
+Q/kROpgWMqGU+iNDGQtZSrWNQKzugwIupSbUyIWbx9wvg2y336WaHMDF5bodGy5Y
+sjTh9wUvk8E4XI8oscm6e5gvWv/a2/6RZSsiDDsB1LGoWxG256im316o/UlpU+68
+TcO46+D8mdub96JPSQOMHotyHnPheRm7s5MIVfN1+SQDMSQGM2C+z1N2y1XT+I6N
+kmw54rQdoyrDwYjWZe4mu+RwG73vr4Ful5c5WjjfzhPlGi1ItyusKrMrNsd4wgxT
+opmzMjDZBgSPzJkklZF2RWDtuopH/Rt1DngQeTCHG9gMt164bQ7N5JjO/alcq8j4
+TW/IRlZOllqJ0KogOn9nX2ce9Kfxz+H36Yj54sKuOOYvKRsoiTNdTD3D6eB7pwnQ
+KGWAGrpU4llbzotiG5NJ8sDHYUwynmhfmwIeBjq0vuXlITLQplGYQnsQJI29Py3N
+KWBOC9HaiKCKq2gAUacj8BK+BLeGEiV9sxWQb7/MbWRxXnW4KhNI8+ft+PZOuvZZ
+vLxH0wg4/bYQISMaeaqWL4LksKtV7es4MglFdCCZGDMdy1/btIHjRFPQWwIaXxij
+2OtCozfmmzIc76UQ8g506q4rSgzZclDvI3Yd3cm3XFl4cQfr5l8WTQ313wrlmo5U
+DjYdKipOGFRSLHt7aXABAoIBAQDY4KyfuCHFMqKC0FJZUCr3/gGU0aqZqHR4l5jl
+N0TsTuwCRf4lK9BuM1bqumv6Nbi6VwWmp3+BzZCI/Nn7+s6KN5hyulBd56X7owef
+zl9yWW0n6nJxKzutiH06krjmODtO44gLjR7ddcEd+i023hwIQffdAE6IEtYuuoD4
+94pKd+dB9GQmgITwjS5vEP67A0lpFlL6pNMfhhe2QOLUnDPKsgSnKgwJsbBYC19j
+TQUpgFh4iCYKSGAX4ABdKpOUjbKGqNGrZNPQv+4MS9u6s/HWN7yDaSMT/tB9n1MC
+g8m7crWyOuNJ5oO6SPnetkdTbcam7tiCce/auqjx2cMJ2eDBAoIBAQDFHxPHXP6Z
+FHxI2pYBFyUB7j0VipwG3105ujrJJWu2abFU778SrkmM16eaWHVH6tMvuAo24mP1
+6Qfi09uAjdwhRPmIfManxj5wpDafgvG7H5g7+VhY2/IXTahO46JuZAxVoiXUGmct
+WwmOy0vpI2IxoXY8qLvaJv+b9nLpNi1PVJ743BmPMqG3dInoRAIBxMMEu6Drrbj3
+bjPmRNpqhs7/Kn4IahCalD6lgSBkDuz7DaJji8jINw5OhiL9VU1eslXmGrCbZMXv
+1QG0EjAZvGzqWPL88mKYTecndP1k9DMqVBVGhCT2dW1aLypQgDCC5YxyRc5vOnZ+
+2vQpELPeS0hhAoIBABBEqi5A7aeRKMePQN4aOV7o2s2C/L0R+cqh9IIdJzpioSl6
+fpnjM3tQtpBc84SNSxIPPQlHPzVJajIcZW2VXrDXgsP4XdbtbXH2xLekD1zQgHOi
+DnuWtp9JwbsHDn+WcDx2rNnQ+CO8lYPeJE4dUxT7fdBCGaHzZ8WRj+MdDm6Pl/VG
+k8yfj1lL/dOu/qygjn0ng4nxmzSeJmExdNJl9SybNeYkLUr83TF9iOY1/NEkI37H
+F7Nlwm+ICf7zFqbqCh43w6KLqafa/cxGVHEo1lcvTyC8Xjk9v/3sWZmysQsyi5aW
+/D2q4O60Uqn2GluTvHcBK5R9X3SU099wakTu5wECggEAUljjOFu++FA4g27dT2NN
+0HqoBgG7oJtbJKyJtlHtp2yL6kGlfrZUf4PvvmjJxdtxkfO+QKNewvIwmy+J+TBK
+D5Py8nO9wYTtvLy9HPHk7hkKzbMilyx6/AUzFJG/34HoLTXpu6u0ApyPZ5nCAokH
+klgzPq/2mfHEwnC4HHjHgOaG6st32fx61lrW6bLPa9G47pc7aHlQVf0xrTaCUBI1
+Ex+7OuSkPw9DBHzm/SXHFjHh7tgMbqehUGh04YPrKG4zuEbaFHCKx+AiMAmREo9G
+qLez+rt/OMUCldcnrC7f2QT7RlQZ5OO1ZQFjGfITUft3Kp3C2XCA5AmwCh+yJGEq
+wQKCAQANvxxFh6VvjU2+rB8Q4mDzYdr9OFTWMag3SNjBwwWoSXbL2wXPE5gFpzKj
+yvEbjmOgzIRABt6Eytx32p0pC5UFIey5PNu+/4ejxiiQdKSLQbqQavKYdfGgyZ0/
+JVqNKiiEJ0b9VtqhAG+Ye1mHZIBzXncWyBSZtxUGVuLG29uKbBo4ufyKauPd3dDv
+wR+JqEmAg0ICIFR+q81dEWY/gKsyyI5hMYTTsWge3l3FAdwMZEn9Ek0nclSb3dev
+ZiVlFvMZPdp5IwZljClRxnyto7bOTw+X/RMuVLB6p+v3URY4oUSL15+RNODn/tWM
+zJOG+48NgohVKfBhGN7JyxV1dq/X
+-----END PRIVATE KEY-----