Merge commit from fork

* verify recovery key signatures before comitting recovery

Signed-off-by: Daniel Weiße <[email protected]>

* cli: fix private key loading function

Signed-off-by: Daniel Weiße <[email protected]>

* coordinator: increase security version number to 2

Signed-off-by: Daniel Weiße <[email protected]>

* store: tighten up interface

Signed-off-by: Daniel Weiße <[email protected]>

* stdstore: revert nil check

Signed-off-by: Daniel Weiße <[email protected]>

* coordinator: bind sealed key to state

Signed-off-by: Daniel Weiße <[email protected]>

* coordinator: split up setting and sealing of store key

Signed-off-by: Daniel Weiße <[email protected]>

* coordinator: add context to BeginReadTransaction

Signed-off-by: Daniel Weiße <[email protected]>

* coordinator: remove redundant key sealing

Signed-off-by: Daniel Weiße <[email protected]>

* clientapi: fix incorrect secret when starting read transaction

Signed-off-by: Daniel Weiße <[email protected]>

* cli: add test for pkcs11 rsa key loading

Signed-off-by: Daniel Weiße <[email protected]>

* cli: update recover command documentation

Signed-off-by: Daniel Weiße <[email protected]>

* sealer: fix doc comment

Signed-off-by: Daniel Weiße <[email protected]>

* server: remove redundant return statement

Signed-off-by: Daniel Weiße <[email protected]>

* stdstore: replace key backup with atomic file replace

Signed-off-by: Daniel Weiße <[email protected]>

* add tests for security fix

Signed-off-by: Daniel Weiße <[email protected]>

* stdstore: fix seal mode not being correctly set after recovery

Signed-off-by: Daniel Weiße <[email protected]>

* add integration test to verify key binding

Signed-off-by: Daniel Weiße <[email protected]>

* docs: update recovery workflows

Signed-off-by: Daniel Weiße <[email protected]>

* fix linting issues

Signed-off-by: Daniel Weiße <[email protected]>

---------

Signed-off-by: Daniel Weiße <[email protected]>

Author: daniel-weisse

api/api.go

@@ -9,7 +9,9 @@ package api
 import (
 	"bytes"
 	"context"
+	"crypto"
 	"crypto/ecdsa"
+	"crypto/rand"
 	"crypto/rsa"
 	"crypto/sha256"
 	"crypto/tls"
@@ -159,38 +161,33 @@ func VerifyMarbleRunDeployment(ctx context.Context, endpoint string, opts Verify
 }
 
 // Recover performs recovery on a Coordinator instance by setting the decrypted recoverySecret.
+// The signer is used to generate a signature over the recoverySecret.
+// The Coordinator will verify this signature matches one of the recovery public keys set in the manifest.
 // On success, it returns the number of remaining recovery secrets to be set,
 // as well as the verified SGX quote.
 //
 // If this function is called from inside an EGo enclave, the "marblerun_ego_enclave" build tag must be set when building the binary.
-func Recover(ctx context.Context, endpoint string, opts VerifyOptions, recoverySecret []byte) (remaining int, sgxQuote []byte, err error) {
-	opts.setDefaults()
-
-	rootCert, _, sgxQuote, err := VerifyCoordinator(ctx, endpoint, opts)
+func Recover(ctx context.Context, endpoint string, opts VerifyOptions, recoverySecret []byte, signer crypto.Signer) (remaining int, sgxQuote []byte, err error) {
+	signature, err := util.SignPKCS1v15(signer, recoverySecret)
 	if err != nil {
 		return -1, nil, err
 	}
+	return recoverCoordinator(ctx, endpoint, opts, recoverySecret, signature)
+}
 
-	client, err := rest.NewClient(endpoint, rootCert, nil)
-	if err != nil {
-		return -1, nil, fmt.Errorf("setting up client: %w", err)
-	}
-
-	// Attempt recovery using the v2 API first
-	remaining, err = recoverV2(ctx, client, recoverySecret)
-	if rest.IsNotAllowedErr(err) {
-		remaining, err = recoverV1(ctx, client, recoverySecret)
-	}
-	if err != nil {
-		return -1, nil, fmt.Errorf("sending recovery request: %w", err)
-	}
-
-	return remaining, sgxQuote, err
+// RecoverWithSignature performs recovery on a Coordinator instance by setting the decrypted recoverySecret.
+// This is the same as [Recover], but allows passing in the recoverySecretSignature directly,
+// instead of generating it using a [crypto.Signer].
+// The recoveryKeySignature must be a PKCS#1 v1.5 signature over the SHA-256 hash of recoverySecret.
+//
+// If this function is called from inside an EGo enclave, the "marblerun_ego_enclave" build tag must be set when building the binary.
+func RecoverWithSignature(ctx context.Context, endpoint string, opts VerifyOptions, recoverySecret, recoverySecretSignature []byte) (remaining int, sgxQuote []byte, err error) {
+	return recoverCoordinator(ctx, endpoint, opts, recoverySecret, recoverySecretSignature)
 }
 
 // DecryptRecoveryData decrypts recovery data returned by a Coordinator during [ManifestSet] using a parties private recovery key.
-func DecryptRecoveryData(recoveryData []byte, recoveryPrivateKey *rsa.PrivateKey) ([]byte, error) {
-	return util.DecryptOAEP(recoveryPrivateKey, recoveryData)
+func DecryptRecoveryData(recoveryData []byte, recoveryPrivateKey crypto.Decrypter) ([]byte, error) {
+	return recoveryPrivateKey.Decrypt(rand.Reader, recoveryData, &rsa.OAEPOptions{Hash: crypto.SHA256})
 }
 
 // GetStatus retrieves the status of a MarbleRun Coordinator instance.
@@ -577,3 +574,30 @@ func getMarbleCredentialsFromEnv() (tls.Certificate, *x509.Certificate, error) {
 
 	return tlsCert, coordinatorRoot, nil
 }
+
+// recoverCoordinator performs recovery on a Coordinator instance by setting the decrypted recoverySecret.
+// The signer is used to generate a signature over the recoverySecret.
+// The Coordinator will verify this signature matches one of the recovery public keys set in the manifest.
+// On success, it returns the number of remaining recovery secrets to be set,
+// as well as the verified SGX quote.
+func recoverCoordinator(ctx context.Context, endpoint string, opts VerifyOptions, recoverySecret, recoverySecretSignature []byte) (remaining int, sgxQuote []byte, err error) {
+	opts.setDefaults()
+
+	rootCert, _, sgxQuote, err := VerifyCoordinator(ctx, endpoint, opts)
+	if err != nil {
+		return -1, nil, err
+	}
+
+	client, err := rest.NewClient(endpoint, rootCert, nil)
+	if err != nil {
+		return -1, nil, fmt.Errorf("setting up client: %w", err)
+	}
+
+	// The v1 API does not support recovery, therefore only attempt the v2 API
+	remaining, err = recoverV2(ctx, client, recoverySecret, recoverySecretSignature)
+	if err != nil {
+		return -1, nil, fmt.Errorf("sending recovery request: %w", err)
+	}
+
+	return remaining, sgxQuote, err
+}

api/v1.go

@@ -20,31 +20,6 @@ import (
 	apiv1 "github.com/edgelesssys/marblerun/coordinator/server/v1"
 )
 
-// recoverV1 performs recovery of the Coordinator using the legacy v1 API.
-func recoverV1(ctx context.Context, client *rest.Client, recoverySecret []byte) (remaining int, err error) {
-	resp, err := client.Post(ctx, rest.RecoverEndpoint, rest.ContentPlain, bytes.NewReader(recoverySecret))
-	if err != nil {
-		return -1, err
-	}
-
-	var response apiv1.RecoveryStatusResponse
-	if err := json.Unmarshal(resp, &response); err != nil {
-		return -1, fmt.Errorf("unmarshalling Coordinator response: %w", err)
-	}
-
-	if response.StatusMessage == "Recovery successful." {
-		return 0, nil
-	}
-
-	remainingStr, _, _ := strings.Cut(response.StatusMessage, ": ")
-	remaining, err = strconv.Atoi(remainingStr)
-	if err != nil {
-		return -1, fmt.Errorf("parsing remaining recovery secrets: %w", err)
-	}
-
-	return remaining, nil
-}
-
 // getStatusV1 retrieves the status of the Coordinator using the legacy v1 API.
 func getStatusV1(ctx context.Context, client *rest.Client) (int, string, error) {
 	resp, err := client.Get(ctx, rest.StatusEndpoint, http.NoBody)

api/v2.go

@@ -19,8 +19,11 @@ import (
 )
 
 // recoverV2 performs recovery of the Coordinator using the v2 API.
-func recoverV2(ctx context.Context, client *rest.Client, recoverySecret []byte) (remaining int, err error) {
-	recoverySecretJSON, err := json.Marshal(apiv2.RecoveryRequest{RecoverySecret: recoverySecret})
+func recoverV2(ctx context.Context, client *rest.Client, recoverySecret, recoverySecretSignature []byte) (remaining int, err error) {
+	recoverySecretJSON, err := json.Marshal(apiv2.RecoveryRequest{
+		RecoverySecret:          recoverySecret,
+		RecoverySecretSignature: recoverySecretSignature,
+	})
 	if err != nil {
 		return -1, fmt.Errorf("marshalling request: %w", err)
 	}

cli/internal/cmd/recover.go

@@ -7,23 +7,43 @@ SPDX-License-Identifier: BUSL-1.1
 package cmd
 
 import (
+	"crypto"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/pem"
+	"errors"
+	"fmt"
+	"os"
+
 	"github.com/edgelesssys/marblerun/api"
 	"github.com/edgelesssys/marblerun/cli/internal/file"
+	"github.com/edgelesssys/marblerun/cli/internal/pkcs11"
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 )
 
 // NewRecoverCmd returns the recover command.
 func NewRecoverCmd() *cobra.Command {
 	cmd := &cobra.Command{
-		Use:     "recover <recovery_key_decrypted> <IP:PORT>",
-		Short:   "Recover the MarbleRun Coordinator from a sealed state",
-		Long:    "Recover the MarbleRun Coordinator from a sealed state",
+		Use:   "recover <recovery_key_file> <IP:PORT>",
+		Short: "Recover the MarbleRun Coordinator from a sealed state",
+		Long: "Recover the MarbleRun Coordinator from a sealed state.\n" +
+			"<recovery_key_file> may be either a decrypted recovery secret, or an encrypted recovery secret,\n" +
+			"in which case the private key is used to decrypt the secret.",
 		Example: "marblerun recover recovery_key_decrypted $MARBLERUN",
 		Args:    cobra.ExactArgs(2),
 		RunE:    runRecover,
 	}
 
+	cmd.Flags().StringP("key", "k", "", "Path to a the recovery private key to decrypt and/or sign the recovery key")
+	cmd.Flags().String("pkcs11-config", "", "Path to a PKCS#11 configuration file to load the recovery private key with")
+	cmd.Flags().String("pkcs11-key-id", "", "ID of the private key in the PKCS#11 token")
+	cmd.Flags().String("pkcs11-key-label", "", "Label of the private key in the PKCS#11 token")
+	must(cobra.MarkFlagFilename(cmd.Flags(), "pkcs11-config", "json"))
+	cmd.MarkFlagsOneRequired("pkcs11-key-id", "pkcs11-key-label", "key")
+	cmd.MarkFlagsMutuallyExclusive("pkcs11-config", "key")
+
 	return cmd
 }
 
@@ -42,7 +62,22 @@ func runRecover(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	remaining, sgxQuote, err := api.Recover(cmd.Context(), hostname, verifyOpts, recoveryKey)
+	keyHandle, cancel, err := getRecoveryKeySigner(cmd)
+	if err != nil {
+		return err
+	}
+	defer func() {
+		if err := cancel(); err != nil {
+			cmd.PrintErrf("Failed to close PKCS #11 session: %s\n", err)
+		}
+	}()
+
+	recoveryKey, err = maybeDecryptRecoveryKey(recoveryKey, keyHandle)
+	if err != nil {
+		return err
+	}
+
+	remaining, sgxQuote, err := api.Recover(cmd.Context(), hostname, verifyOpts, recoveryKey, keyHandle)
 	if err != nil {
 		return err
 	}
@@ -58,3 +93,53 @@ func runRecover(cmd *cobra.Command, args []string) error {
 	}
 	return nil
 }
+
+func getRecoveryKeySigner(cmd *cobra.Command) (pkcs11.SignerDecrypter, func() error, error) {
+	privKeyFile, err := cmd.Flags().GetString("key")
+	if err != nil {
+		return nil, nil, err
+	}
+
+	if privKeyFile == "" {
+		pkcs11ConfigFile, err := cmd.Flags().GetString("pkcs11-config")
+		if err != nil {
+			return nil, nil, err
+		}
+		pkcs11KeyID, err := cmd.Flags().GetString("pkcs11-key-id")
+		if err != nil {
+			return nil, nil, err
+		}
+		pkcs11KeyLabel, err := cmd.Flags().GetString("pkcs11-key-label")
+		if err != nil {
+			return nil, nil, err
+		}
+		return pkcs11.LoadRSAPrivateKey(pkcs11ConfigFile, pkcs11KeyID, pkcs11KeyLabel)
+	}
+
+	privKeyPEM, err := os.ReadFile(privKeyFile)
+	if err != nil {
+		return nil, nil, err
+	}
+	privateKeyBlock, _ := pem.Decode(privKeyPEM)
+	if privateKeyBlock == nil {
+		return nil, nil, fmt.Errorf("%q did not contain a valid PEM block", privKeyFile)
+	}
+	privK, err := x509.ParsePKCS8PrivateKey(privateKeyBlock.Bytes)
+	if err != nil {
+		return nil, nil, err
+	}
+	signer, ok := privK.(pkcs11.SignerDecrypter)
+	if !ok {
+		return nil, nil, errors.New("loaded private key does not fulfill required interface")
+	}
+	return signer, func() error { return nil }, nil
+}
+
+// maybeDecryptRecoveryKey tries to decrypt the given recoveryKey using OAEP.
+// If the recoveryKey is already 16 bytes long, it is returned as is.
+func maybeDecryptRecoveryKey(recoveryKey []byte, decrypter crypto.Decrypter) ([]byte, error) {
+	if len(recoveryKey) != 16 {
+		return decrypter.Decrypt(rand.Reader, recoveryKey, &rsa.OAEPOptions{Hash: crypto.SHA256})
+	}
+	return recoveryKey, nil
+}

cli/internal/pkcs11/pkcs11.go

@@ -16,8 +16,15 @@ import (
 	"github.com/ThalesGroup/crypto11"
 )
 
-// LoadX509KeyPair loads a [tls.Certificate] using the provided PKCS#11 configuration file.
-// The returned cancel function must be called to release the PKCS#11 resources only after the certificate is no longer needed.
+// SignerDecrypter is a combined interface for [crypto.Signer] and [crypto.Decrypter].
+// An RSA private key in a PKCS #11 token implements this interface.
+type SignerDecrypter interface {
+	crypto.Signer
+	crypto.Decrypter
+}
+
+// LoadX509KeyPair loads a [tls.Certificate] using the provided PKCS #11 configuration file.
+// The returned cancel function must be called to release the PKCS #11 resources only after the certificate is no longer needed.
 func LoadX509KeyPair(pkcs11ConfigPath string, keyID, keyLabel, certID, certLabel string) (crt tls.Certificate, cancel func() error, err error) {
 	pkcs11, err := crypto11.ConfigureFromFile(pkcs11ConfigPath)
 	if err != nil {
@@ -59,6 +66,39 @@ func LoadX509KeyPair(pkcs11ConfigPath string, keyID, keyLabel, certID, certLabel
 	}, pkcs11.Close, nil
 }
 
+// LoadRSAPrivateKey loads a [SignerDecrypter] using the provided PKCS #11 configuration file.
+// The returned cancel function must be called to release the PKCS #11 resources only after the key is no longer needed.
+func LoadRSAPrivateKey(pkcs11ConfigPath string, keyID, keyLabel string) (signer SignerDecrypter, cancel func() error, err error) {
+	pkcs11, err := crypto11.ConfigureFromFile(pkcs11ConfigPath)
+	if err != nil {
+		return nil, nil, err
+	}
+	defer func() {
+		if err != nil {
+			err = errors.Join(err, pkcs11.Close())
+		}
+	}()
+
+	var keyIDBytes, keyLabelBytes []byte
+	if keyID != "" {
+		keyIDBytes = []byte(keyID)
+	}
+	if keyLabel != "" {
+		keyLabelBytes = []byte(keyLabel)
+	}
+
+	privK, err := loadPrivateKey(pkcs11, keyIDBytes, keyLabelBytes)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	signer, ok := privK.(crypto11.SignerDecrypter)
+	if !ok {
+		return nil, nil, errors.New("loaded private key does not support decryption")
+	}
+	return signer, pkcs11.Close, err
+}
+
 func loadPrivateKey(pkcs11 *crypto11.Context, id, label []byte) (crypto.Signer, error) {
 	priv, err := pkcs11.FindKeyPair(id, label)
 	if err != nil {