only return an invalid first key phase error for decryptable packets

This commit is contained in:
Marten Seemann
2020-09-07 21:12:54 +07:00
parent bed802aee5
commit 34c325919c
2 changed files with 25 additions and 12 deletions

View File

@@ -153,26 +153,31 @@ func (a *updatableAEAD) Open(dst, src []byte, rcvTime time.Time, pn protocol.Pac
} }
binary.BigEndian.PutUint64(a.nonceBuf[len(a.nonceBuf)-8:], uint64(pn)) binary.BigEndian.PutUint64(a.nonceBuf[len(a.nonceBuf)-8:], uint64(pn))
if kp != a.keyPhase.Bit() { if kp != a.keyPhase.Bit() {
var receivedWrongInitialKeyPhase bool
if a.firstRcvdWithCurrentKey == protocol.InvalidPacketNumber || pn < a.firstRcvdWithCurrentKey { if a.firstRcvdWithCurrentKey == protocol.InvalidPacketNumber || pn < a.firstRcvdWithCurrentKey {
if a.keyPhase == 0 { if a.keyPhase == 0 {
// This can only occur when the first packet received has key phase 1. // This can only occur when the first packet received has key phase 1.
// This is an error, since the key phase starts at 0, // This is an error, since the key phase starts at 0,
// and peers are only allowed to update keys after the handshake is confirmed. // and peers are only allowed to update keys after the handshake is confirmed.
return nil, qerr.NewError(qerr.ProtocolViolation, "wrong initial keyphase") // Proceed from here, and only return an error if decryption of the packet succeeds.
receivedWrongInitialKeyPhase = true
} else {
if a.prevRcvAEAD == nil {
return nil, ErrKeysDropped
}
// we updated the key, but the peer hasn't updated yet
dec, err := a.prevRcvAEAD.Open(dst, a.nonceBuf, src, ad)
if err != nil {
err = ErrDecryptionFailed
}
return dec, err
} }
if a.prevRcvAEAD == nil {
return nil, ErrKeysDropped
}
// we updated the key, but the peer hasn't updated yet
dec, err := a.prevRcvAEAD.Open(dst, a.nonceBuf, src, ad)
if err != nil {
err = ErrDecryptionFailed
}
return dec, err
} }
// try opening the packet with the next key phase // try opening the packet with the next key phase
dec, err := a.nextRcvAEAD.Open(dst, a.nonceBuf, src, ad) dec, err := a.nextRcvAEAD.Open(dst, a.nonceBuf, src, ad)
if err != nil { if err == nil && receivedWrongInitialKeyPhase {
return nil, qerr.NewError(qerr.ProtocolViolation, "wrong initial key phase")
} else if err != nil {
return nil, ErrDecryptionFailed return nil, ErrDecryptionFailed
} }
// Opening succeeded. Check if the peer was allowed to update. // Opening succeeded. Check if the peer was allowed to update.

View File

@@ -200,7 +200,15 @@ var _ = Describe("Updatable AEAD", func() {
client.rollKeys(time.Now()) client.rollKeys(time.Now())
encrypted := client.Seal(nil, msg, 0x1337, ad) encrypted := client.Seal(nil, msg, 0x1337, ad)
_, err := server.Open(nil, encrypted, time.Now(), 0x1337, protocol.KeyPhaseOne, ad) _, err := server.Open(nil, encrypted, time.Now(), 0x1337, protocol.KeyPhaseOne, ad)
Expect(err).To(MatchError("PROTOCOL_VIOLATION: wrong initial keyphase")) Expect(err).To(MatchError("PROTOCOL_VIOLATION: wrong initial key phase"))
})
It("only errors when the peer starts with key phase 1 if decrypting the packet succeeds", func() {
client.rollKeys(time.Now())
encrypted := client.Seal(nil, msg, 0x1337, ad)
encrypted = encrypted[:len(encrypted)-1]
_, err := server.Open(nil, encrypted, time.Now(), 0x1337, protocol.KeyPhaseOne, ad)
Expect(err).To(MatchError(ErrDecryptionFailed))
}) })
It("errors when the peer updates keys too frequently", func() { It("errors when the peer updates keys too frequently", func() {