From 35c5ec1a73f2f0814e421e9f011e455a1bb10172 Mon Sep 17 00:00:00 2001 From: Jernej Kos Date: Mon, 13 Jan 2020 11:19:01 +0100 Subject: [PATCH] go/consensus/tendermint/apps: Avoid loading all registered runtimes This is a performance optimization to avoid loading a list of all registered runtimes into memory in cases when only a specific runtime is actually needed. --- .changelog/2538.feature.md | 4 + .../tendermint/apps/keymanager/keymanager.go | 2 +- .../tendermint/apps/registry/transactions.go | 30 ++-- go/keymanager/api/api.go | 5 +- go/registry/api/api.go | 165 +++++++++--------- 5 files changed, 97 insertions(+), 109 deletions(-) create mode 100644 .changelog/2538.feature.md diff --git a/.changelog/2538.feature.md b/.changelog/2538.feature.md new file mode 100644 index 00000000000..54c2ef10d41 --- /dev/null +++ b/.changelog/2538.feature.md @@ -0,0 +1,4 @@ +Optimize registry runtime lookups during node registration. + +A performance optimization to avoid loading a list of all registered runtimes into memory in cases +when only a specific runtime is actually needed. diff --git a/go/consensus/tendermint/apps/keymanager/keymanager.go b/go/consensus/tendermint/apps/keymanager/keymanager.go index d8125d67244..54d1b34d772 100644 --- a/go/consensus/tendermint/apps/keymanager/keymanager.go +++ b/go/consensus/tendermint/apps/keymanager/keymanager.go @@ -187,7 +187,7 @@ func (app *keymanagerApplication) generateStatus(kmrt *registry.Runtime, oldStat continue } - initResponse, err := api.VerifyExtraInfo(kmrt, nodeRt, ts) + initResponse, err := api.VerifyExtraInfo(app.logger, kmrt, nodeRt, ts) if err != nil { app.logger.Error("failed to validate ExtraInfo", "err", err, diff --git a/go/consensus/tendermint/apps/registry/transactions.go b/go/consensus/tendermint/apps/registry/transactions.go index ef3251682e5..7953c531234 100644 --- a/go/consensus/tendermint/apps/registry/transactions.go +++ b/go/consensus/tendermint/apps/registry/transactions.go @@ -156,16 +156,16 @@ func (app *registryApplication) registerNode( // nolint: gocyclo ) return err } - // TODO: Avoid loading a list of all runtimes. - regRuntimes, err := state.AllRuntimes() - if err != nil { - app.logger.Error("RegisterNode: failed to obtain registry runtimes", - "err", err, - "signed_node", sigNode, - ) - return err - } - newNode, err := registry.VerifyRegisterNodeArgs(params, app.logger, sigNode, untrustedEntity, ctx.Now(), ctx.IsInitChain(), regRuntimes, state) + newNode, paidRuntimes, err := registry.VerifyRegisterNodeArgs( + params, + app.logger, + sigNode, + untrustedEntity, + ctx.Now(), + ctx.IsInitChain(), + state, + state, + ) if err != nil { return err } @@ -255,16 +255,6 @@ func (app *registryApplication) registerNode( // nolint: gocyclo additionalEpochs = 0 } } - var paidRuntimes []*registry.Runtime - for _, nodeRt := range newNode.Runtimes { - var rt *registry.Runtime - rt, err = state.AnyRuntime(nodeRt.ID) - if err != nil { - return fmt.Errorf("failed to fetch runtime: %w", err) - } - - paidRuntimes = append(paidRuntimes, rt) - } feeCount := len(paidRuntimes) * int(additionalEpochs) if err = ctx.Gas().UseGas(feeCount, registry.GasOpRuntimeEpochMaintenance, params.GasCosts); err != nil { return err diff --git a/go/keymanager/api/api.go b/go/keymanager/api/api.go index 70f184e5fe5..2c16f51930a 100644 --- a/go/keymanager/api/api.go +++ b/go/keymanager/api/api.go @@ -11,6 +11,7 @@ import ( "github.com/oasislabs/oasis-core/go/common/crypto/signature" memorySigner "github.com/oasislabs/oasis-core/go/common/crypto/signature/signers/memory" "github.com/oasislabs/oasis-core/go/common/errors" + "github.com/oasislabs/oasis-core/go/common/logging" "github.com/oasislabs/oasis-core/go/common/node" "github.com/oasislabs/oasis-core/go/common/pubsub" registry "github.com/oasislabs/oasis-core/go/registry/api" @@ -107,7 +108,7 @@ func (r *SignedInitResponse) Verify(pk signature.PublicKey) error { // VerifyExtraInfo verifies and parses the per-node + per-runtime ExtraInfo // blob for a key manager. -func VerifyExtraInfo(rt *registry.Runtime, nodeRt *node.Runtime, ts time.Time) (*InitResponse, error) { +func VerifyExtraInfo(logger *logging.Logger, rt *registry.Runtime, nodeRt *node.Runtime, ts time.Time) (*InitResponse, error) { var ( hw node.TEEHardware rak signature.PublicKey @@ -121,7 +122,7 @@ func VerifyExtraInfo(rt *registry.Runtime, nodeRt *node.Runtime, ts time.Time) ( } if hw != rt.TEEHardware { return nil, fmt.Errorf("keymanger: TEEHardware mismatch") - } else if err := registry.VerifyNodeRuntimeEnclaveIDs(nil, nodeRt, []*registry.Runtime{rt}, ts); err != nil { + } else if err := registry.VerifyNodeRuntimeEnclaveIDs(logger, nodeRt, rt, ts); err != nil { return nil, err } diff --git a/go/registry/api/api.go b/go/registry/api/api.go index 423663b3ced..468b257b672 100644 --- a/go/registry/api/api.go +++ b/go/registry/api/api.go @@ -288,9 +288,17 @@ type NodeLookup interface { // RuntimeLookup interface implements various ways for the verification // functions to look-up runtimes in the registry's state. type RuntimeLookup interface { - // Runtime looks up a runtime by its identifier. - // It returns either the runtime with given id or error. + // Runtime looks up a runtime by its identifier and returns it. + // + // This excludes any suspended runtimes, use SuspendedRuntime to query suspended runtimes only. Runtime(id common.Namespace) (*Runtime, error) + + // SuspendedRuntime looks up a suspended runtime by its identifier and + // returns it. + SuspendedRuntime(id common.Namespace) (*Runtime, error) + + // AnyRuntime looks up either an active or suspended runtime by its identifier and returns it. + AnyRuntime(id common.Namespace) (*Runtime, error) } // VerifyRegisterEntityArgs verifies arguments for RegisterEntity. @@ -345,6 +353,8 @@ func VerifyRegisterEntityArgs(logger *logging.Logger, sigEnt *entity.SignedEntit } // VerifyRegisterNodeArgs verifies arguments for RegisterNode. +// +// Returns the node descriptor and a list of runtime descriptors the node is registering for. func VerifyRegisterNodeArgs( // nolint: gocyclo params *ConsensusParameters, logger *logging.Logger, @@ -352,12 +362,12 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo entity *entity.Entity, now time.Time, isGenesis bool, - regRuntimes []*Runtime, + runtimeLookup RuntimeLookup, nodeLookup NodeLookup, -) (*node.Node, error) { +) (*node.Node, []*Runtime, error) { var n node.Node if sigNode == nil { - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } var ctx signature.Context @@ -372,7 +382,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo logger.Error("RegisterNode: invalid signature", "signed_node", sigNode, ) - return nil, ErrInvalidSignature + return nil, nil, ErrInvalidSignature } // This should never happen, unless there's a bug in the caller. @@ -381,7 +391,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "node", n, "entity", entity, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } // Determine which key should be expected to have signed the node descriptor. @@ -403,7 +413,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "node", n, "entity", entity, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } // Validate that the node is signed by the correct signer. @@ -413,7 +423,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "node", n, "entity", entity, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } // Make sure that a node has at least one valid role. @@ -422,55 +432,55 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo logger.Error("RegisterNode: no roles specified", "node", n, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument case n.HasRoles(node.RoleReserved): logger.Error("RegisterNode: invalid role specified", "node", n, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } // TODO: Key manager nodes maybe should be restricted to only being a // key manager at the expense of breaking some of our test configs. + var runtimes []*Runtime switch len(n.Runtimes) { case 0: if n.HasRoles(RuntimesRequiredRoles) { logger.Error("RegisterNode: no runtimes in registration", "node", n, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } default: rtMap := make(map[common.Namespace]bool) - regRtMap := make(map[common.Namespace]bool) - - for _, rt := range regRuntimes { - regRtMap[rt.ID] = true - } for _, rt := range n.Runtimes { if rtMap[rt.ID] { logger.Error("RegisterNode: duplicate runtime IDs", - "id", rt.ID, + "runtime_id", rt.ID, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } rtMap[rt.ID] = true // Make sure that the claimed runtime actually exists. - if !regRtMap[rt.ID] { - logger.Error("RegisterNode: runtime does not exist", - "id", rt.ID, + regRt, err := runtimeLookup.AnyRuntime(rt.ID) + if err != nil { + logger.Error("RegisterNode: failed to fetch supported runtime", + "err", err, + "runtime_id", rt.ID, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } // If the node indicates TEE support for any of it's runtimes, // validate the attestation evidence. - if err := VerifyNodeRuntimeEnclaveIDs(logger, rt, regRuntimes, now); err != nil { - return nil, err + if err := VerifyNodeRuntimeEnclaveIDs(logger, rt, regRt, now); err != nil { + return nil, nil, err } + + runtimes = append(runtimes, regRt) } } @@ -479,7 +489,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo logger.Error("RegisterNode: invalid consensus id", "node", n, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } consensusAddressRequired := n.HasRoles(ConsensusAddressRequiredRoles) if err := verifyAddresses(params, consensusAddressRequired, n.Consensus.Addresses); err != nil { @@ -488,7 +498,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "node", n, "consensus_addrs", addrs, ) - return nil, err + return nil, nil, err } // If node is a key manager, ensure that it is owned by the key manager @@ -498,7 +508,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo logger.Error("RegisterNode: key manager not owned by key manager operator", "node", n, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } } @@ -509,7 +519,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "node", n, "err", err, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } committeeAddressRequired := n.HasRoles(CommitteeAddressRequiredRoles) if err := verifyAddresses(params, committeeAddressRequired, n.Committee.Addresses); err != nil { @@ -518,7 +528,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "node", n, "committee_addrs", addrs, ) - return nil, err + return nil, nil, err } // Validate P2PInfo. @@ -526,7 +536,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo logger.Error("RegisterNode: invalid P2P id", "node", n, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } p2pAddressRequired := n.HasRoles(P2PAddressRequiredRoles) if err := verifyAddresses(params, p2pAddressRequired, n.P2P.Addresses); err != nil { @@ -535,7 +545,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "node", n, "p2p_addrs", addrs, ) - return nil, err + return nil, nil, err } // Make sure that the consensus and P2P keys, as well as the committee @@ -548,7 +558,7 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo logger.Error("RegisterNode: node consensus and P2P IDs must differ", "node", n, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } existingNode, err := nodeLookup.NodeByConsensusOrP2PKey(n.Consensus.ID) @@ -557,14 +567,14 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "err", err, "consensus_id", n.Consensus.ID.String(), ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } if existingNode != nil && existingNode.ID != n.ID { logger.Error("RegisterNode: duplicate node consensus ID", "node_id", n.ID, "existing_node_id", existingNode.ID, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } existingNode, err = nodeLookup.NodeByConsensusOrP2PKey(n.P2P.ID) @@ -573,14 +583,14 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo "err", err, "p2p_id", n.P2P.ID.String(), ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } if existingNode != nil && existingNode.ID != n.ID { logger.Error("RegisterNode: duplicate node p2p ID", "node_id", n.ID, "existing_node_id", existingNode.ID, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } existingNode, err = nodeLookup.NodeByCertificate(n.Committee.Certificate) @@ -588,21 +598,21 @@ func VerifyRegisterNodeArgs( // nolint: gocyclo logger.Error("RegisterNode: failed to get node by committee certificate", "err", err, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } if existingNode != nil && existingNode.ID != n.ID { logger.Error("RegisterNode: duplicate node committee certificate", "node_id", n.ID, "existing_node_id", existingNode.ID, ) - return nil, ErrInvalidArgument + return nil, nil, ErrInvalidArgument } - return &n, nil + return &n, runtimes, nil } // VerifyNodeRuntimeEnclaveIDs verifies TEE-specific attributes of the node's runtime. -func VerifyNodeRuntimeEnclaveIDs(logger *logging.Logger, rt *node.Runtime, regRuntimes []*Runtime, ts time.Time) error { +func VerifyNodeRuntimeEnclaveIDs(logger *logging.Logger, rt *node.Runtime, regRt *Runtime, ts time.Time) error { // If no TEE available, do nothing. if rt.Capabilities.TEE == nil { return nil @@ -612,7 +622,6 @@ func VerifyNodeRuntimeEnclaveIDs(logger *logging.Logger, rt *node.Runtime, regRu case node.TEEHardwareInvalid: case node.TEEHardwareIntelSGX: // Check MRENCLAVE/MRSIGNER. - var eidValid bool var avrBundle ias.AVRBundle if err := cbor.Unmarshal(rt.Capabilities.TEE.Attestation, &avrBundle); err != nil { return err @@ -629,54 +638,38 @@ func VerifyNodeRuntimeEnclaveIDs(logger *logging.Logger, rt *node.Runtime, regRu return err } - regRtLoop: - for _, regRt := range regRuntimes { - // Make sure we compare EnclaveIdentity of the same registered RuntimeIDs only! - if !regRt.ID.Equal(&rt.ID) { - continue - } - - if regRt.TEEHardware != rt.Capabilities.TEE.Hardware { - rtJSON, _ := json.Marshal(rt) - regRtJSON, _ := json.Marshal(regRt) - quoteJSON, _ := json.Marshal(q) - logger.Error("VerifyNodeRuntimeEnclaveIDs: runtime TEE.Hardware mismatch", - "quote", quoteJSON, - "node.Runtime", rtJSON, - "registered runtime", regRtJSON, - "ts", ts, - ) - return ErrTEEHardwareMismatch - } + if regRt.TEEHardware != rt.Capabilities.TEE.Hardware { + logger.Error("VerifyNodeRuntimeEnclaveIDs: runtime TEE.Hardware mismatch", + "quote", q, + "node_runtime", rt, + "registry_runtime", regRt, + "ts", ts, + ) + return ErrTEEHardwareMismatch + } - var vi VersionInfoIntelSGX - if err := cbor.Unmarshal(regRt.Version.TEE, &vi); err != nil { - return err - } - for _, eid := range vi.Enclaves { - eidMrenclave := eid.MrEnclave - eidMrsigner := eid.MrSigner - // Compare MRENCLAVE/MRSIGNER to the one stored in the registry. - if bytes.Equal(eidMrenclave[:], q.Report.MRENCLAVE[:]) && bytes.Equal(eidMrsigner[:], q.Report.MRSIGNER[:]) { - eidValid = true - break regRtLoop - } + var vi VersionInfoIntelSGX + if err := cbor.Unmarshal(regRt.Version.TEE, &vi); err != nil { + return err + } + var eidValid bool + for _, eid := range vi.Enclaves { + eidMrenclave := eid.MrEnclave + eidMrsigner := eid.MrSigner + // Compare MRENCLAVE/MRSIGNER to the one stored in the registry. + if bytes.Equal(eidMrenclave[:], q.Report.MRENCLAVE[:]) && bytes.Equal(eidMrsigner[:], q.Report.MRSIGNER[:]) { + eidValid = true + break } } if !eidValid { - if logger != nil { - rtJSON, _ := json.Marshal(rt) - regRuntimesJSON, _ := json.Marshal(regRuntimes) - quoteJSON, _ := json.Marshal(q) - logger.Error("VerifyNodeRuntimeEnclaveIDs: bad enclave ID", - "quote", quoteJSON, - "node.Runtime", rtJSON, - "registered runtimes", regRuntimesJSON, - "ts", ts, - ) - } - + logger.Error("VerifyNodeRuntimeEnclaveIDs: bad enclave ID", + "quote", q, + "node_runtime", rt, + "registry_runtime", regRt, + "ts", ts, + ) return ErrBadEnclaveIdentity } default: @@ -685,7 +678,7 @@ func VerifyNodeRuntimeEnclaveIDs(logger *logging.Logger, rt *node.Runtime, regRu if err := rt.Capabilities.TEE.Verify(ts); err != nil { logger.Error("VerifyNodeRuntimeEnclaveIDs: failed to validate attestation", - "runtime", rt.ID, + "runtime_id", rt.ID, "ts", ts, "err", err, )