Merge pull request #5398 from oasisprotocol/peternose/bugfix/fix-upgr…

…ade-tests

tests/upgrade: Minor upgrade bug fixes and improvements

.changelog/5398.bugfix.1.md

@@ -0,0 +1 @@
+keymanager: Fix public key decoding for legacy key manager clients

.changelog/5398.bugfix.2.md

@@ -0,0 +1 @@
+go/worker/keymanager: Fix race condition when accessing runtime status

.changelog/5398.feature.md

@@ -0,0 +1 @@
+go/oasis-test-runner: Add encrypt/decrypt txs to test client scenarios

go/oasis-test-runner/scenario/e2e/runtime/test_client.go

@@ -1,11 +1,13 @@
 package runtime
 
 import (
+	"bytes"
 	"context"
 	"crypto"
 	"fmt"
 	"math/rand"
 
+	beacon "github.com/oasisprotocol/oasis-core/go/beacon/api"
 	"github.com/oasisprotocol/oasis-core/go/common"
 	"github.com/oasisprotocol/oasis-core/go/common/cbor"
 	"github.com/oasisprotocol/oasis-core/go/common/crypto/drbg"
@@ -157,6 +159,33 @@ func (cli *TestClient) submit(ctx context.Context, req interface{}, rng rand.Sou
 			return fmt.Errorf("response does not have expected value (got: '%v', expected: '%v')", rsp, req.Response)
 		}
 
+	case EncryptDecryptTx:
+		ciphertext, err := cli.sc.submitKeyValueRuntimeEncryptTx(
+			ctx,
+			KeyValueRuntimeID,
+			rng.Uint64(),
+			req.Epoch,
+			req.KeyPairID,
+			req.Message,
+		)
+		if err != nil {
+			return fmt.Errorf("failed to encrypt message: %w", err)
+		}
+		plaintext, err := cli.sc.submitKeyValueRuntimeDecryptTx(
+			ctx,
+			KeyValueRuntimeID,
+			rng.Uint64(),
+			req.Epoch,
+			req.KeyPairID,
+			ciphertext,
+		)
+		if err != nil {
+			return fmt.Errorf("failed to decrypt ciphertext: %w", err)
+		}
+		if !bytes.Equal(plaintext, req.Message) {
+			return fmt.Errorf("decrypted message does not have expected value (got: '%v', expected: '%v')", plaintext, req.Message)
+		}
+
 	case InsertKeyValueTx:
 		rsp, err := cli.sc.submitKeyValueRuntimeInsertTx(
 			ctx,
@@ -252,26 +281,108 @@ func NewTestClient() *TestClient {
 	}
 }
 
-func (sc *Scenario) submitAndDecodeRuntimeTx(
+func (sc *Scenario) submitRuntimeTxAndDecode(
 	ctx context.Context,
 	id common.Namespace,
 	nonce uint64,
 	method string,
 	args interface{},
-) (string, error) {
+	rsp interface{},
+) error {
 	rawRsp, err := sc.submitRuntimeTx(ctx, id, nonce, method, args)
 	if err != nil {
-		return "", fmt.Errorf("failed to submit %s tx to runtime: %w", method, err)
+		return fmt.Errorf("failed to submit %s tx to runtime: %w", method, err)
+	}
+
+	if err = cbor.Unmarshal(rawRsp, rsp); err != nil {
+		return fmt.Errorf("failed to unmarshal %s tx response from runtime: %w", method, err)
 	}
 
+	return nil
+}
+
+func (sc *Scenario) submitRuntimeTxAndDecodeString(
+	ctx context.Context,
+	id common.Namespace,
+	nonce uint64,
+	method string,
+	args interface{},
+) (string, error) {
 	var rsp string
-	if err = cbor.Unmarshal(rawRsp, &rsp); err != nil {
-		return "", fmt.Errorf("failed to unmarshal %s tx response from runtime: %w", method, err)
+	if err := sc.submitRuntimeTxAndDecode(ctx, id, nonce, method, args, &rsp); err != nil {
+		return "", err
 	}
+	return rsp, nil
+}
 
+func (sc *Scenario) submitRuntimeTxAndDecodeByteSlice(
+	ctx context.Context,
+	id common.Namespace,
+	nonce uint64,
+	method string,
+	args interface{},
+) ([]byte, error) {
+	var rsp []byte
+	if err := sc.submitRuntimeTxAndDecode(ctx, id, nonce, method, args, &rsp); err != nil {
+		return nil, err
+	}
 	return rsp, nil
 }
 
+func (sc *Scenario) submitKeyValueRuntimeEncryptTx(
+	ctx context.Context,
+	id common.Namespace,
+	nonce uint64,
+	epoch beacon.EpochTime,
+	keyPairID string,
+	plaintext []byte,
+) ([]byte, error) {
+	sc.Logger.Info("encrypting",
+		"epoch", epoch,
+		"key_pair_id", keyPairID,
+		"plaintext", plaintext,
+	)
+
+	args := struct {
+		Epoch     beacon.EpochTime `json:"epoch"`
+		KeyPairID string           `json:"key_pair_id"`
+		Plaintext []byte           `json:"plaintext"`
+	}{
+		Epoch:     epoch,
+		KeyPairID: keyPairID,
+		Plaintext: plaintext,
+	}
+
+	return sc.submitRuntimeTxAndDecodeByteSlice(ctx, id, nonce, "encrypt", args)
+}
+
+func (sc *Scenario) submitKeyValueRuntimeDecryptTx(
+	ctx context.Context,
+	id common.Namespace,
+	nonce uint64,
+	epoch beacon.EpochTime,
+	keyPairID string,
+	ciphertext []byte,
+) ([]byte, error) {
+	sc.Logger.Info("decrypting",
+		"epoch", epoch,
+		"key_pair_id", keyPairID,
+		"ciphertext", ciphertext,
+	)
+
+	args := struct {
+		Epoch      beacon.EpochTime `json:"epoch"`
+		KeyPairID  string           `json:"key_pair_id"`
+		Ciphertext []byte           `json:"ciphertext"`
+	}{
+		Epoch:      epoch,
+		KeyPairID:  keyPairID,
+		Ciphertext: ciphertext,
+	}
+
+	return sc.submitRuntimeTxAndDecodeByteSlice(ctx, id, nonce, "decrypt", args)
+}
+
 func (sc *Scenario) submitKeyValueRuntimeInsertTx(
 	ctx context.Context,
 	id common.Namespace,
@@ -298,9 +409,9 @@ func (sc *Scenario) submitKeyValueRuntimeInsertTx(
 	}
 
 	if encrypted {
-		return sc.submitAndDecodeRuntimeTx(ctx, id, nonce, "enc_insert", args)
+		return sc.submitRuntimeTxAndDecodeString(ctx, id, nonce, "enc_insert", args)
 	}
-	return sc.submitAndDecodeRuntimeTx(ctx, id, nonce, "insert", args)
+	return sc.submitRuntimeTxAndDecodeString(ctx, id, nonce, "insert", args)
 }
 
 func (sc *Scenario) submitKeyValueRuntimeGetTx(
@@ -326,9 +437,9 @@ func (sc *Scenario) submitKeyValueRuntimeGetTx(
 	}
 
 	if encrypted {
-		return sc.submitAndDecodeRuntimeTx(ctx, id, nonce, "enc_get", args)
+		return sc.submitRuntimeTxAndDecodeString(ctx, id, nonce, "enc_get", args)
 	}
-	return sc.submitAndDecodeRuntimeTx(ctx, id, nonce, "get", args)
+	return sc.submitRuntimeTxAndDecodeString(ctx, id, nonce, "get", args)
 }
 
 func (sc *Scenario) submitKeyValueRuntimeRemoveTx(
@@ -354,9 +465,9 @@ func (sc *Scenario) submitKeyValueRuntimeRemoveTx(
 	}
 
 	if encrypted {
-		return sc.submitAndDecodeRuntimeTx(ctx, id, nonce, "enc_remove", args)
+		return sc.submitRuntimeTxAndDecodeString(ctx, id, nonce, "enc_remove", args)
 	}
-	return sc.submitAndDecodeRuntimeTx(ctx, id, nonce, "remove", args)
+	return sc.submitRuntimeTxAndDecodeString(ctx, id, nonce, "remove", args)
 }
 
 func (sc *Scenario) submitKeyValueRuntimeGetRuntimeIDTx(
@@ -366,7 +477,7 @@ func (sc *Scenario) submitKeyValueRuntimeGetRuntimeIDTx(
 ) (string, error) {
 	sc.Logger.Info("retrieving runtime ID")
 
-	rsp, err := sc.submitAndDecodeRuntimeTx(ctx, id, nonce, "get_runtime_id", nil)
+	rsp, err := sc.submitRuntimeTxAndDecodeString(ctx, id, nonce, "get_runtime_id", nil)
 	if err != nil {
 		return "", fmt.Errorf("failed to query remote runtime ID: %w", err)
 	}

go/oasis-test-runner/scenario/e2e/runtime/test_client_scenario.go

@@ -2,6 +2,8 @@ package runtime
 
 import (
 	"fmt"
+
+	beacon "github.com/oasisprotocol/oasis-core/go/beacon/api"
 )
 
 var (
@@ -124,6 +126,14 @@ type KeyValueQuery struct {
 	Round    uint64
 }
 
+// EncryptDecryptTx encrypts and decrypts a message while verifying if the original message
+// matches the decrypted result.
+type EncryptDecryptTx struct {
+	Message   []byte
+	KeyPairID string
+	Epoch     beacon.EpochTime
+}
+
 // InsertKeyValueTx inserts a key/value pair to the database, and verifies that the response
 // (previous value) contains the expected data.
 type InsertKeyValueTx struct {
@@ -182,3 +192,16 @@ func NewTestClientScenario(requests []interface{}) TestClientScenario {
 		return nil
 	}
 }
+
+// JoinTestClientScenarios joins an arbitrary number of test client scenarios into a single scenario
+// that executes them in the order they were provided.
+func JoinTestClientScenarios(scenarios ...TestClientScenario) TestClientScenario {
+	return func(submit func(req interface{}) error) error {
+		for _, scenario := range scenarios {
+			if err := scenario(submit); err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+}

go/worker/keymanager/worker.go

@@ -238,6 +238,7 @@ func (w *Worker) CallEnclave(ctx context.Context, data []byte, kind enclaverpc.K
 	if err != nil {
 		w.logger.Error("failed to dispatch RPC call to runtime",
 			"err", err,
+			"kind", kind,
 		)
 		return nil, err
 	}
@@ -1075,11 +1076,12 @@ func (w *Worker) handleRuntimeHostEvent(ev *host.Event) {
 		// control. Without it, the enclave won't be able to replicate the master secrets
 		// needed for initialization.
 		if w.enclaveStatus == nil {
+			rtStatus := w.rtStatus
 			w.roleProvider.SetAvailableWithCallback(func(n *node.Node) error {
-				rt := n.AddOrUpdateRuntime(w.runtime.ID(), w.rtStatus.version)
-				rt.Version = w.rtStatus.version
+				rt := n.AddOrUpdateRuntime(w.runtime.ID(), rtStatus.version)
+				rt.Version = rtStatus.version
 				rt.ExtraInfo = nil
-				rt.Capabilities.TEE = w.rtStatus.capabilityTEE
+				rt.Capabilities.TEE = rtStatus.capabilityTEE
 				return nil
 			}, func(context.Context) error {
 				w.logger.Info("key manager registered (pre-registration)")

keymanager/src/runtime/methods.rs

@@ -153,8 +153,9 @@ pub fn get_public_ephemeral_key(
     // they never verify their signatures and are not aware that signed ephemeral keys expire.
     // To ensure backwards compatibility we clear the expiration epoch. This should be removed
     // in the future once all clients are upgraded.
-    if ctx.session_info.is_some() {
-        sig.expiration = None
+    // XXX: Remove this after 23.0.x.
+    if ctx.is_secure {
+        sig.expiration = None;
     }
 
     Ok(sig)

runtime/src/dispatcher.rs

@@ -32,8 +32,7 @@ use crate::{
         dispatcher::Dispatcher as RpcDispatcher,
         session::SessionInfo,
         types::{
-            Kind, Kind as RpcKind, Message as RpcMessage, Request as RpcRequest,
-            Response as RpcResponse,
+            Kind as RpcKind, Message as RpcMessage, Request as RpcRequest, Response as RpcResponse,
         },
         Context as RpcContext,
     },
@@ -324,14 +323,14 @@ impl Dispatcher {
                 );
 
                 match kind {
-                    Kind::NoiseSession => self.dispatch_secure_rpc(state, request).await,
-                    Kind::InsecureQuery => self.dispatch_insecure_rpc(state, request).await,
-                    Kind::LocalQuery => self.dispatch_local_rpc(state, request).await,
+                    RpcKind::NoiseSession => self.dispatch_secure_rpc(state, request).await,
+                    RpcKind::InsecureQuery => self.dispatch_insecure_rpc(state, request).await,
+                    RpcKind::LocalQuery => self.dispatch_local_rpc(state, request).await,
                 }
             }
             Body::RuntimeLocalRPCCallRequest { request } => {
                 debug!(self.logger, "Received RPC call request";
-                    "kind" => ?Kind::LocalQuery,
+                    "kind" => ?RpcKind::LocalQuery,
                 );
 
                 self.dispatch_local_rpc(state, request).await
@@ -798,7 +797,7 @@ impl Dispatcher {
                     // Note: MKVS commit is omitted, this MUST be global side-effect free.
 
                     debug!(self.logger, "RPC call dispatch complete";
-                        "kind" => ?Kind::NoiseSession,
+                        "kind" => ?RpcKind::NoiseSession,
                     );
 
                     let mut buffer = vec![];
@@ -850,7 +849,7 @@ impl Dispatcher {
         // Note: MKVS commit is omitted, this MUST be global side-effect free.
 
         debug!(self.logger, "RPC call dispatch complete";
-            "kind" => ?Kind::InsecureQuery,
+            "kind" => ?RpcKind::InsecureQuery,
         );
 
         Ok(Body::RuntimeRPCCallResponse { response })
@@ -872,7 +871,7 @@ impl Dispatcher {
         let response = cbor::to_vec(response);
 
         debug!(self.logger, "RPC call dispatch complete";
-            "kind" => ?Kind::LocalQuery,
+            "kind" => ?RpcKind::LocalQuery,
         );
 
         Ok(Body::RuntimeLocalRPCCallResponse { response })
@@ -889,11 +888,17 @@ impl Dispatcher {
         let protocol = state.protocol.clone();
         let consensus_verifier = state.consensus_verifier.clone();
         let rpc_dispatcher = state.rpc_dispatcher.clone();
+        let is_secure = kind == RpcKind::NoiseSession;
 
         let response = tokio::task::spawn_blocking(move || {
             let untrusted_local = Arc::new(ProtocolUntrustedLocalStorage::new(protocol.clone()));
-            let rpc_ctx =
-                RpcContext::new(identity, session_info, consensus_verifier, &untrusted_local);
+            let rpc_ctx = RpcContext::new(
+                identity,
+                is_secure,
+                session_info,
+                consensus_verifier,
+                &untrusted_local,
+            );
 
             rpc_dispatcher.dispatch(rpc_ctx, request, kind)
         })

runtime/src/enclave_rpc/context.rs

@@ -10,6 +10,8 @@ struct NoRuntimeContext;
 pub struct Context<'a> {
     /// The current runtime identity if any.
     pub identity: Arc<Identity>,
+    /// Whether the RPC call is using a secure channel.
+    pub is_secure: bool,
     /// Information about the session the RPC call was delivered over.
     pub session_info: Option<Arc<SessionInfo>>,
     /// Consensus verifier.
@@ -24,12 +26,14 @@ impl<'a> Context<'a> {
     /// Construct new transaction context.
     pub fn new(
         identity: Arc<Identity>,
+        is_secure: bool,
         session_info: Option<Arc<SessionInfo>>,
         consensus_verifier: Arc<dyn Verifier>,
         untrusted_local_storage: &'a dyn KeyValue,
     ) -> Self {
         Self {
             identity,
+            is_secure,
             session_info,
             consensus_verifier,
             runtime: Box::new(NoRuntimeContext),