Skip to content

Commit

Permalink
Report decryption error reasons from QuicReadCodec.
Browse files Browse the repository at this point in the history
Summary:
To facilitate better stats when we get to the point of invoking the
Aead, but decryption fails (eg if we derived the wrong keys).

Reviewed By: hanidamlaj

Differential Revision: D66040217

fbshipit-source-id: 5662c0a0f07a9c421b07b3b9fea53678f9db44d2
  • Loading branch information
Kyle Nekritz authored and facebook-github-bot committed Nov 20, 2024
1 parent fb2c430 commit df29c69
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 7 deletions.
3 changes: 3 additions & 0 deletions quic/QuicConstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ BETTER_ENUM(
uint8_t,
NONE,
CONNECTION_NOT_FOUND,
DECRYPTION_ERROR_INITIAL,
DECRYPTION_ERROR_HANDSHAKE,
DECRYPTION_ERROR_0RTT,
DECRYPTION_ERROR,
INVALID_PACKET_SIZE,
INVALID_PACKET_SIZE_INITIAL,
Expand Down
22 changes: 18 additions & 4 deletions quic/codec/QuicReadCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,19 @@ folly::Expected<ParsedLongHeader, TransportErrorCode> tryParseLongHeader(
return std::move(parsedLongHeader.value());
}

static PacketDropReason getDecryptErrorReason(ProtectionType protectionType) {
switch (protectionType) {
case ProtectionType::Initial:
return PacketDropReason::DECRYPTION_ERROR_INITIAL;
case ProtectionType::Handshake:
return PacketDropReason::DECRYPTION_ERROR_HANDSHAKE;
case ProtectionType::ZeroRtt:
return PacketDropReason::DECRYPTION_ERROR_0RTT;
default:
return PacketDropReason::DECRYPTION_ERROR;
}
}

CodecResult QuicReadCodec::parseLongHeaderPacket(
BufQueue& queue,
const AckStates& ackStates) {
Expand Down Expand Up @@ -236,7 +249,7 @@ CodecResult QuicReadCodec::parseLongHeaderPacket(
<< " packetNumLen=" << parsePacketNumberLength(initialByte)
<< " protectionType=" << toString(protectionType) << " "
<< connIdToHex();
return CodecResult(Nothing());
return CodecResult(Nothing(getDecryptErrorReason(protectionType)));
}
decrypted = std::move(*decryptAttempt);

Expand Down Expand Up @@ -347,7 +360,7 @@ CodecResult QuicReadCodec::tryParseShortHeaderPacket(
VLOG(10) << "Unable to decrypt packet=" << packetNum.first
<< " protectionType=" << (int)protectionType << " "
<< connIdToHex();
return CodecResult(Nothing());
return CodecResult(Nothing(PacketDropReason::DECRYPTION_ERROR));
}
decrypted = std::move(*decryptAttempt);
if (!decrypted) {
Expand Down Expand Up @@ -627,8 +640,9 @@ CodecResult::CodecResult(RetryPacket&& retryPacketIn)
new (&retry) RetryPacket(std::move(retryPacketIn));
}

CodecResult::CodecResult(Nothing&&) : type_(CodecResult::Type::NOTHING) {
new (&none) Nothing();
CodecResult::CodecResult(Nothing&& nothing)
: type_(CodecResult::Type::NOTHING) {
new (&none) Nothing(std::move(nothing));
}

void CodecResult::destroyCodecResult() {
Expand Down
7 changes: 6 additions & 1 deletion quic/codec/QuicReadCodec.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,12 @@ struct CipherUnavailable {
/**
* A type which represents no data.
*/
struct Nothing {};
struct Nothing {
PacketDropReason reason;

Nothing() : reason(PacketDropReason::UNEXPECTED_NOTHING) {}
explicit Nothing(PacketDropReason reasonIn) : reason(reasonIn) {}
};

struct CodecResult {
enum class Type {
Expand Down
13 changes: 11 additions & 2 deletions quic/codec/test/QuicReadCodecTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ std::unique_ptr<QuicReadCodec> makeEncryptedCodec(
TEST_F(QuicReadCodecTest, EmptyBuffer) {
auto emptyQueue = bufToQueue(folly::IOBuf::create(0));
AckStates ackStates;
EXPECT_FALSE(
parseSuccess(makeUnencryptedCodec()->parsePacket(emptyQueue, ackStates)));
auto packet = makeUnencryptedCodec()->parsePacket(emptyQueue, ackStates);
EXPECT_EQ(
packet.nothing()->reason,
PacketDropReason(PacketDropReason::UNEXPECTED_NOTHING));
EXPECT_FALSE(parseSuccess(std::move(packet)));
}

TEST_F(QuicReadCodecTest, TooSmallBuffer) {
Expand Down Expand Up @@ -312,6 +315,9 @@ TEST_F(QuicReadCodecTest, PacketDecryptFail) {
auto packetQueue = bufToQueue(packetToBuf(streamPacket));
auto packet = makeEncryptedCodec(connId, std::move(aead))
->parsePacket(packetQueue, ackStates);
EXPECT_EQ(
packet.nothing()->reason,
PacketDropReason(PacketDropReason::DECRYPTION_ERROR));
EXPECT_FALSE(parseSuccess(std::move(packet)));
}

Expand Down Expand Up @@ -607,6 +613,9 @@ TEST_F(QuicReadCodecTest, FailToDecryptLongHeaderNoReset) {
AckStates ackStates;
auto packetQueue = bufToQueue(packetToBuf(streamPacket));
auto packet = codec->parsePacket(packetQueue, ackStates);
EXPECT_EQ(
packet.nothing()->reason,
PacketDropReason(PacketDropReason::DECRYPTION_ERROR_0RTT));
EXPECT_FALSE(isReset(std::move(packet)));
}

Expand Down

0 comments on commit df29c69

Please sign in to comment.