Skip to content

Commit db96bf2

Browse files
dhaidashenkodhaidashenko
committed
Refactor ocr attestation
1 parent 28e17c5 commit db96bf2

17 files changed

Lines changed: 177 additions & 192 deletions

File tree

‎core/capabilities/remote/executable/request/client_request.go‎

Lines changed: 27 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -316,35 +316,11 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err
316316
return fmt.Errorf("failed to unmarshal capability response: %w", err)
317317
}
318318

319-
if resp.Metadata.OCRAttestation != nil {
320-
var rpt commoncap.MeteringNodeDetail
321-
rpt, err = extractMeteringFromMetadata(sender, resp.Metadata)
322-
if err != nil {
323-
return fmt.Errorf("failed to extract metering detail from metadata: %w", err)
324-
}
325-
// Since signatures are provided switch to OCR based validation. It's enough to get 1 response with F+1 signatures
326-
// to be confident that the response is honest.
327-
err = c.verifyAttestation(resp, rpt)
328-
if err != nil {
329-
c.lggr.Errorw("failed to verify capability response OCR attestation", "peer", sender, "err", err, "requestID", c.id, "msgPayload", hex.EncodeToString(msg.Payload))
330-
return fmt.Errorf("failed to verify capability response OCR attestation: %w", err)
331-
}
332-
333-
var payload []byte
334-
payload, err = c.encodePayloadWithMetadata(msg, commoncap.ResponseMetadata{Metering: []commoncap.MeteringNodeDetail{rpt}})
335-
if err != nil {
336-
return fmt.Errorf("failed to encode payload with metadata: %w", err)
337-
}
338-
339-
c.sendResponse(clientResponse{Result: payload})
340-
return nil
341-
}
342-
343319
// metering reports per node are aggregated into a single array of values. for any single node message, the
344320
// metering values are extracted from the CapabilityResponse, added to an array, and the CapabilityResponse
345321
// is marshalled without the metering value to get the hash. each node could have a different metering value
346322
// which would result in different hashes. removing the metering detail allows for direct comparison of results.
347-
responseID, metadata, err := c.getMessageHashAndMetadata(msg)
323+
responseID, err := c.getMessageHash(resp)
348324
if err != nil {
349325
return fmt.Errorf("failed to get message hash: %w", err)
350326
}
@@ -356,7 +332,7 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err
356332
nodeReports = make([]commoncap.MeteringNodeDetail, 0)
357333
}
358334

359-
rpt, err := extractMeteringFromMetadata(sender, metadata)
335+
rpt, err := commoncap.ExtractMeteringFromMetadata(sender, resp.Metadata)
360336
if err != nil {
361337
lggr.Warnw("invalid metering detail", "err", err)
362338
} else {
@@ -370,7 +346,7 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err
370346
lggr.Warnw("received multiple unique responses for the same request", "count for responseID", len(c.responseIDCount))
371347
}
372348

373-
if c.responseIDCount[responseID] == c.requiredResponseConfirmations {
349+
if c.responseIDCount[responseID] == c.requiredResponseConfirmations || c.hasValidAttestation(resp) {
374350
payload, err := c.encodePayloadWithMetadata(msg, commoncap.ResponseMetadata{Metering: nodeReports})
375351
if err != nil {
376352
return fmt.Errorf("failed to encode payload with metadata: %w", err)
@@ -406,20 +382,24 @@ func (c *ClientRequest) OnMessage(_ context.Context, msg *types.MessageBody) err
406382
return nil
407383
}
408384

409-
func extractMeteringFromMetadata(sender p2ptypes.PeerID, metadata commoncap.ResponseMetadata) (commoncap.MeteringNodeDetail, error) {
410-
if len(metadata.Metering) != 1 {
411-
return commoncap.MeteringNodeDetail{}, fmt.Errorf("unexpected number of metering records received from peer %s: got %d, want 1", sender, len(metadata.Metering))
385+
func (c *ClientRequest) hasValidAttestation(resp commoncap.CapabilityResponse) bool {
386+
if resp.OCRAttestation == nil {
387+
return false
388+
}
389+
390+
err := c.verifyAttestation(resp)
391+
if err != nil {
392+
c.lggr.Errorw("Attestation is present, but not valid. This is most likely a bug and requires investigation - falling back to identical responses verification", "error", err)
393+
return false
412394
}
413395

414-
rpt := metadata.Metering[0]
415-
rpt.Peer2PeerID = sender.String()
416-
return rpt, nil
396+
return true
417397
}
418398

419-
func (c *ClientRequest) verifyAttestation(resp commoncap.CapabilityResponse, metering commoncap.MeteringNodeDetail) error {
420-
attestation := resp.Metadata.OCRAttestation
399+
func (c *ClientRequest) verifyAttestation(resp commoncap.CapabilityResponse) error {
400+
attestation := resp.OCRAttestation
421401
if attestation == nil {
422-
return errors.New("OCR attestation missing from response metadata")
402+
return errors.New("attestation is missing")
423403
}
424404

425405
if len(attestation.Sigs) < c.requiredResponseConfirmations {
@@ -430,7 +410,10 @@ func (c *ClientRequest) verifyAttestation(resp commoncap.CapabilityResponse, met
430410
return fmt.Errorf("number of configured OCR signers is less than required confirmations: got %d, need at least %d", len(c.signers), c.requiredResponseConfirmations)
431411
}
432412

433-
reportData := commoncap.ResponseToReportData(c.workflowExecutionID, c.referenceID, resp.Payload.Value, metering.SpendUnit, metering.SpendValue)
413+
reportData, err := commoncap.ResponseToReportData(c.workflowExecutionID, c.referenceID, resp.Payload.Value, resp.Metadata)
414+
if err != nil {
415+
return fmt.Errorf("failed to convert response to report data: %w", err)
416+
}
434417
sigData := ocr2key.ReportToSigData3(attestation.ConfigDigest, attestation.SequenceNumber, reportData[:])
435418
signed := make([]bool, len(c.signers))
436419
for _, sig := range attestation.Sigs {
@@ -463,23 +446,16 @@ func (c *ClientRequest) sendResponse(response clientResponse) {
463446
c.lggr.Debugw("received OK response")
464447
}
465448

466-
func (c *ClientRequest) getMessageHashAndMetadata(msg *types.MessageBody) ([32]byte, commoncap.ResponseMetadata, error) {
467-
var metadata commoncap.ResponseMetadata
468-
469-
resp, err := pb.UnmarshalCapabilityResponse(msg.Payload)
470-
if err != nil {
471-
return [32]byte{}, metadata, err
472-
}
473-
474-
metadata = resp.Metadata
475-
resp.Metadata = commoncap.ResponseMetadata{}
476-
477-
payload, err := pb.MarshalCapabilityResponse(resp)
449+
func (c *ClientRequest) getMessageHash(msg commoncap.CapabilityResponse) ([32]byte, error) {
450+
// clear metadata to ensure it doesn't affect the hash, as different nodes might have different metadata (e.g. different metering values)
451+
// since msg is passed as value, this won't affect the original message
452+
msg.Metadata = commoncap.ResponseMetadata{}
453+
payload, err := pb.MarshalCapabilityResponse(msg)
478454
if err != nil {
479-
return [32]byte{}, metadata, err
455+
return [32]byte{}, err
480456
}
481457

482-
return sha256.Sum256(payload), metadata, nil
458+
return sha256.Sum256(payload), nil
483459
}
484460

485461
func (c *ClientRequest) encodePayloadWithMetadata(msg *types.MessageBody, metadata commoncap.ResponseMetadata) ([]byte, error) {

‎core/capabilities/remote/executable/request/client_request_internal_test.go‎

Lines changed: 64 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,17 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
3535
kb2, err := ocr2key.New(corekeys.EVM)
3636
require.NoError(t, err)
3737

38-
reportData := commoncap.ResponseToReportData(workflowExecutionID, referenceID, valueBytes, spendUnit, spendValue)
38+
validResp := commoncap.CapabilityResponse{
39+
Metadata: commoncap.ResponseMetadata{
40+
Metering: []commoncap.MeteringNodeDetail{
41+
{SpendUnit: spendUnit, SpendValue: spendValue},
42+
},
43+
},
44+
Payload: &anypb.Any{TypeUrl: "type.googleapis.com/values.v1.Map", Value: valueBytes},
45+
}
46+
47+
reportData, err := commoncap.ResponseToReportData(workflowExecutionID, referenceID, valueBytes, validResp.Metadata)
48+
require.NoError(t, err)
3949

4050
sig1, err := kb1.Sign3(configDigest, seqNr, reportData[:])
4151
require.NoError(t, err)
@@ -44,6 +54,15 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
4454

4555
signers := [][]byte{kb1.PublicKey(), kb2.PublicKey()}
4656

57+
validResp.OCRAttestation = &commoncap.OCRAttestation{
58+
ConfigDigest: configDigest,
59+
SequenceNumber: seqNr,
60+
Sigs: []commoncap.AttributedSignature{
61+
{Signer: 0, Signature: sig1},
62+
{Signer: 1, Signature: sig2},
63+
},
64+
}
65+
4766
c := &ClientRequest{
4867
lggr: logger.Test(t),
4968
signers: signers,
@@ -52,31 +71,14 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
5271
requiredResponseConfirmations: 2,
5372
}
5473

55-
validResp := commoncap.CapabilityResponse{
56-
Metadata: commoncap.ResponseMetadata{
57-
Metering: []commoncap.MeteringNodeDetail{
58-
{SpendUnit: spendUnit, SpendValue: spendValue},
59-
},
60-
OCRAttestation: &commoncap.ResponseOCRAttestation{
61-
ConfigDigest: configDigest,
62-
SequenceNumber: seqNr,
63-
Sigs: []commoncap.AttributedSignature{
64-
{Signer: 0, Signature: sig1},
65-
{Signer: 1, Signature: sig2},
66-
},
67-
},
68-
},
69-
Payload: &anypb.Any{TypeUrl: "type.googleapis.com/values.v1.Map", Value: valueBytes},
70-
}
71-
7274
t.Run("not enough signers returns error", func(t *testing.T) {
7375
cBad := &ClientRequest{
7476
workflowExecutionID: workflowExecutionID,
7577
referenceID: referenceID,
7678
lggr: logger.Test(t),
7779
requiredResponseConfirmations: 2,
7880
}
79-
err := cBad.verifyAttestation(validResp, validResp.Metadata.Metering[0])
81+
err := cBad.verifyAttestation(validResp)
8082
require.Error(t, err)
8183
require.Contains(t, err.Error(), "number of configured OCR signers is less than required confirmations: got 0, need at least 2")
8284
})
@@ -85,15 +87,15 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
8587
respFewSigs := commoncap.CapabilityResponse{
8688
Metadata: commoncap.ResponseMetadata{
8789
Metering: []commoncap.MeteringNodeDetail{{SpendUnit: spendUnit, SpendValue: spendValue}},
88-
OCRAttestation: &commoncap.ResponseOCRAttestation{
89-
ConfigDigest: configDigest,
90-
SequenceNumber: seqNr,
91-
Sigs: []commoncap.AttributedSignature{{Signer: 0, Signature: sig1}},
92-
},
9390
},
9491
Payload: &anypb.Any{TypeUrl: "type.googleapis.com/values.v1.Map", Value: valueBytes},
92+
OCRAttestation: &commoncap.OCRAttestation{
93+
ConfigDigest: configDigest,
94+
SequenceNumber: seqNr,
95+
Sigs: []commoncap.AttributedSignature{{Signer: 0, Signature: sig1}},
96+
},
9597
}
96-
err := c.verifyAttestation(respFewSigs, validResp.Metadata.Metering[0])
98+
err := c.verifyAttestation(respFewSigs)
9799
require.Error(t, err)
98100
require.Contains(t, err.Error(), "not enough signatures")
99101
})
@@ -102,18 +104,18 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
102104
respBadSigner := commoncap.CapabilityResponse{
103105
Metadata: commoncap.ResponseMetadata{
104106
Metering: []commoncap.MeteringNodeDetail{{SpendUnit: spendUnit, SpendValue: spendValue}},
105-
OCRAttestation: &commoncap.ResponseOCRAttestation{
106-
ConfigDigest: configDigest,
107-
SequenceNumber: seqNr,
108-
Sigs: []commoncap.AttributedSignature{
109-
{Signer: 0, Signature: sig1},
110-
{Signer: 99, Signature: sig2},
111-
},
112-
},
113107
},
114108
Payload: &anypb.Any{TypeUrl: "type.googleapis.com/values.v1.Map", Value: valueBytes},
109+
OCRAttestation: &commoncap.OCRAttestation{
110+
ConfigDigest: configDigest,
111+
SequenceNumber: seqNr,
112+
Sigs: []commoncap.AttributedSignature{
113+
{Signer: 0, Signature: sig1},
114+
{Signer: 99, Signature: sig2},
115+
},
116+
},
115117
}
116-
err := c.verifyAttestation(respBadSigner, validResp.Metadata.Metering[0])
118+
err := c.verifyAttestation(respBadSigner)
117119
require.Error(t, err)
118120
require.Contains(t, err.Error(), "invalid signer index")
119121
})
@@ -122,18 +124,18 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
122124
respDupSig := commoncap.CapabilityResponse{
123125
Metadata: commoncap.ResponseMetadata{
124126
Metering: []commoncap.MeteringNodeDetail{{SpendUnit: spendUnit, SpendValue: spendValue}},
125-
OCRAttestation: &commoncap.ResponseOCRAttestation{
126-
ConfigDigest: configDigest,
127-
SequenceNumber: seqNr,
128-
Sigs: []commoncap.AttributedSignature{
129-
{Signer: 0, Signature: sig1},
130-
{Signer: 0, Signature: sig1},
131-
},
132-
},
133127
},
134128
Payload: &anypb.Any{TypeUrl: "type.googleapis.com/values.v1.Map", Value: valueBytes},
129+
OCRAttestation: &commoncap.OCRAttestation{
130+
ConfigDigest: configDigest,
131+
SequenceNumber: seqNr,
132+
Sigs: []commoncap.AttributedSignature{
133+
{Signer: 0, Signature: sig1},
134+
{Signer: 0, Signature: sig1},
135+
},
136+
},
135137
}
136-
err := c.verifyAttestation(respDupSig, validResp.Metadata.Metering[0])
138+
err := c.verifyAttestation(respDupSig)
137139
require.Error(t, err)
138140
require.Contains(t, err.Error(), "duplicate signature")
139141
})
@@ -145,18 +147,18 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
145147
respBadSig := commoncap.CapabilityResponse{
146148
Metadata: commoncap.ResponseMetadata{
147149
Metering: []commoncap.MeteringNodeDetail{{SpendUnit: spendUnit, SpendValue: spendValue}},
148-
OCRAttestation: &commoncap.ResponseOCRAttestation{
149-
ConfigDigest: configDigest,
150-
SequenceNumber: seqNr,
151-
Sigs: []commoncap.AttributedSignature{
152-
{Signer: 0, Signature: sig1},
153-
{Signer: 1, Signature: badSig},
154-
},
155-
},
156150
},
157151
Payload: &anypb.Any{TypeUrl: "type.googleapis.com/values.v1.Map", Value: valueBytes},
152+
OCRAttestation: &commoncap.OCRAttestation{
153+
ConfigDigest: configDigest,
154+
SequenceNumber: seqNr,
155+
Sigs: []commoncap.AttributedSignature{
156+
{Signer: 0, Signature: sig1},
157+
{Signer: 1, Signature: badSig},
158+
},
159+
},
158160
}
159-
err = c.verifyAttestation(respBadSig, validResp.Metadata.Metering[0])
161+
err = c.verifyAttestation(respBadSig)
160162
require.Error(t, err)
161163
require.Contains(t, err.Error(), "invalid signature")
162164
})
@@ -166,24 +168,24 @@ func Test_ClientRequest_VerifyAttestation(t *testing.T) {
166168
respWrongPayload := commoncap.CapabilityResponse{
167169
Metadata: commoncap.ResponseMetadata{
168170
Metering: []commoncap.MeteringNodeDetail{{SpendUnit: spendUnit, SpendValue: spendValue}},
169-
OCRAttestation: &commoncap.ResponseOCRAttestation{
170-
ConfigDigest: configDigest,
171-
SequenceNumber: seqNr,
172-
Sigs: []commoncap.AttributedSignature{
173-
{Signer: 0, Signature: sig1},
174-
{Signer: 1, Signature: sig2},
175-
},
176-
},
177171
},
178172
Payload: &anypb.Any{TypeUrl: "x", Value: wrongBytes},
173+
OCRAttestation: &commoncap.OCRAttestation{
174+
ConfigDigest: configDigest,
175+
SequenceNumber: seqNr,
176+
Sigs: []commoncap.AttributedSignature{
177+
{Signer: 0, Signature: sig1},
178+
{Signer: 1, Signature: sig2},
179+
},
180+
},
179181
}
180-
err := c.verifyAttestation(respWrongPayload, validResp.Metadata.Metering[0])
182+
err := c.verifyAttestation(respWrongPayload)
181183
require.Error(t, err)
182184
require.Contains(t, err.Error(), "invalid signature")
183185
})
184186

185187
t.Run("valid attestation succeeds", func(t *testing.T) {
186-
err := c.verifyAttestation(validResp, validResp.Metadata.Metering[0])
188+
err := c.verifyAttestation(validResp)
187189
require.NoError(t, err)
188190
})
189191
}

0 commit comments

Comments
 (0)