Skip to content

Commit 4a9e898

Browse files
committed
sphinx: add strictAttribution flag for onion error decryption
We now add a flag to the decryption function which controls the way we're carrying out the decryption with regards to the attribution data. This is primarily done for graceful backwards compatibility, in order to prevent nodes upgrading to attributable failures from blaming their first hop peers, which would very fast blacklist their channels and cause failed payments. As an initial step, we'd want the senders to perform the non-strict decryption, eventually switching to the strict decryption once adoption is sufficient.
1 parent 38d2274 commit 4a9e898

File tree

3 files changed

+114
-57
lines changed

3 files changed

+114
-57
lines changed

attr_error_test.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ func TestAttributableOnionFailure(t *testing.T) {
6969

7070
// Emulate that sender node receive the failure message and trying to
7171
// unwrap it, by applying obfuscation and checking the hmac.
72-
decryptedError, err := deobfuscator.DecryptError(legacyData, attrData)
72+
decryptedError, err := deobfuscator.DecryptError(
73+
legacyData, attrData, true,
74+
)
7375
require.NoError(t, err)
7476

7577
// We should understand the node from which error have been received.
@@ -144,7 +146,9 @@ func TestOnionFailureCorruption(t *testing.T) {
144146

145147
// Emulate that sender node receive the failure message and trying to
146148
// unwrap it, by applying obfuscation and checking the hmac.
147-
decryptedError, err := deobfuscator.DecryptError(legacyData, attrData)
149+
decryptedError, err := deobfuscator.DecryptError(
150+
legacyData, attrData, true,
151+
)
148152
require.NoError(t, err)
149153

150154
// Assert that the second hop is correctly identified as the error
@@ -247,7 +251,9 @@ func TestAttributableFailureSpecVector(t *testing.T) {
247251

248252
// Emulate that sender node receives the failure message and trying to
249253
// unwrap it, by applying obfuscation and checking the hmac.
250-
decryptedError, err := deobfuscator.DecryptError(legacyData, attrData)
254+
decryptedError, err := deobfuscator.DecryptError(
255+
legacyData, attrData, true,
256+
)
251257
require.NoError(t, err)
252258

253259
// Check that message have been properly de-obfuscated.
@@ -291,7 +297,9 @@ func TestAttributableOnionFailureZeroesMessage(t *testing.T) {
291297
// unwrap it, by applying obfuscation and checking the hmac.
292298
obfuscatedData := make([]byte, 20000)
293299

294-
decryptedError, err := deobfuscator.DecryptError(obfuscatedData, nil)
300+
decryptedError, err := deobfuscator.DecryptError(
301+
obfuscatedData, nil, true,
302+
)
295303
require.NoError(t, err)
296304

297305
require.Equal(t, 1, decryptedError.SenderIdx)
@@ -317,7 +325,9 @@ func TestAttributableOnionFailureShortMessage(t *testing.T) {
317325
obfuscatedData := make([]byte, deobfuscator.hmacsAndPayloadsLen()-1)
318326
failureMsg := bytes.Repeat([]byte{1}, minOnionErrorLength)
319327

320-
decryptedError, err := deobfuscator.DecryptError(failureMsg, obfuscatedData)
328+
decryptedError, err := deobfuscator.DecryptError(
329+
failureMsg, obfuscatedData, true,
330+
)
321331
require.NoError(t, err)
322332

323333
require.Equal(t, 1, decryptedError.SenderIdx)

crypto.go

Lines changed: 90 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -340,17 +340,40 @@ const minOnionErrorLength = minPaddedOnionErrorLength + sha256.Size
340340
// returned that contains the decrypted error message and information of the
341341
// error sender. We also report the hold times in ms for each hop on the error
342342
// path.
343+
//
344+
// The strictAttribution flag controls the behavior of the decryption logic
345+
// surrounding the presence of attribution data:
346+
//
347+
// - If set, then the first node with bad attribution data will be blamed
348+
// immediately.
349+
//
350+
// - If unset, decryption continues optimistically until a successful error
351+
// message decryption occurs, regardless of attribution data validity. Hold
352+
// times are still extracted for nodes that provided valid attribution data.
343353
func (o *OnionErrorDecrypter) DecryptError(encryptedData []byte,
344-
attrData []byte) (*DecryptedError, error) {
354+
attrData []byte, strictAttribution bool) (*DecryptedError, error) {
345355

346-
// Ensure the error message and attribution data length is as expected.
347-
if len(encryptedData) < minOnionErrorLength ||
348-
len(attrData) < o.hmacsAndPayloadsLen() {
356+
// Ensure the error message field is present and has the correct length.
357+
if len(encryptedData) < minOnionErrorLength {
358+
return nil, fmt.Errorf("invalid error length: "+
359+
"expected at least %v got %v", minOnionErrorLength,
360+
len(encryptedData))
361+
}
349362

350-
return &DecryptedError{
351-
Sender: o.circuit.PaymentPath[0],
352-
SenderIdx: 1,
353-
}, nil
363+
validAttr := true
364+
365+
// If we're decrypting with strict attribution, we need to have the
366+
// correct attribution data present too. If strictAttribution is set
367+
// then we immediately blame the first hop.
368+
if len(attrData) < o.hmacsAndPayloadsLen() {
369+
if strictAttribution {
370+
return &DecryptedError{
371+
Sender: o.circuit.PaymentPath[0],
372+
SenderIdx: 1,
373+
}, nil
374+
} else {
375+
validAttr = false
376+
}
354377
}
355378

356379
sharedSecrets, err := generateSharedSecrets(
@@ -393,49 +416,69 @@ func (o *OnionErrorDecrypter) DecryptError(encryptedData []byte,
393416

394417
// With the shared secret, we'll now strip off a layer of
395418
// encryption from the encrypted failure and attribution
396-
// data.
419+
// data. This needs to be done before parsing the attribution
420+
// data, as the attribution data HMACs commit to it.
397421
failData = onionEncrypt(AMMAG, &sharedSecret, failData)
398-
attrData = onionEncrypt(AMMAG_EXT, &sharedSecret, attrData)
399-
400-
payloads := o.payloads(attrData)
401-
hmacs := o.hmacs(attrData)
402422

403-
// Let's calculate the HMAC we expect for the corresponding
404-
// payloads.
405-
position := o.hopCount - i - 1
406-
expectedAttrHmac := o.calculateHmac(
407-
sharedSecret, position, failData, payloads, hmacs,
408-
)
409-
410-
// Let's retrieve the actual HMAC from the correct position in
411-
// the HMACs array.
412-
actualAttrHmac := hmacs[i*o.hmacSize : (i+1)*o.hmacSize]
413-
414-
// If the hmac does not match up, exit with a nil message. This
415-
// is not done for the dummy iterations.
416-
if !bytes.Equal(actualAttrHmac, expectedAttrHmac) &&
417-
sender == 0 && i < len(o.circuit.PaymentPath) {
418-
419-
sender = i + 1
420-
msg = nil
423+
// If the attribution data are valid then do another round of
424+
// attribution data decryption.
425+
if validAttr {
426+
attrData = onionEncrypt(
427+
AMMAG_EXT, &sharedSecret, attrData,
428+
)
429+
430+
payloads := o.payloads(attrData)
431+
hmacs := o.hmacs(attrData)
432+
433+
// Let's calculate the HMAC we expect for the
434+
// corresponding payloads.
435+
position := o.hopCount - i - 1
436+
expectedAttrHmac := o.calculateHmac(
437+
sharedSecret, position, failData, payloads,
438+
hmacs,
439+
)
440+
441+
// Let's retrieve the actual HMAC from the correct
442+
// position in the HMACs array.
443+
actualAttrHmac := hmacs[i*o.hmacSize : (i+1)*o.hmacSize]
444+
445+
// If the hmac does not match up, exit with a nil
446+
// message. This is not done for the dummy iterations.
447+
if !bytes.Equal(actualAttrHmac, expectedAttrHmac) &&
448+
sender == 0 && i < len(o.circuit.PaymentPath) {
449+
450+
switch strictAttribution {
451+
case true:
452+
sender = i + 1
453+
msg = nil
454+
455+
case false:
456+
// Flag the attribution data as invalid
457+
// from this point onwards. This will
458+
// prevent the loop from trying to
459+
// extract anything from the attribution
460+
// data.
461+
validAttr = false
462+
}
463+
}
464+
465+
// Extract the payload and exit with a nil message if it
466+
// is invalid.
467+
holdTime := o.extractPayload(payloads)
468+
if sender == 0 {
469+
// Store hold time reported by this node.
470+
hopPayloads = append(hopPayloads, holdTime)
471+
472+
// Update the message.
473+
msg = failData[sha256.Size:]
474+
}
475+
476+
// Shift payloads and hmacs to the left to prepare for
477+
// the next iteration.
478+
o.shiftPayloadsLeft(payloads)
479+
o.shiftHmacsLeft(hmacs)
421480
}
422481

423-
// Extract the payload and exit with a nil message if it is
424-
// invalid.
425-
holdTime := o.extractPayload(payloads)
426-
if sender == 0 {
427-
// Store hold time reported by this node.
428-
hopPayloads = append(hopPayloads, holdTime)
429-
430-
// Update the message.
431-
msg = failData[sha256.Size:]
432-
}
433-
434-
// Shift payloads and hmacs to the left to prepare for the next
435-
// iteration.
436-
o.shiftPayloadsLeft(payloads)
437-
o.shiftHmacsLeft(hmacs)
438-
439482
// Next, we'll need to separate the failure data, from the MAC
440483
// itself so we can reconstruct and verify it.
441484
expectedMac := failData[:sha256.Size]

obfuscation_test.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestOnionFailure(t *testing.T) {
4242

4343
// Emulate the situation when last hop creates the onion failure
4444
// message and send it back.
45-
legacyData, attrData, err := obfuscator.EncryptError(
45+
legacyData, _, err := obfuscator.EncryptError(
4646
true, failureData, nil, 0,
4747
)
4848
require.NoError(t, err)
@@ -55,8 +55,8 @@ func TestOnionFailure(t *testing.T) {
5555
sharedSecrets[i], attributableErrorTestStructure,
5656
)
5757

58-
legacyData, attrData, err = obfuscator.EncryptError(
59-
false, legacyData, attrData, 1,
58+
legacyData, _, err = obfuscator.EncryptError(
59+
false, legacyData, nil, 1,
6060
)
6161
require.NoError(t, err)
6262
}
@@ -69,7 +69,9 @@ func TestOnionFailure(t *testing.T) {
6969

7070
// Emulate that sender node receive the failure message and trying to
7171
// unwrap it, by applying obfuscation and checking the hmac.
72-
decryptedError, err := deobfuscator.DecryptError(legacyData, attrData)
72+
decryptedError, err := deobfuscator.DecryptError(
73+
legacyData, nil, false,
74+
)
7375
if err != nil {
7476
t.Fatalf("unable to de-obfuscate the onion failure: %v", err)
7577
}
@@ -278,7 +280,9 @@ func TestOnionFailureSpecVector(t *testing.T) {
278280

279281
// Emulate that sender node receives the failure message and trying to
280282
// unwrap it, by applying obfuscation and checking the hmac.
281-
decryptedError, err := deobfuscator.DecryptError(legacyData, attrData)
283+
decryptedError, err := deobfuscator.DecryptError(
284+
legacyData, attrData, false,
285+
)
282286
if err != nil {
283287
t.Fatalf("unable to de-obfuscate the onion failure: %v", err)
284288
}

0 commit comments

Comments
 (0)