Merge pull request #151 from azdagron/remove-non-spec-conformant-field

Remove non-spec conformant field from Workload proto
spiffe · Aug 3, 2020 · dfb504d · dfb504d
2 parents 181b656 + 07d5d81
commit dfb504d
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 97 deletions.
diff --git a/proto/spiffe/workload/workload.pb.go b/proto/spiffe/workload/workload.pb.go
diff --git a/proto/spiffe/workload/workload.proto b/proto/spiffe/workload/workload.proto
@@ -38,13 +38,8 @@ message X509SVID {
     // CA certificates belonging to the Trust Domain
     // ASN.1 DER encoded
     bytes bundle = 4;
-
-    // List of trust domains the SVID federates with, which corresponds to
-    // keys in the federated_bundles map in the X509SVIDResponse message.
-    repeated string federates_with = 5;
 }
 
-
 message JWTSVID {
     string spiffe_id = 1;
 

diff --git a/spiffe/tls_peer_test.go b/spiffe/tls_peer_test.go
@@ -29,9 +29,8 @@ func TestTLSPeer(t *testing.T) {
 		Bundle: domain1CA.Roots(),
 		SVIDs: []spiffetest.X509SVID{
 			{
-				CertChain:     serverSVID,
-				Key:           serverKey,
-				FederatesWith: []string{"spiffe://domain2.test"},
+				CertChain: serverSVID,
+				Key:       serverKey,
 			},
 		},
 		FederatedBundles: map[string][]*x509.Certificate{
@@ -48,9 +47,8 @@ func TestTLSPeer(t *testing.T) {
 		Bundle: domain2CA.Roots(),
 		SVIDs: []spiffetest.X509SVID{
 			{
-				CertChain:     clientSVID,
-				Key:           clientKey,
-				FederatesWith: []string{"spiffe://domain1.test"},
+				CertChain: clientSVID,
+				Key:       clientKey,
 			},
 		},
 		FederatedBundles: map[string][]*x509.Certificate{
@@ -103,9 +101,8 @@ func TestTLSPeerGRPC(t *testing.T) {
 		Bundle: domain1CA.Roots(),
 		SVIDs: []spiffetest.X509SVID{
 			{
-				CertChain:     serverSVID,
-				Key:           serverKey,
-				FederatesWith: []string{"spiffe://domain2.test"},
+				CertChain: serverSVID,
+				Key:       serverKey,
 			},
 		},
 		FederatedBundles: map[string][]*x509.Certificate{
@@ -122,9 +119,8 @@ func TestTLSPeerGRPC(t *testing.T) {
 		Bundle: domain2CA.Roots(),
 		SVIDs: []spiffetest.X509SVID{
 			{
-				CertChain:     clientSVID,
-				Key:           clientKey,
-				FederatesWith: []string{"spiffe://domain1.test"},
+				CertChain: clientSVID,
+				Key:       clientKey,
 			},
 		},
 		FederatedBundles: map[string][]*x509.Certificate{

diff --git a/spiffetest/workload_api.go b/spiffetest/workload_api.go
@@ -142,9 +142,8 @@ func (w *workloadAPIWrapper) FetchX509SVID(req *workload.X509SVIDRequest, stream
 }
 
 type X509SVID struct {
-	CertChain     []*x509.Certificate
-	Key           crypto.Signer
-	FederatesWith []string
+	CertChain []*x509.Certificate
+	Key       crypto.Signer
 }
 
 type X509SVIDResponse struct {
@@ -174,11 +173,10 @@ func (r *X509SVIDResponse) ToProto(tb testing.TB) *workload.X509SVIDResponse {
 			require.NoError(tb, err)
 		}
 		pb.Svids = append(pb.Svids, &workload.X509SVID{
-			SpiffeId:      spiffeID,
-			X509Svid:      derBlobFromCerts(svid.CertChain),
-			X509SvidKey:   keyDER,
-			Bundle:        bundle,
-			FederatesWith: svid.FederatesWith,
+			SpiffeId:    spiffeID,
+			X509Svid:    derBlobFromCerts(svid.CertChain),
+			X509SvidKey: keyDER,
+			Bundle:      bundle,
 		})
 	}
 	for k, v := range r.FederatedBundles {

diff --git a/workload/proto.go b/workload/proto.go
@@ -40,7 +40,7 @@ func protoToX509SVIDs(protoSVIDs *workload.X509SVIDResponse) (*X509SVIDs, error)
 	return svids, nil
 }
 
-func protoToX509SVID(svid *workload.X509SVID, allFederatedBundles map[string][]*x509.Certificate) (*X509SVID, error) {
+func protoToX509SVID(svid *workload.X509SVID, federatedTrustBundles map[string][]*x509.Certificate) (*X509SVID, error) {
 	certificates, err := x509.ParseCertificates(svid.GetX509Svid())
 	if err != nil {
 		return nil, err
@@ -65,15 +65,9 @@ func protoToX509SVID(svid *workload.X509SVID, allFederatedBundles map[string][]*
 	}
 	trustBundlePool := internal.CertPoolFromCerts(trustBundle)
 
-	federatedTrustBundles := make(map[string][]*x509.Certificate)
 	federatedTrustBundlePools := make(map[string]*x509.CertPool)
-	for _, federatesWith := range svid.GetFederatesWith() {
-		bundle, ok := allFederatedBundles[federatesWith]
-		if !ok {
-			return nil, fmt.Errorf("missing bundle for federated domain %q", federatesWith)
-		}
-		federatedTrustBundles[federatesWith] = bundle
-		federatedTrustBundlePools[federatesWith] = internal.CertPoolFromCerts(bundle)
+	for trustDomainID, federatedTrustBundle := range federatedTrustBundles {
+		federatedTrustBundlePools[trustDomainID] = internal.CertPoolFromCerts(federatedTrustBundle)
 	}
 
 	return &X509SVID{

diff --git a/workload/proto_test.go b/workload/proto_test.go
@@ -19,9 +19,8 @@ func TestProtoToX509SVIDs(t *testing.T) {
 	require.Len(t, svidChain, 2)
 	svids := []spiffetest.X509SVID{
 		{
-			CertChain:     svidChain,
-			Key:           svidKey,
-			FederatesWith: []string{"spiffe://domain2.test"},
+			CertChain: svidChain,
+			Key:       svidKey,
 		},
 	}
 
@@ -114,14 +113,6 @@ func TestProtoToX509SVIDs(t *testing.T) {
 			},
 			err: `failed to parse svid entry 0 for spiffe id "spiffe://domain1.test/workload": failed to parse trust bundle: asn1: syntax error: truncated tag or length`,
 		},
-		{
-			name: "missing federated trust bundle",
-			resp: &spiffetest.X509SVIDResponse{
-				Bundle: domain1Bundle,
-				SVIDs:  svids,
-			},
-			err: `failed to parse svid entry 0 for spiffe id "spiffe://domain1.test/workload": missing bundle for federated domain "spiffe://domain2.test"`,
-		},
 		{
 			name: "success",
 			resp: &spiffetest.X509SVIDResponse{
@@ -154,11 +145,8 @@ func TestProtoToX509SVIDs(t *testing.T) {
 				require.Equal(t, svidIn.CertChain, svidOut.Certificates)
 				require.Equal(t, testCase.resp.Bundle, svidOut.TrustBundle)
 				require.Equal(t, svidIn.CertChain[0].URIs[0].String(), svidOut.SPIFFEID)
-
-				require.Len(t, svidOut.FederatedTrustBundles, len(svidIn.FederatesWith))
-				for _, trustDomain := range svidIn.FederatesWith {
-					require.Equal(t, testCase.resp.FederatedBundles[trustDomain], svidOut.FederatedTrustBundles[trustDomain])
-				}
+				require.Equal(t, testCase.resp.FederatedBundles, svidOut.FederatedTrustBundles)
+				require.Len(t, svidOut.FederatedTrustBundlePools, len(testCase.resp.FederatedBundles))
 			}
 		})
 	}