Skip to content

Commit

Permalink
go/worker/keymanager: Fix race condition when accessing enclave status
Browse files Browse the repository at this point in the history
  • Loading branch information
peternose committed Jan 16, 2024
1 parent c7ae397 commit 135bd82
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 14 deletions.
1 change: 1 addition & 0 deletions .changelog/5529.bugfix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
go/worker/keymanager: Fix race conditions when accessing status fields
7 changes: 1 addition & 6 deletions go/worker/keymanager/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ func (w *Worker) GetStatus() (*api.Status, error) {
al = append(al, ral)
}

var pc []byte
if w.enclaveStatus != nil {
pc = w.enclaveStatus.InitResponse.PolicyChecksum
}

gs := w.globalStatus
ws := api.WorkerStatus{
Status: ss,
Expand All @@ -70,7 +65,7 @@ func (w *Worker) GetStatus() (*api.Status, error) {
AccessList: al,
PrivatePeers: ps,
Policy: w.policy,
PolicyChecksum: pc,
PolicyChecksum: w.policyChecksum,
MasterSecrets: w.masterSecretStats,
EphemeralSecrets: w.ephemeralSecretStats,
}
Expand Down
22 changes: 14 additions & 8 deletions go/worker/keymanager/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ type Worker struct { // nolint: maligned
roleProvider registration.RoleProvider
backend api.Backend

globalStatus *api.Status
enclaveStatus *api.SignedInitResponse
policy *api.SignedPolicySGX
activeVersion *version.Version
globalStatus *api.Status
policy *api.SignedPolicySGX
policyChecksum []byte
activeVersion *version.Version

masterSecretStats workerKeymanager.MasterSecretStats
ephemeralSecretStats workerKeymanager.EphemeralSecretStats
Expand Down Expand Up @@ -358,13 +358,13 @@ func (w *Worker) initEnclave(kmStatus *api.Status, rtStatus *runtimeStatus) (*ap

// Update metrics.
enclaveMasterSecretGenerationNumber.WithLabelValues(w.runtimeLabel).Set(float64(kmStatus.Generation))
if w.enclaveStatus == nil || !bytes.Equal(w.enclaveStatus.InitResponse.PolicyChecksum, signedInitResp.InitResponse.PolicyChecksum) {
if !bytes.Equal(w.policyChecksum, signedInitResp.InitResponse.PolicyChecksum) {
policyUpdateCount.WithLabelValues(w.runtimeLabel).Inc()
}

// Cache the key manager enclave status and the currently active policy.
w.enclaveStatus = &signedInitResp
// Cache the currently active policy and its checksum.
w.policy = kmStatus.Policy
w.policyChecksum = signedInitResp.InitResponse.PolicyChecksum

return &signedInitResp, nil
}
Expand Down Expand Up @@ -1043,10 +1043,16 @@ func (w *Worker) handleRuntimeHostEvent(ev *host.Event) {
return
}

// Check whether the enclave has been initialized at least once.
// If true, preregistration is not required.
w.RLock()
initialized := w.policyChecksum != nil
w.RUnlock()

// Send a node preregistration, so that other nodes know to update their access
// control. Without it, the enclave won't be able to replicate the master secrets
// needed for initialization.
if w.enclaveStatus == nil {
if !initialized {
rtStatus := w.rtStatus
w.roleProvider.SetAvailableWithCallback(func(n *node.Node) error {
rt := n.AddOrUpdateRuntime(w.runtime.ID(), rtStatus.version)
Expand Down

0 comments on commit 135bd82

Please sign in to comment.