joinservice: cache certificates for Azure SEV-SNP attestation (#2336)

* add ASK caching in joinservice Signed-off-by: Moritz Sanft <[email protected]> * use cached ASK in Azure SEV-SNP attestation Signed-off-by: Moritz Sanft <[email protected]> * update test charts Signed-off-by: Moritz Sanft <[email protected]> * fix linter Signed-off-by: Moritz Sanft <[email protected]> * fix typ Signed-off-by: Moritz Sanft <[email protected]> * make caching mechanism less provider-specific Signed-off-by: Moritz Sanft <[email protected]> * update buildfiles Signed-off-by: Moritz Sanft <[email protected]> * add `omitempty` flag Co-authored-by: Daniel Weiße <[email protected]> * frontload certificate getter Co-authored-by: Daniel Weiße <[email protected]> * rename frontloaded function Signed-off-by: Moritz Sanft <[email protected]> * pass cached certificates to constructor Signed-off-by: Moritz Sanft <[email protected]> * fix race condition Signed-off-by: Moritz Sanft <[email protected]> * fix marshalling of empty certs Signed-off-by: Moritz Sanft <[email protected]> * fix validator usage Signed-off-by: Moritz Sanft <[email protected]> * [wip] add certcache tests Signed-off-by: Moritz Sanft <[email protected]> * add certcache tests Signed-off-by: Moritz Sanft <[email protected]> * tidy Signed-off-by: Moritz Sanft <[email protected]> * fix validator test Signed-off-by: Moritz Sanft <[email protected]> * remove unused fields in validator Signed-off-by: Moritz Sanft <[email protected]> * fix certificate precedence Signed-off-by: Moritz Sanft <[email protected]> * use separate context Signed-off-by: Moritz Sanft <[email protected]> * tidy Signed-off-by: Moritz Sanft <[email protected]> * linter fixes Signed-off-by: Moritz Sanft <[email protected]> * linter fixes Signed-off-by: Moritz Sanft <[email protected]> * Remove unnecessary comment Co-authored-by: Thomas Tendyck <[email protected]> * use background context Signed-off-by: Moritz Sanft <[email protected]> * Use error format directive Co-authored-by: Thomas Tendyck <[email protected]> * `azure` -> `Azure` Co-authored-by: Thomas Tendyck <[email protected]> * improve error messages Signed-off-by: Moritz Sanft <[email protected]> * add x509 -> PEM util function Signed-off-by: Moritz Sanft <[email protected]> * use crypto util functions Signed-off-by: Moritz Sanft <[email protected]> * fix certificate replacement logic Signed-off-by: Moritz Sanft <[email protected]> * only require ASK from certcache Signed-off-by: Moritz Sanft <[email protected]> * tidy Signed-off-by: Moritz Sanft <[email protected]> * fix comment typo Signed-off-by: Moritz Sanft <[email protected]> --------- Signed-off-by: Moritz Sanft <[email protected]> Co-authored-by: Daniel Weiße <[email protected]> Co-authored-by: Thomas Tendyck <[email protected]>
edgelesssys · Sep 29, 2023 · a5021c5 · a5021c5
1 parent 68d8b29
commit a5021c5
Show file tree

Hide file tree

Showing 39 changed files with 1,197 additions and 50 deletions.
diff --git a/...elm/charts/edgeless/constellation-services/charts/join-service/templates/clusterrole.yaml b/...elm/charts/edgeless/constellation-services/charts/join-service/templates/clusterrole.yaml
@@ -28,6 +28,7 @@ rules:
   - configmaps
   verbs:
   - get
+  - create
 - apiGroups:
   - "update.edgeless.systems"
   resources:

diff --git a/...l/helm/testdata/AWS/constellation-services/charts/join-service/templates/clusterrole.yaml b/...l/helm/testdata/AWS/constellation-services/charts/join-service/templates/clusterrole.yaml
@@ -28,6 +28,7 @@ rules:
   - configmaps
   verbs:
   - get
+  - create
 - apiGroups:
   - "update.edgeless.systems"
   resources:

diff --git a/...helm/testdata/Azure/constellation-services/charts/join-service/templates/clusterrole.yaml b/...helm/testdata/Azure/constellation-services/charts/join-service/templates/clusterrole.yaml
@@ -28,6 +28,7 @@ rules:
   - configmaps
   verbs:
   - get
+  - create
 - apiGroups:
   - "update.edgeless.systems"
   resources:

diff --git a/...l/helm/testdata/GCP/constellation-services/charts/join-service/templates/clusterrole.yaml b/...l/helm/testdata/GCP/constellation-services/charts/join-service/templates/clusterrole.yaml
@@ -28,6 +28,7 @@ rules:
   - configmaps
   verbs:
   - get
+  - create
 - apiGroups:
   - "update.edgeless.systems"
   resources:

diff --git a/.../testdata/OpenStack/constellation-services/charts/join-service/templates/clusterrole.yaml b/.../testdata/OpenStack/constellation-services/charts/join-service/templates/clusterrole.yaml
@@ -28,6 +28,7 @@ rules:
   - configmaps
   verbs:
   - get
+  - create
 - apiGroups:
   - "update.edgeless.systems"
   resources:

diff --git a/.../helm/testdata/QEMU/constellation-services/charts/join-service/templates/clusterrole.yaml b/.../helm/testdata/QEMU/constellation-services/charts/join-service/templates/clusterrole.yaml
@@ -28,6 +28,7 @@ rules:
   - configmaps
   verbs:
   - get
+  - create
 - apiGroups:
   - "update.edgeless.systems"
   resources:

diff --git a/internal/attestation/azure/snp/BUILD.bazel b/internal/attestation/azure/snp/BUILD.bazel
@@ -19,6 +19,7 @@ go_library(
         "//internal/attestation/vtpm",
         "//internal/cloud/azure",
         "//internal/config",
+        "//internal/constants",
         "@com_github_edgelesssys_go_azguestattestation//maa",
         "@com_github_google_go_sev_guest//abi",
         "@com_github_google_go_sev_guest//kds",

diff --git a/internal/attestation/azure/snp/validator.go b/internal/attestation/azure/snp/validator.go
@@ -24,6 +24,7 @@ import (
 	"github.com/edgelesssys/constellation/v2/internal/attestation/variant"
 	"github.com/edgelesssys/constellation/v2/internal/attestation/vtpm"
 	"github.com/edgelesssys/constellation/v2/internal/config"
+	"github.com/edgelesssys/constellation/v2/internal/constants"
 	"github.com/google/go-sev-guest/abi"
 	"github.com/google/go-sev-guest/kds"
 	spb "github.com/google/go-sev-guest/proto/sevsnp"
@@ -101,19 +102,28 @@ func NewValidator(cfg *config.AzureSEVSNP, log attestation.Logger) *Validator {
 // getTrustedKey establishes trust in the given public key.
 // It does so by verifying the SNP attestation document.
 func (v *Validator) getTrustedKey(ctx context.Context, attDoc vtpm.AttestationDocument, extraData []byte) (crypto.PublicKey, error) {
+	trustedAsk := (*x509.Certificate)(&v.config.AMDSigningKey) // ASK, cached by the Join-Service
+	trustedArk := (*x509.Certificate)(&v.config.AMDRootKey)    // ARK, specified in the Constellation config
+
+	// fallback certificates, used if not present in THIM response.
+	cachedCerts := sevSnpCerts{
+		ask: trustedAsk,
+		ark: trustedArk,
+	}
+
 	// transform the instanceInfo received from Microsoft into a verifiable attestation report format.
 	var instanceInfo azureInstanceInfo
 	if err := json.Unmarshal(attDoc.InstanceInfo, &instanceInfo); err != nil {
 		return nil, fmt.Errorf("unmarshalling instanceInfo: %w", err)
 	}
-	att, err := instanceInfo.attestationWithCerts(v.log, v.getter)
+
+	att, err := instanceInfo.attestationWithCerts(v.log, v.getter, cachedCerts)
 	if err != nil {
 		return nil, fmt.Errorf("parsing attestation report: %w", err)
 	}
 
-	// Verify the attestation report's certificates.
-	trustedArk := x509.Certificate(v.config.AMDRootKey)             // ARK, specified in Constellation config.
-	ask, err := x509.ParseCertificate(att.CertificateChain.AskCert) // ASK, as reported from THIM / KDS.
+	// ASK, as cached in joinservice or reported from THIM / KDS.
+	ask, err := x509.ParseCertificate(att.CertificateChain.AskCert)
 	if err != nil {
 		return nil, fmt.Errorf("parsing ASK certificate: %w", err)
 	}
@@ -125,7 +135,7 @@ func (v *Validator) getTrustedKey(ctx context.Context, attDoc vtpm.AttestationDo
 					Product: "Milan",
 					ProductCerts: &trust.ProductCerts{
 						Ask: ask,
-						Ark: &trustedArk,
+						Ark: trustedArk,
 					},
 				},
 			},
@@ -246,9 +256,13 @@ type azureInstanceInfo struct {
 }
 
 // attestationWithCerts returns a formatted version of the attestation report and its certificates from the instanceInfo.
-// if the VCEK certificate or the certificate chain is not present, the given getter is used to retrieve them
-// from the AMD KDS.
-func (a *azureInstanceInfo) attestationWithCerts(logger attestation.Logger, getter trust.HTTPSGetter) (*spb.Attestation, error) {
+// Certificates are retrieved in the following precedence:
+// 1. ASK or ARK from THIM
+// 2. ASK or ARK from fallbackCerts
+// 3. ASK or ARK from AMD KDS.
+func (a *azureInstanceInfo) attestationWithCerts(logger attestation.Logger, getter trust.HTTPSGetter,
+	fallbackCerts sevSnpCerts,
+) (*spb.Attestation, error) {
 	report, err := abi.ReportToProto(a.AttestationReport)
 	if err != nil {
 		return nil, fmt.Errorf("converting report to proto: %w", err)
@@ -282,17 +296,29 @@ func (a *azureInstanceInfo) attestationWithCerts(logger attestation.Logger, gett
 		att.CertificateChain.VcekCert = vcek
 	}
 
-	// If the certificate chain is present, parse it and format it.
+	// If the certificate chain from THIM is present, parse it and format it.
 	ask, ark, err := a.parseCertChain()
 	if err != nil {
 		logger.Warnf("Error parsing certificate chain: %v", err)
 	}
 	if ask != nil {
+		logger.Infof("Using ASK certificate from Azure THIM")
 		att.CertificateChain.AskCert = ask.Raw
 	}
 	if ark != nil {
+		logger.Infof("Using ARK certificate from Azure THIM")
 		att.CertificateChain.ArkCert = ark.Raw
 	}
+
+	// If a cached ASK or an ARK from the Constellation config is present, use it.
+	if att.CertificateChain.AskCert == nil && fallbackCerts.ask != nil {
+		logger.Infof("Using cached ASK certificate")
+		att.CertificateChain.AskCert = fallbackCerts.ask.Raw
+	}
+	if att.CertificateChain.ArkCert == nil && fallbackCerts.ark != nil {
+		logger.Infof("Using ARK certificate from %s", constants.ConfigFilename)
+		att.CertificateChain.ArkCert = fallbackCerts.ark.Raw
+	}
 	// Otherwise, retrieve it from AMD KDS.
 	if att.CertificateChain.AskCert == nil || att.CertificateChain.ArkCert == nil {
 		logger.Infof(
@@ -304,17 +330,24 @@ func (a *azureInstanceInfo) attestationWithCerts(logger attestation.Logger, gett
 		if err != nil {
 			return nil, fmt.Errorf("retrieving certificate chain from AMD KDS: %w", err)
 		}
-		if att.CertificateChain.AskCert == nil {
+		if att.CertificateChain.AskCert == nil && kdsCertChain.Ask != nil {
+			logger.Infof("Using ASK certificate from AMD KDS")
 			att.CertificateChain.AskCert = kdsCertChain.Ask.Raw
 		}
-		if att.CertificateChain.ArkCert == nil {
+		if att.CertificateChain.ArkCert == nil && kdsCertChain.Ask != nil {
+			logger.Infof("Using ARK certificate from AMD KDS")
 			att.CertificateChain.ArkCert = kdsCertChain.Ark.Raw
 		}
 	}
 
 	return att, nil
 }
 
+type sevSnpCerts struct {
+	ask *x509.Certificate
+	ark *x509.Certificate
+}
+
 // parseCertChain parses the certificate chain from the instanceInfo into x509-formatted ASK and ARK certificates.
 // If less than 2 certificates are present, only the present certificate is returned.
 // If more than 2 certificates are present, an error is returned.

diff --git a/internal/attestation/azure/snp/validator_test.go b/internal/attestation/azure/snp/validator_test.go
@@ -10,6 +10,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/sha256"
+	"crypto/x509"
 	"encoding/base64"
 	"encoding/binary"
 	"encoding/hex"
@@ -169,22 +170,31 @@ func TestParseVCEK(t *testing.T) {
 	}
 }
 
-// TestInstanceInfoAttestation tests the basic unmarshalling of the attestation report.
+// TestInstanceInfoAttestation tests the basic unmarshalling of the attestation report and the ASK / ARK precedence.
 func TestInstanceInfoAttestation(t *testing.T) {
 	defaultReport := testdata.AttestationReport
+	testdataArk, testdataAsk := mustCertChainToPem(t, testdata.CertChain)
+	exampleCert := &x509.Certificate{
+		Raw: []byte{1, 2, 3},
+	}
 	cfg := config.DefaultForAzureSEVSNP()
 
 	testCases := map[string]struct {
-		report    []byte
-		vcek      []byte
-		certChain []byte
-		getter    *stubHTTPSGetter
-		wantErr   bool
+		report        []byte
+		vcek          []byte
+		certChain     []byte
+		fallbackCerts sevSnpCerts
+		getter        *stubHTTPSGetter
+		expectedArk   *x509.Certificate
+		expectedAsk   *x509.Certificate
+		wantErr       bool
 	}{
 		"success": {
-			report:    defaultReport,
-			vcek:      testdata.AzureThimVCEK,
-			certChain: testdata.CertChain,
+			report:      defaultReport,
+			vcek:        testdata.AzureThimVCEK,
+			certChain:   testdata.CertChain,
+			expectedArk: testdataArk,
+			expectedAsk: testdataAsk,
 		},
 		"retrieve vcek": {
 			report:    defaultReport,
@@ -196,6 +206,8 @@ func TestInstanceInfoAttestation(t *testing.T) {
 				},
 				nil,
 			),
+			expectedArk: testdataArk,
+			expectedAsk: testdataAsk,
 		},
 		"retrieve certchain": {
 			report: defaultReport,
@@ -207,6 +219,37 @@ func TestInstanceInfoAttestation(t *testing.T) {
 				},
 				nil,
 			),
+			expectedArk: testdataArk,
+			expectedAsk: testdataAsk,
+		},
+		"use fallback certs": {
+			report: defaultReport,
+			vcek:   testdata.AzureThimVCEK,
+			fallbackCerts: sevSnpCerts{
+				ask: exampleCert,
+				ark: exampleCert,
+			},
+			getter: newStubHTTPSGetter(
+				&urlResponseMatcher{},
+				nil,
+			),
+			expectedArk: exampleCert,
+			expectedAsk: exampleCert,
+		},
+		"use certchain with fallback certs": {
+			report:    defaultReport,
+			certChain: testdata.CertChain,
+			vcek:      testdata.AzureThimVCEK,
+			fallbackCerts: sevSnpCerts{
+				ask: &x509.Certificate{},
+				ark: &x509.Certificate{},
+			},
+			getter: newStubHTTPSGetter(
+				&urlResponseMatcher{},
+				nil,
+			),
+			expectedArk: testdataArk,
+			expectedAsk: testdataAsk,
 		},
 		"retrieve vcek and certchain": {
 			report: defaultReport,
@@ -219,6 +262,8 @@ func TestInstanceInfoAttestation(t *testing.T) {
 				},
 				nil,
 			),
+			expectedArk: testdataArk,
+			expectedAsk: testdataAsk,
 		},
 		"report too short": {
 			report:  defaultReport[:len(defaultReport)-100],
@@ -245,7 +290,7 @@ func TestInstanceInfoAttestation(t *testing.T) {
 				VCEK:              tc.vcek,
 			}
 
-			att, err := instanceInfo.attestationWithCerts(logger.NewTest(t), tc.getter)
+			att, err := instanceInfo.attestationWithCerts(logger.NewTest(t), tc.getter, tc.fallbackCerts)
 			if tc.wantErr {
 				assert.Error(err)
 			} else {
@@ -263,11 +308,21 @@ func TestInstanceInfoAttestation(t *testing.T) {
 				assert.True(tcbValues.TeeSpl >= cfg.TEEVersion.Value)
 				assert.True(tcbValues.SnpSpl >= cfg.SNPVersion.Value)
 				assert.True(tcbValues.UcodeSpl >= cfg.MicrocodeVersion.Value)
+				assert.Equal(tc.expectedArk.Raw, att.CertificateChain.ArkCert)
+				assert.Equal(tc.expectedAsk.Raw, att.CertificateChain.AskCert)
 			}
 		})
 	}
 }
 
+func mustCertChainToPem(t *testing.T, certchain []byte) (ark, ask *x509.Certificate) {
+	t.Helper()
+	a := azureInstanceInfo{CertChain: certchain}
+	ask, ark, err := a.parseCertChain()
+	require.NoError(t, err)
+	return ark, ask
+}
+
 type stubHTTPSGetter struct {
 	urlResponseMatcher *urlResponseMatcher // maps responses to requested URLs
 	err                error

diff --git a/internal/attestation/choose/choose.go b/internal/attestation/choose/choose.go
@@ -66,6 +66,6 @@ func Validator(cfg config.AttestationCfg, log attestation.Logger) (atls.Validato
 	case *config.DummyCfg:
 		return atls.NewFakeValidator(variant.Dummy{}), nil
 	default:
-		return nil, fmt.Errorf("unknown attestation variant")
+		return nil, fmt.Errorf("unknown attestation variant: %s", cfg.GetVariant())
 	}
 }
diff --git a/internal/attestation/variant/variant.go b/internal/attestation/variant/variant.go
@@ -134,7 +134,7 @@ func ValidProvider(provider cloudprovider.Provider, variant Variant) bool {
 	return false
 }
 
-// Dummy OID for testfing.
+// Dummy OID for testing.
 type Dummy struct{}
 
 // OID returns the struct's object identifier.

diff --git a/internal/config/attestation.go b/internal/config/attestation.go
@@ -65,18 +65,27 @@ type Certificate x509.Certificate
 
 // MarshalJSON marshals the certificate to PEM.
 func (c Certificate) MarshalJSON() ([]byte, error) {
+	if len(c.Raw) == 0 {
+		return json.Marshal(new(string))
+	}
 	pem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: c.Raw})
 	return json.Marshal(string(pem))
 }
 
 // MarshalYAML marshals the certificate to PEM.
 func (c Certificate) MarshalYAML() (any, error) {
+	if len(c.Raw) == 0 {
+		return "", nil
+	}
 	pem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: c.Raw})
 	return string(pem), nil
 }
 
 // UnmarshalJSON unmarshals the certificate from PEM.
 func (c *Certificate) UnmarshalJSON(data []byte) error {
+	if len(data) == 0 {
+		return nil
+	}
 	return c.unmarshal(func(val any) error {
 		return json.Unmarshal(data, val)
 	})
@@ -92,6 +101,9 @@ func (c *Certificate) unmarshal(unmarshalFunc func(any) error) error {
 	if err := unmarshalFunc(&pemData); err != nil {
 		return err
 	}
+	if pemData == "" {
+		return nil
+	}
 	block, _ := pem.Decode([]byte(pemData))
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {