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

cli: allow users to use a custom nonce for SGX quote verification using --nonce flag #644

Merged
merged 6 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion .github/workflows/update-cli-reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ jobs:
git config --global --add safe.directory "$GITHUB_WORKSPACE"
- name: Generate reference docs
run: go run . | cat header.md - > ../../cli.md
run: XDG_CONFIG_HOME=\$HOME/.config go run . | cat header.md - > ../../cli.md
working-directory: hack/clidocgen

- name: Get commit sha
Expand Down
1 change: 1 addition & 0 deletions cli/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ To install and configure MarbleRun, run:
rootCmd.PersistentFlags().BoolP("insecure", "i", false, "Set to skip quote verification, needed when running in simulation mode")
rootCmd.PersistentFlags().StringSlice("accepted-tcb-statuses", []string{"UpToDate", "SWHardeningNeeded"}, "Comma-separated list of user accepted TCB statuses")
rootCmd.PersistentFlags().StringP("namespace", "n", helm.Namespace, "Kubernetes namespace of the MarbleRun installation")
rootCmd.PersistentFlags().String("nonce", "", "(Optional) nonce to use for quote verification. If set, the Coordinator will generate a quote over sha256(CoordinatorCert + nonce)")

must(rootCmd.MarkPersistentFlagFilename("coordinator-cert", "pem", "crt"))
must(rootCmd.MarkPersistentFlagFilename("era-config", "json"))
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cmd/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func runCertificate(saveCert func(io.Writer, *file.Handler, []*pem.Block) error,

certs, err := rest.VerifyCoordinator(
cmd.Context(), cmd.OutOrStdout(), hostname,
flags.eraConfig, flags.k8sNamespace, flags.insecure, flags.acceptedTCBStatuses,
flags.eraConfig, flags.k8sNamespace, flags.nonce, flags.insecure, flags.acceptedTCBStatuses,
)
if err != nil {
return fmt.Errorf("retrieving certificate from Coordinator: %w", err)
Expand Down
7 changes: 7 additions & 0 deletions cli/internal/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ type restFlags struct {
// acceptedTCBStatuses is a list of TCB statuses that are accepted by the CLI.
// This can be used to allow connections to Coordinator instances running on outdated hardware or firmware.
acceptedTCBStatuses []string
// nonce is a user supplied nonce to be used in the attestation process.
nonce []byte
}

// parseRestFlags parses the command line flags used to configure the REST client.
Expand All @@ -62,12 +64,17 @@ func parseRestFlags(flags *pflag.FlagSet) (restFlags, error) {
if err != nil {
return restFlags{}, err
}
nonce, err := flags.GetString("nonce")
if err != nil {
return restFlags{}, err
}

return restFlags{
k8sNamespace: k8snamespace,
eraConfig: eraConfig,
insecure: insecure,
acceptedTCBStatuses: acceptedTCBStatuses,
nonce: []byte(nonce),
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cmd/manifestGet.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func runManifestGet(cmd *cobra.Command, args []string) error {
}
caCert, err := rest.VerifyCoordinator(
cmd.Context(), cmd.OutOrStdout(), hostname,
restFlags.eraConfig, restFlags.k8sNamespace, restFlags.insecure, restFlags.acceptedTCBStatuses,
restFlags.eraConfig, restFlags.k8sNamespace, restFlags.nonce, restFlags.insecure, restFlags.acceptedTCBStatuses,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cmd/manifestSet.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func runManifestSet(cmd *cobra.Command, args []string) (retErr error) {

caCert, err := rest.VerifyCoordinator(
cmd.Context(), cmd.OutOrStdout(), hostname,
restFlags.eraConfig, restFlags.k8sNamespace, restFlags.insecure, restFlags.acceptedTCBStatuses,
restFlags.eraConfig, restFlags.k8sNamespace, restFlags.nonce, restFlags.insecure, restFlags.acceptedTCBStatuses,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cmd/manifestVerify.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func runManifestVerify(cmd *cobra.Command, args []string) error {
}
caCert, err := rest.VerifyCoordinator(
cmd.Context(), cmd.OutOrStdout(), hostname,
restFlags.eraConfig, restFlags.k8sNamespace, restFlags.insecure, restFlags.acceptedTCBStatuses,
restFlags.eraConfig, restFlags.k8sNamespace, restFlags.nonce, restFlags.insecure, restFlags.acceptedTCBStatuses,
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/cmd/recover.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func runRecover(cmd *cobra.Command, args []string) error {
// Therefore we need to verify the Coordinator is running in the expected enclave instead
caCert, err := rest.VerifyCoordinator(
cmd.Context(), cmd.OutOrStdout(), hostname,
flags.eraConfig, flags.k8sNamespace, flags.insecure, flags.acceptedTCBStatuses,
flags.eraConfig, flags.k8sNamespace, flags.nonce, flags.insecure, flags.acceptedTCBStatuses,
)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions cli/internal/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ func (c *Client) do(req *http.Request) ([]byte, error) {
// VerifyCoordinator verifies the connection to the MarbleRun Coordinator.
func VerifyCoordinator(
ctx context.Context, out io.Writer, host, configFilename, k8sNamespace string,
insecure bool, acceptedTCBStatuses []string,
nonce []byte, insecure bool, acceptedTCBStatuses []string,
) ([]*pem.Block, error) {
// skip verification if specified
if insecure {
Expand Down Expand Up @@ -188,7 +188,7 @@ func VerifyCoordinator(
return nil, fmt.Errorf("unmarshalling era config: %w", err)
}

pemBlock, tcbStatus, _, err := attestation.GetCertificate(ctx, host, nil, eraCfg)
pemBlock, tcbStatus, _, err := attestation.GetCertificate(ctx, host, nonce, eraCfg)
validity, err := tcb.CheckStatus(tcbStatus, err, acceptedTCBStatuses)
if err != nil {
return nil, err
Expand Down
16 changes: 13 additions & 3 deletions coordinator/clientapi/clientapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ type core interface {
GenerateSecrets(
map[string]manifest.Secret, uuid.UUID, *x509.Certificate, *ecdsa.PrivateKey, *ecdsa.PrivateKey,
) (map[string]manifest.Secret, error)
GetQuote() []byte
GetQuote(reportData []byte) ([]byte, error)
GenerateQuote([]byte) error
}

Expand Down Expand Up @@ -93,7 +93,7 @@ func New(txHandle transactionHandle, recovery recovery.Recovery, core core, log
//
// Returns the remote attestation quote of its own certificate alongside this certificate,
// which allows to verify the Coordinator's integrity and authentication for use of the ClientAPI.
func (a *ClientAPI) GetCertQuote(ctx context.Context) (cert string, certQuote []byte, err error) {
func (a *ClientAPI) GetCertQuote(ctx context.Context, nonce []byte) (cert string, certQuote []byte, err error) {
a.log.Info("GetCertQuote called")
defer a.core.Unlock()
if err := a.core.RequireState(ctx, state.AcceptingManifest, state.AcceptingMarbles, state.Recovery); err != nil {
Expand Down Expand Up @@ -139,8 +139,18 @@ func (a *ClientAPI) GetCertQuote(ctx context.Context) (cert string, certQuote []
return "", nil, errors.New("pem.EncodeToMemory failed for intermediate certificate")
}

// Get existing quote for root cert, or generate a new one over provided nonce
var reportData []byte
if len(nonce) > 0 {
reportData = append(rootCert.Raw, nonce...)
}
quote, err := a.core.GetQuote(reportData)
if err != nil {
return "", nil, fmt.Errorf("getting quote: %w", err)
}

a.log.Info("GetCertQuote successful")
return string(pemCertIntermediate) + string(pemCertRoot), a.core.GetQuote(), nil
return string(pemCertIntermediate) + string(pemCertRoot), quote, nil
}

// GetManifestSignature returns the hash of the manifest.
Expand Down
31 changes: 24 additions & 7 deletions coordinator/clientapi/clientapi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,30 +55,35 @@ func TestGetCertQuote(t *testing.T) {
}

testCases := map[string]struct {
store store.Store
core *fakeCore
wantErr bool
store store.Store
core *fakeCore
nonce []byte
wantQuote []byte
wantErr bool
}{
"success state accepting Marbles": {
store: prepareDefaultStore(),
core: &fakeCore{
state: state.AcceptingMarbles,
quote: []byte("quote"),
},
wantQuote: []byte("quote"),
},
"success state accepting manifest": {
store: prepareDefaultStore(),
core: &fakeCore{
state: state.AcceptingManifest,
quote: []byte("quote"),
},
wantQuote: []byte("quote"),
},
"success state recovery": {
store: prepareDefaultStore(),
core: &fakeCore{
state: state.Recovery,
quote: []byte("quote"),
},
wantQuote: []byte("quote"),
},
"unsupported state": {
store: prepareDefaultStore(),
Expand Down Expand Up @@ -120,6 +125,15 @@ func TestGetCertQuote(t *testing.T) {
},
wantErr: true,
},
"get quote with nonce": {
store: prepareDefaultStore(),
core: &fakeCore{
state: state.AcceptingMarbles,
quote: []byte("quote"),
},
nonce: []byte("nonce"),
wantQuote: []byte("nonce" + "quote"),
},
}

for name, tc := range testCases {
Expand All @@ -141,14 +155,14 @@ func TestGetCertQuote(t *testing.T) {
rootCert = testutil.GetCertificate(t, tc.store, constants.SKCoordinatorRootCert)
}

cert, quote, err := api.GetCertQuote(context.Background())
cert, quote, err := api.GetCertQuote(context.Background(), tc.nonce)
if tc.wantErr {
assert.Error(err)
return
}

require.NoError(err)
assert.Equal(tc.core.quote, quote)
assert.Equal(tc.wantQuote, quote)
assert.Equal(mustEncodeToPem(t, intermediateCert)+mustEncodeToPem(t, rootCert), cert)
})
}
Expand Down Expand Up @@ -715,8 +729,11 @@ func (c *fakeCore) GenerateSecrets(
return secrets, nil
}

func (c *fakeCore) GetQuote() []byte {
return c.quote
func (c *fakeCore) GetQuote(reportData []byte) ([]byte, error) {
if reportData != nil {
return append([]byte("nonce"), c.quote...), nil
}
return c.quote, nil
}

func (c *fakeCore) GenerateQuote(quoteData []byte) error {
Expand Down
12 changes: 10 additions & 2 deletions coordinator/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,16 @@ func (c *Core) GetTLSMarbleRootCertificate(clientHello *tls.ClientHelloInfo) (*t
}

// GetQuote returns the quote of the Coordinator.
func (c *Core) GetQuote() []byte {
return c.quote
// If reportData is not nil, a new quote is generated over the data and returned.
func (c *Core) GetQuote(reportData []byte) ([]byte, error) {
daniel-weisse marked this conversation as resolved.
Show resolved Hide resolved
if len(reportData) == 0 {
return c.quote, nil
}
quote, err := c.qi.Issue(reportData)
if err != nil && err.Error() != "OE_UNSUPPORTED" {
return nil, QuoteError{err}
}
return quote, nil
}

// GenerateQuote generates a quote for the Coordinator using the given certificate.
Expand Down
63 changes: 63 additions & 0 deletions coordinator/core/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"errors"
"math/big"
"testing"
"time"
Expand Down Expand Up @@ -317,3 +318,65 @@ func TestUnsetRestart(t *testing.T) {
c2Cert := testutil.GetCertificate(t, c2.txHandle, constants.SKCoordinatorRootCert)
assert.NotEqual(*cCert, *c2Cert)
}

func TestGetQuote(t *testing.T) {
testCases := map[string]struct {
reportData []byte
savedQuote []byte
issuer stubIssuer
wantErr bool
}{
"no report data": {
reportData: nil,
savedQuote: []byte("quote"),
issuer: stubIssuer{},
},
"with report data": {
reportData: []byte("report data"),
savedQuote: []byte("quote"),
issuer: stubIssuer{},
},
"issuer error": {
reportData: []byte("report data"),
savedQuote: []byte("quote"),
issuer: stubIssuer{err: assert.AnError},
wantErr: true,
},
"OE_UNSUPPORTED error is ignored": {
issuer: stubIssuer{err: errors.New("OE_UNSUPPORTED")},
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
assert := assert.New(t)

zapLogger := zaptest.NewLogger(t)
core := Core{
qi: &tc.issuer,
log: zapLogger,
quote: tc.savedQuote,
}

quote, err := core.GetQuote(tc.reportData)
if tc.wantErr {
assert.Error(err)
return
}
assert.NoError(err)
if len(tc.reportData) == 0 {
assert.Equal(tc.savedQuote, quote)
} else {
assert.Equal(tc.reportData, quote) // stubIssuer returns the input message as quote
}
})
}
}

type stubIssuer struct {
err error
}

func (s *stubIssuer) Issue(message []byte) ([]byte, error) {
return message, s.err
}
Loading
Loading