Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 35 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
391aac8
add ASK caching in joinservice
msanft Sep 12, 2023
77283d9
use cached ASK in Azure SEV-SNP attestation
msanft Sep 14, 2023
4ed3a1a
update test charts
msanft Sep 14, 2023
0a2d666
fix linter
msanft Sep 14, 2023
71ae332
fix typ
msanft Sep 14, 2023
fe44a5c
make caching mechanism less provider-specific
msanft Sep 21, 2023
f31383b
update buildfiles
msanft Sep 22, 2023
44c3cd5
add `omitempty` flag
msanft Sep 22, 2023
5bd9ae3
frontload certificate getter
msanft Sep 22, 2023
d8cc18b
rename frontloaded function
msanft Sep 22, 2023
e337bab
pass cached certificates to constructor
msanft Sep 22, 2023
b2c9159
fix race condition
msanft Sep 22, 2023
1f22c01
fix marshalling of empty certs
msanft Sep 25, 2023
3ad03ce
fix validator usage
msanft Sep 25, 2023
f3894a7
[wip] add certcache tests
msanft Sep 25, 2023
590a689
add certcache tests
msanft Sep 25, 2023
469c6f3
tidy
msanft Sep 25, 2023
225d9b1
fix validator test
msanft Sep 25, 2023
dbd4109
remove unused fields in validator
msanft Sep 25, 2023
9859854
fix certificate precedence
msanft Sep 26, 2023
101bc94
use separate context
msanft Sep 26, 2023
5c4779a
tidy
msanft Sep 26, 2023
e2f65c5
linter fixes
msanft Sep 27, 2023
f5761bc
linter fixes
msanft Sep 27, 2023
7c943f7
Remove unnecessary comment
msanft Sep 27, 2023
561a460
use background context
msanft Sep 27, 2023
73c2860
Use error format directive
msanft Sep 28, 2023
1b07e8b
`azure` -> `Azure`
msanft Sep 28, 2023
2d74227
improve error messages
msanft Sep 28, 2023
2aaf98c
add x509 -> PEM util function
msanft Sep 28, 2023
26f1ef9
use crypto util functions
msanft Sep 28, 2023
17d9928
fix certificate replacement logic
msanft Sep 28, 2023
692315b
only require ASK from certcache
msanft Sep 28, 2023
32888f9
tidy
msanft Sep 28, 2023
87f29b2
fix comment typo
msanft Sep 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rules:
- configmaps
verbs:
- get
- create
- apiGroups:
- "update.edgeless.systems"
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rules:
- configmaps
verbs:
- get
- create
- apiGroups:
- "update.edgeless.systems"
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rules:
- configmaps
verbs:
- get
- create
- apiGroups:
- "update.edgeless.systems"
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rules:
- configmaps
verbs:
- get
- create
- apiGroups:
- "update.edgeless.systems"
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rules:
- configmaps
verbs:
- get
- create
- apiGroups:
- "update.edgeless.systems"
resources:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rules:
- configmaps
verbs:
- get
- create
- apiGroups:
- "update.edgeless.systems"
resources:
Expand Down
1 change: 1 addition & 0 deletions internal/attestation/azure/snp/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
55 changes: 44 additions & 11 deletions internal/attestation/azure/snp/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand All @@ -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,
},
},
},
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand All @@ -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.
Expand Down
75 changes: 65 additions & 10 deletions internal/attestation/azure/snp/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"bytes"
"context"
"crypto/sha256"
"crypto/x509"
"encoding/base64"
"encoding/binary"
"encoding/hex"
Expand Down Expand Up @@ -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,
Expand All @@ -196,6 +206,8 @@ func TestInstanceInfoAttestation(t *testing.T) {
},
nil,
),
expectedArk: testdataArk,
expectedAsk: testdataAsk,
},
"retrieve certchain": {
report: defaultReport,
Expand All @@ -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,
Expand All @@ -219,6 +262,8 @@ func TestInstanceInfoAttestation(t *testing.T) {
},
nil,
),
expectedArk: testdataArk,
expectedAsk: testdataAsk,
},
"report too short": {
report: defaultReport[:len(defaultReport)-100],
Expand All @@ -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 {
msanft marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/attestation/choose/choose.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
2 changes: 1 addition & 1 deletion internal/attestation/variant/variant.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions internal/config/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand All @@ -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 {
Expand Down
Loading