diff --git a/internal/handshake/updatable_aead.go b/internal/handshake/updatable_aead.go index 7e777eb9..dfad557a 100644 --- a/internal/handshake/updatable_aead.go +++ b/internal/handshake/updatable_aead.go @@ -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)) if kp != a.keyPhase.Bit() { + var receivedWrongInitialKeyPhase bool if a.firstRcvdWithCurrentKey == protocol.InvalidPacketNumber || pn < a.firstRcvdWithCurrentKey { if a.keyPhase == 0 { // This can only occur when the first packet received has key phase 1. // This is an error, since the key phase starts at 0, // 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 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 } // Opening succeeded. Check if the peer was allowed to update. diff --git a/internal/handshake/updatable_aead_test.go b/internal/handshake/updatable_aead_test.go index 3ed89caf..985df248 100644 --- a/internal/handshake/updatable_aead_test.go +++ b/internal/handshake/updatable_aead_test.go @@ -207,7 +207,15 @@ var _ = Describe("Updatable AEAD", func() { client.rollKeys(time.Now()) encrypted := client.Seal(nil, msg, 0x1337, 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() {