From 437a0cda4cb9918c1c6e9a98f976dd7c85574e9d Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 11:33:10 +0200 Subject: [PATCH 01/17] analyzer: Replace try/backtrack with parse-at Improves performance processing pure QUIC traffic by ~20% Relates to zeek/spicy#1565. --- analyzer/QUIC.spicy | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index 456e366..8ebb69f 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -131,9 +131,6 @@ public type InitialByte = unit { header_form: 7 &convert=cast(cast($$)); long_packet_type: 4..5 &convert=cast(cast($$)); }; - on %done { - self.backtrack(); - } }; type VariableLengthInteger = unit { @@ -173,10 +170,6 @@ type AllData = unit { : bytes &eod { self.data = $$; } - - on %done { - self.backtrack(); - } }; public type LongHeaderPacket = unit { @@ -362,6 +355,7 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { var long_packet_type: LongPacketType; var decrypted_data: bytes; var full_packet: bytes; + var start: iterator; sink crypto_sink; var crypto_buffer: CryptoBuffer&; @@ -379,19 +373,28 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { context.did_ssl_begin = True; } @endif + + self.start = self.input(); } # Peek into the header to check if it's a SHORT or LONG header # and get the LONG packet type. - : InitialByte &try { + : InitialByte &parse-at=self.input() { self.header_form = $$.initialbyte.header_form; self.long_packet_type = $$.initialbyte.long_packet_type; + self.set_input(self.start); } # Capture all the packet bytes if we're still have a chance of decrypting the INITIAL PACKETS - fpack: AllData &try if ( self.header_form == HeaderForm::LONG && + # + # TODO: Make decrypt_crypto_payload() use the iterator self.start instead of copying + # the data here. + fpack: AllData &parse-at=self.input() if ( self.header_form == HeaderForm::LONG && self.long_packet_type == LongPacketType::INITIAL && - should_buffer(context, from_client) ); + should_buffer(context, from_client) ) { + + self.set_input(self.start); + } # Depending on the header, parse it and update the src/dest ConnectionID's switch ( self.header_form ) { From c5a2a56a2c47a6999b62a6b1ef9226de6f65ad8e Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 16:09:21 +0200 Subject: [PATCH 02/17] decrypt_crypto_payload: Pass stream iterator instead of AllData trick There's still a full packet copy within the decrypt_crypto_payload(), but it's one less now. --- analyzer/QUIC.spicy | 30 +++++------------------------- analyzer/decrypt_crypto.cc | 15 +++++++-------- 2 files changed, 12 insertions(+), 33 deletions(-) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index 8ebb69f..c252dc7 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -4,7 +4,7 @@ import spicy; import zeek; # The interface to the C++ code that handles the decryption of the INITIAL packet payload using well-known keys -public function decrypt_crypto_payload(entire_packet: bytes, connection_id: bytes, encrypted_offset: uint64, payload_offset: uint64, from_client: bool): bytes &cxxname="decrypt_crypto_payload"; +public function decrypt_crypto_payload(packet: iterator, connection_id: bytes, encrypted_offset: uint64, payload_offset: uint64, from_client: bool): bytes &cxxname="decrypt_crypto_payload"; ############## @@ -161,17 +161,6 @@ type VariableLengthInteger = unit { # Generic units ############## -# Used to capture all data form the entire frame. May be inefficient, but works for now. -# This is passed to the decryption function, as this function needs both the header and the payload -# Performs a backtrack() at the end -type AllData = unit { - var data: bytes; - - : bytes &eod { - self.data = $$; - } -}; - public type LongHeaderPacket = unit { var encrypted_offset: uint64; var payload_length: uint64; @@ -385,17 +374,6 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { self.set_input(self.start); } - # Capture all the packet bytes if we're still have a chance of decrypting the INITIAL PACKETS - # - # TODO: Make decrypt_crypto_payload() use the iterator self.start instead of copying - # the data here. - fpack: AllData &parse-at=self.input() if ( self.header_form == HeaderForm::LONG && - self.long_packet_type == LongPacketType::INITIAL && - should_buffer(context, from_client) ) { - - self.set_input(self.start); - } - # Depending on the header, parse it and update the src/dest ConnectionID's switch ( self.header_form ) { HeaderForm::SHORT -> short_header: ShortHeader(context.client_cid_len); @@ -434,7 +412,8 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { # This means that here, we can try to decrypt the initial packet! # All data is accessible via the `long_header` unit - self.decrypted_data = decrypt_crypto_payload(self.fpack.data, + self.decrypted_data = decrypt_crypto_payload( + self.start, self.long_header.dest_conn_id, self.long_header.encrypted_offset, self.long_header.payload_length, @@ -451,7 +430,8 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { # Assuming that the client set up the connection, this can be considered the first # received Initial from the client. So disable change of ConnectionID's afterwards - self.decrypted_data = decrypt_crypto_payload(self.fpack.data, + self.decrypted_data = decrypt_crypto_payload( + self.start, context.initial_destination_conn_id, self.long_header.encrypted_offset, self.long_header.payload_length, diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index e1b6a32..88a1352 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -259,7 +259,7 @@ Function that is called from Spicy. It's a wrapper around `process_data`; it stores all the passed data in a global struct and then calls `process_data`, which will eventually return the decrypted data and pass it back to Spicy. */ -hilti::rt::Bytes decrypt_crypto_payload(const hilti::rt::Bytes& entire_packet, +hilti::rt::Bytes decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterator& packet_stream, const hilti::rt::Bytes& connection_id, const hilti::rt::integer::safe& encrypted_offset, const hilti::rt::integer::safe& payload_length, @@ -269,16 +269,15 @@ hilti::rt::Bytes decrypt_crypto_payload(const hilti::rt::Bytes& entire_packet, if ( payload_length < 20 ) throw hilti::rt::RuntimeError(hilti::rt::fmt("payload too small %ld < 20", payload_length)); - if ( entire_packet.size() < encrypted_offset + payload_length ) - throw hilti::rt::RuntimeError(hilti::rt::fmt( - "packet too small %ld < %ld", entire_packet.size(), encrypted_offset + payload_length)); + if ( (packet_stream + (encrypted_offset + payload_length - 1)).isEnd() ) + throw hilti::rt::RuntimeError( + hilti::rt::fmt("packet too small %ld", encrypted_offset + payload_length)); // Fill in the entire packet bytes std::vector e_pkt; - for ( const auto& singlebyte : entire_packet ) - { - e_pkt.push_back(singlebyte); - } + hilti::rt::stream::SafeConstIterator it = packet_stream; + while ( ! it.isEnd() ) + e_pkt.push_back(*(it++)); std::vector cnnid; for ( const auto& singlebyte : connection_id ) From 20b11d287602927cc5aa9dda7523d8b2b6a996b9 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 16:31:00 +0200 Subject: [PATCH 03/17] hkdf_extract: Pass hilti::rt::Bytes directly There should not be a need for the extra copying. hilti::rt::Bytes are mostly std::string and we can pass by const reference as well. --- analyzer/decrypt_crypto.cc | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 88a1352..1947ad7 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -74,7 +74,7 @@ const size_t MAXIMUM_PACKET_NUMBER_LENGTH = 4; /* HKDF-Extract as described in https://www.rfc-editor.org/rfc/rfc8446.html#section-7.1 */ -std::vector hkdf_extract(std::vector connection_id) +std::vector hkdf_extract(const hilti::rt::Bytes& connection_id) { std::vector out_temp(INITIAL_SECRET_LEN); size_t initial_secret_len = out_temp.size(); @@ -83,9 +83,10 @@ std::vector hkdf_extract(std::vector connection_id) EVP_PKEY_derive_init(pctx); EVP_PKEY_CTX_hkdf_mode(pctx, EVP_PKEY_HKDEF_MODE_EXTRACT_ONLY); EVP_PKEY_CTX_set_hkdf_md(pctx, digest); - EVP_PKEY_CTX_set1_hkdf_key(pctx, connection_id.data(), connection_id.size()); + EVP_PKEY_CTX_set1_hkdf_key(pctx, reinterpret_cast(connection_id.data()), + connection_id.size()); EVP_PKEY_CTX_set1_hkdf_salt(pctx, INITIAL_SALT_V1.data(), INITIAL_SALT_V1.size()); - EVP_PKEY_derive(pctx, out_temp.data(), reinterpret_cast(&initial_secret_len)); + EVP_PKEY_derive(pctx, out_temp.data(), &initial_secret_len); EVP_PKEY_CTX_free(pctx); return out_temp; } @@ -279,13 +280,7 @@ hilti::rt::Bytes decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterat while ( ! it.isEnd() ) e_pkt.push_back(*(it++)); - std::vector cnnid; - for ( const auto& singlebyte : connection_id ) - { - cnnid.push_back(singlebyte); - } - - std::vector initial_secret = hkdf_extract(cnnid); + std::vector initial_secret = hkdf_extract(connection_id); std::vector server_client_secret; if ( from_client ) From 9a3d655e2a495c2fdf2cc2160e6fd627d449642a Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 16:37:27 +0200 Subject: [PATCH 04/17] hkdf_expand: Pass vectors by const-reference No need for the copy. --- analyzer/decrypt_crypto.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 1947ad7..86c6cb6 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -95,8 +95,8 @@ std::vector hkdf_extract(const hilti::rt::Bytes& connection_id) HKDF-Expand-Label as described in https://www.rfc-editor.org/rfc/rfc8446.html#section-7.1 that uses the global constant labels such as 'quic hp'. */ -std::vector hkdf_expand(size_t out_len, std::vector key, - std::vector info) +std::vector hkdf_expand(size_t out_len, const std::vector& key, + const std::vector& info) { std::vector out_temp(out_len); const EVP_MD* digest = EVP_sha256(); From 360f910b28ce97879b2b01379cda611d65e277d7 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 16:42:39 +0200 Subject: [PATCH 05/17] remove_header_protection: Avoid copies As before, avoid unnecessary copies of std::vector instances. --- analyzer/decrypt_crypto.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 86c6cb6..302e61c 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -115,9 +115,9 @@ std::vector hkdf_expand(size_t out_len, const std::vector& key Removes the header protection from the INITIAL packet and returns a DecryptionInformation struct that is partially filled */ -DecryptionInformation remove_header_protection(std::vector client_hp, +DecryptionInformation remove_header_protection(const std::vector& client_hp, uint64_t encrypted_offset, - std::vector encrypted_packet) + const std::vector& encrypted_packet) { DecryptionInformation decryptInfo; int outlen; @@ -128,13 +128,13 @@ DecryptionInformation remove_header_protection(std::vector client_hp, // Passing an 1 means ENCRYPT EVP_CipherInit_ex(ctx, NULL, NULL, client_hp.data(), NULL, 1); - std::vector sample(encrypted_packet.begin() + encrypted_offset + - MAXIMUM_PACKET_NUMBER_LENGTH, + assert(encrypted_packet.size() >= + encrypted_offset + MAXIMUM_PACKET_NUMBER_LENGTH + AEAD_SAMPLE_LENGTH); - encrypted_packet.begin() + encrypted_offset + - MAXIMUM_PACKET_NUMBER_LENGTH + AEAD_SAMPLE_LENGTH); - std::vector mask(sample.size()); - EVP_CipherUpdate(ctx, mask.data(), &outlen, sample.data(), AEAD_SAMPLE_LENGTH); + const uint8_t* sample = &encrypted_packet[encrypted_offset + MAXIMUM_PACKET_NUMBER_LENGTH]; + + std::vector mask(AEAD_SAMPLE_LENGTH); + EVP_CipherUpdate(ctx, mask.data(), &outlen, sample, AEAD_SAMPLE_LENGTH); EVP_CIPHER_CTX_free(ctx); // To determine the actual packet number length, @@ -175,8 +175,8 @@ DecryptionInformation remove_header_protection(std::vector client_hp, // Store the information back in the struct decryptInfo.packet_number = decoded_packet_number; decryptInfo.packet_number_length = recovered_packet_number_length; - decryptInfo.protected_header = protected_header; - decryptInfo.unprotected_header = unprotected_header; + decryptInfo.protected_header = std::move(protected_header); + decryptInfo.unprotected_header = std::move(unprotected_header); return decryptInfo; } From 1430a498b78215561b983158827efea684b70b53 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 16:46:02 +0200 Subject: [PATCH 06/17] calculate_nonce: Pass std::vector by const-reference --- analyzer/decrypt_crypto.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 302e61c..414dedf 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -184,7 +184,7 @@ DecryptionInformation remove_header_protection(const std::vector& clien Calculate the nonce for the AEAD by XOR'ing the CLIENT_IV and the decoded packet number, and returns the nonce */ -std::vector calculate_nonce(std::vector client_iv, uint64_t packet_number) +std::vector calculate_nonce(const std::vector& client_iv, uint64_t packet_number) { std::vector nonce = client_iv; From 93f7f12b0aa66b4f9b4cf44ff578dd246b056303 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 16:55:00 +0200 Subject: [PATCH 07/17] decrypt: Some more std::vector copy reduction ...and return hilti::rt::Bytes directly. --- analyzer/decrypt_crypto.cc | 40 +++++++++++++++----------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 414dedf..8d03554 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -201,22 +201,20 @@ std::vector calculate_nonce(const std::vector& client_iv, uint Function that calls the AEAD decryption routine, and returns the decrypted data */ -std::vector decrypt(std::vector client_key, std::vector encrypted_packet, - uint64_t payload_length, DecryptionInformation decryptInfo) + +hilti::rt::Bytes decrypt(const std::vector& client_key, + const std::vector& encrypted_packet, uint64_t payload_length, + const DecryptionInformation& decryptInfo) { int out, out2, res; - std::vector encrypted_payload( - encrypted_packet.begin() + decryptInfo.protected_header.size(), - - encrypted_packet.begin() + decryptInfo.protected_header.size() + payload_length - - decryptInfo.packet_number_length - AEAD_TAG_LENGTH); + const uint8_t* encrypted_payload = &encrypted_packet[decryptInfo.protected_header.size()]; + int encrypted_payload_size = payload_length - decryptInfo.packet_number_length - + AEAD_TAG_LENGTH; - std::vector tag_to_check( - encrypted_packet.begin() + decryptInfo.protected_header.size() + payload_length - - decryptInfo.packet_number_length - AEAD_TAG_LENGTH, - - encrypted_packet.begin() + decryptInfo.protected_header.size() + payload_length - - decryptInfo.packet_number_length); + const uint8_t* tag_to_check = + &encrypted_packet[decryptInfo.protected_header.size() + payload_length - + decryptInfo.packet_number_length - AEAD_TAG_LENGTH]; + int tag_to_check_length = AEAD_TAG_LENGTH; unsigned char decrypt_buffer[MAXIMUM_PACKET_LENGTH]; @@ -235,7 +233,7 @@ std::vector decrypt(std::vector client_key, std::vector decrypt(std::vector client_key, std::vector decrypted_data(decrypt_buffer, decrypt_buffer + out); - return decrypted_data; + // Copy the decrypted data from the decrypted buffer into a Bytes instance. + return hilti::rt::Bytes(decrypt_buffer, decrypt_buffer + out); } /* @@ -301,9 +297,5 @@ hilti::rt::Bytes decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterat // Calculate the correct nonce for the decryption decryptInfo.nonce = calculate_nonce(iv, decryptInfo.packet_number); - std::vector decrypted_data = decrypt(key, e_pkt, payload_length, decryptInfo); - - // Return it as hilti Bytes again - hilti::rt::Bytes decr(decrypted_data.begin(), decrypted_data.end()); - return decr; + return decrypt(key, e_pkt, payload_length, decryptInfo); } From 6408f4d92cc46bc69786dfedeaf19be7e62fc5cb Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Thu, 5 Oct 2023 17:35:26 +0200 Subject: [PATCH 08/17] analyzer: Eat padding more efficiently --- analyzer/QUIC.spicy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index c252dc7..562af12 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -208,7 +208,11 @@ public type Frame = unit(header: LongHeaderPacket, from_client: bool, crypto_sin crypto_sink.write(self.c.cryptodata, self.c.offset.result); } FrameType::CONNECTION_CLOSE1 -> : ConnectionClosePayload(header); +@if SPICY_VERSION >= 10800 + FrameType::PADDING -> : skip /\x00*/; # eat the padding +@else FrameType::PADDING -> : /\x00*/; # eat the padding +@endif FrameType::PING -> : void; * -> : void { throw "unhandled frame type %s in %s" % (self.frame_type, header.first_byte.packet_type); From ea4c2de5cd070f414bd7f4fa2a09ced6510846d5 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 11:13:40 +0200 Subject: [PATCH 09/17] should_buffer/can_decrypt: Unify Now that we do not buffer the packet anymore explicitly, we do not need a should_buffer() method. --- analyzer/QUIC.spicy | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index 562af12..9e4f83d 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -11,27 +11,24 @@ public function decrypt_crypto_payload(packet: iterator, connection_id: ## Context - tracked in one connection ############## -# Should we buffer the packet? -function should_buffer(context: ConnectionIDInfo, is_client: bool): bool { - if ( is_client ) - return ! context.client_initial_processed; - - return context.client_initial_processed - && |context.initial_destination_conn_id| > 0 - && ! context.server_initial_processed; -} - # Can we decrypt? -function can_decrypt(long_header: LongHeaderPacket): bool { +function can_decrypt(long_header: LongHeaderPacket, context: ConnectionIDInfo, is_client: bool): bool { if ( long_header.first_byte.packet_type != LongPacketType::INITIAL ) return False; # decrypt_crypto_payload() has known secrets for version 1, nothing else. - if ( long_header.version == 0x00000001 ) - return True; + if ( long_header.version != 0x00000001 ) + return False; + + if ( is_client ) + return ! context.client_initial_processed; - return False; + # This is the responder, can only decrypt if we have an initial + # destination_id from the client + return context.client_initial_processed + && |context.initial_destination_conn_id| > 0 + && ! context.server_initial_processed; } type ConnectionIDInfo = struct { @@ -401,7 +398,7 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { @endif } - if ( can_decrypt(self.long_header) && should_buffer(context, from_client) ) { + if ( can_decrypt(self.long_header, context, from_client) ) { # Initialize sink/buffer for accumulating CRYPTO frames. # From 40d6f174cf8e73f60ad9843c81601e106018fa0a Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 11:26:59 +0200 Subject: [PATCH 10/17] Remove InitialByte I suspect the structure here can be improved, but given we're only interested in the form, replace with an anonymous uint8 field. --- analyzer/QUIC.spicy | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index 9e4f83d..da5fe46 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -122,14 +122,6 @@ type FrameType = enum { # Helper units ############## -# Used to peek into the next byte and determine if it's a long or short packet -public type InitialByte = unit { - initialbyte: bitfield(8) { - header_form: 7 &convert=cast(cast($$)); - long_packet_type: 4..5 &convert=cast(cast($$)); - }; -}; - type VariableLengthInteger = unit { var bytes_to_parse: uint64; var result: uint64; @@ -342,7 +334,6 @@ type CryptoBuffer = unit() { ############## type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { var header_form: HeaderForm; - var long_packet_type: LongPacketType; var decrypted_data: bytes; var full_packet: bytes; var start: iterator; @@ -367,12 +358,10 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { self.start = self.input(); } - # Peek into the header to check if it's a SHORT or LONG header - # and get the LONG packet type. - : InitialByte &parse-at=self.input() { - self.header_form = $$.initialbyte.header_form; - self.long_packet_type = $$.initialbyte.long_packet_type; - self.set_input(self.start); + # Peek into the first byte and determine the header type. + : uint8 { + self.header_form = ($$ & 0x80 == 0x80) ? HeaderForm::LONG : HeaderForm::SHORT; + self.set_input(self.start); # rewind } # Depending on the header, parse it and update the src/dest ConnectionID's From 552256e82ab74f04083052341ec2d6e7068468df Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 11:33:42 +0200 Subject: [PATCH 11/17] Use "skip" for encrypted payload values There's not much point accumulating it in fields if we're never using it, anyhow. --- analyzer/QUIC.spicy | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index da5fe46..12913f2 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -209,11 +209,6 @@ public type Frame = unit(header: LongHeaderPacket, from_client: bool, crypto_sin }; }; -# TODO: investigate whether we can do something useful with this -public type EncryptedLongPacketPayload = unit { - payload: bytes &eod; -}; - type CRYPTOPayload = unit(from_client: bool) { offset: VariableLengthInteger; length: VariableLengthInteger; @@ -312,7 +307,20 @@ public type ShortHeader = unit(dest_conn_id_length: uint8) { # TODO: investigate whether we can parse something useful out of this public type ShortPacketPayload = unit { +@if SPICY_VERSION >= 10800 + payload: skip bytes &eod; +@else + payload: bytes &eod; +@endif +}; + +# TODO: investigate whether we can do something useful with this +public type EncryptedLongPacketPayload = unit { +@if SPICY_VERSION >= 10800 + payload: skip bytes &eod; +@else payload: bytes &eod; +@endif }; # Buffer all crypto messages (which might be fragmented and unordered) From 17d3074ad1a87a10e233aef7887c25dbb8771ef5 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 11:53:04 +0200 Subject: [PATCH 12/17] decrypt_crypto: Switch to std::array --- analyzer/decrypt_crypto.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 8d03554..1e55fc8 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -8,6 +8,7 @@ refactors as C++ development is not our main profession. */ // Default imports +#include #include #include #include @@ -133,7 +134,7 @@ DecryptionInformation remove_header_protection(const std::vector& clien const uint8_t* sample = &encrypted_packet[encrypted_offset + MAXIMUM_PACKET_NUMBER_LENGTH]; - std::vector mask(AEAD_SAMPLE_LENGTH); + std::array mask; EVP_CipherUpdate(ctx, mask.data(), &outlen, sample, AEAD_SAMPLE_LENGTH); EVP_CIPHER_CTX_free(ctx); @@ -216,7 +217,7 @@ hilti::rt::Bytes decrypt(const std::vector& client_key, decryptInfo.packet_number_length - AEAD_TAG_LENGTH]; int tag_to_check_length = AEAD_TAG_LENGTH; - unsigned char decrypt_buffer[MAXIMUM_PACKET_LENGTH]; + std::array decrypt_buffer; // Setup context auto cipher = EVP_aes_128_gcm(); @@ -241,14 +242,14 @@ hilti::rt::Bytes decrypt(const std::vector& client_key, // Set the actual data to decrypt data into the decrypt_buffer. The amount of // byte decrypted is stored into `out` - EVP_CipherUpdate(ctx, decrypt_buffer, &out, encrypted_payload, encrypted_payload_size); + EVP_CipherUpdate(ctx, decrypt_buffer.data(), &out, encrypted_payload, encrypted_payload_size); // Validate whether the decryption was successful or not EVP_CipherFinal_ex(ctx, NULL, &out2); EVP_CIPHER_CTX_free(ctx); // Copy the decrypted data from the decrypted buffer into a Bytes instance. - return hilti::rt::Bytes(decrypt_buffer, decrypt_buffer + out); + return hilti::rt::Bytes(decrypt_buffer.data(), decrypt_buffer.data() + out); } /* From 1e1273960cc4996b7acaa7e761bba9b850e49921 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 11:53:15 +0200 Subject: [PATCH 13/17] decrypt_crypto: Switch to UnsafeConstIterator We only need to copy out the buffer, no need to be overly safe. --- analyzer/decrypt_crypto.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 1e55fc8..8a8fcfc 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -273,7 +273,7 @@ hilti::rt::Bytes decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterat // Fill in the entire packet bytes std::vector e_pkt; - hilti::rt::stream::SafeConstIterator it = packet_stream; + hilti::rt::stream::detail::UnsafeConstIterator it{packet_stream}; while ( ! it.isEnd() ) e_pkt.push_back(*(it++)); From 713794a08c6f6294dc0ec73ce3b08738c454f1c5 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 12:02:46 +0200 Subject: [PATCH 14/17] decrypt_crypto: Move most everything into anonymous namespace Think previously we exported all the symbols :-/ --- analyzer/QUIC.spicy | 8 +++++++- analyzer/decrypt_crypto.cc | 16 +++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index 12913f2..e2ab59f 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -4,7 +4,13 @@ import spicy; import zeek; # The interface to the C++ code that handles the decryption of the INITIAL packet payload using well-known keys -public function decrypt_crypto_payload(packet: iterator, connection_id: bytes, encrypted_offset: uint64, payload_offset: uint64, from_client: bool): bytes &cxxname="decrypt_crypto_payload"; +public function decrypt_crypto_payload( + packet: iterator, + connection_id: bytes, + encrypted_offset: uint64, + payload_offset: uint64, + from_client: bool +): bytes &cxxname="QUIC_decrypt_crypto_payload"; ############## diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index 8a8fcfc..a172bce 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -23,6 +23,9 @@ refactors as C++ development is not our main profession. // Import HILTI #include +namespace + { + // Struct to store decryption info for this specific connection struct DecryptionInformation { @@ -252,16 +255,19 @@ hilti::rt::Bytes decrypt(const std::vector& client_key, return hilti::rt::Bytes(decrypt_buffer.data(), decrypt_buffer.data() + out); } + } + /* Function that is called from Spicy. It's a wrapper around `process_data`; it stores all the passed data in a global struct and then calls `process_data`, which will eventually return the decrypted data and pass it back to Spicy. */ -hilti::rt::Bytes decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterator& packet_stream, - const hilti::rt::Bytes& connection_id, - const hilti::rt::integer::safe& encrypted_offset, - const hilti::rt::integer::safe& payload_length, - const hilti::rt::Bool& from_client) +hilti::rt::Bytes +QUIC_decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterator& packet_stream, + const hilti::rt::Bytes& connection_id, + const hilti::rt::integer::safe& encrypted_offset, + const hilti::rt::integer::safe& payload_length, + const hilti::rt::Bool& from_client) { if ( payload_length < 20 ) From 22fc67265b5342458e2f7d4ad8a729e0419153ab Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 12:10:15 +0200 Subject: [PATCH 15/17] decrypt_crypto: Remove redundant protected_header copy --- analyzer/decrypt_crypto.cc | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index a172bce..ba82627 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -30,7 +30,6 @@ namespace struct DecryptionInformation { std::vector unprotected_header; - std::vector protected_header; uint64_t packet_number; std::vector nonce; uint8_t packet_number_length; @@ -172,14 +171,10 @@ DecryptionInformation remove_header_protection(const std::vector& clien decoded_packet_number = unprotected_header[encrypted_offset + i] | (decoded_packet_number << 8); } - std::vector protected_header(encrypted_packet.begin(), - encrypted_packet.begin() + encrypted_offset + - recovered_packet_number_length); // Store the information back in the struct decryptInfo.packet_number = decoded_packet_number; decryptInfo.packet_number_length = recovered_packet_number_length; - decryptInfo.protected_header = std::move(protected_header); decryptInfo.unprotected_header = std::move(unprotected_header); return decryptInfo; } @@ -211,12 +206,12 @@ hilti::rt::Bytes decrypt(const std::vector& client_key, const DecryptionInformation& decryptInfo) { int out, out2, res; - const uint8_t* encrypted_payload = &encrypted_packet[decryptInfo.protected_header.size()]; + const uint8_t* encrypted_payload = &encrypted_packet[decryptInfo.unprotected_header.size()]; int encrypted_payload_size = payload_length - decryptInfo.packet_number_length - AEAD_TAG_LENGTH; const uint8_t* tag_to_check = - &encrypted_packet[decryptInfo.protected_header.size() + payload_length - + &encrypted_packet[decryptInfo.unprotected_header.size() + payload_length - decryptInfo.packet_number_length - AEAD_TAG_LENGTH]; int tag_to_check_length = AEAD_TAG_LENGTH; From 6cc078bfafe5cc5b7aa3fbebf5cc0009d6773fea Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 12:37:53 +0200 Subject: [PATCH 16/17] analyzer: Some more skip annotations We're not actually using any of the fields, so may as well use skip. --- analyzer/QUIC.spicy | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index e2ab59f..60d8fef 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -265,19 +265,31 @@ type InitialPacket = unit(header: LongHeaderPacket) { # includes the packet number field, but we # do not know its length yet. We need the # payload for sampling, however. +@if SPICY_VERSION >= 10800 + payload: skip bytes &size=self.length.result; +@else payload: bytes &size=self.length.result; +@endif }; type ZeroRTTPacket = unit(header: LongHeaderPacket) { var header: LongHeaderPacket = header; length: VariableLengthInteger; +@if SPICY_VERSION >= 10800 + payload: skip bytes &size=self.length.result; +@else payload: bytes &size=self.length.result; +@endif }; type HandshakePacket = unit(header: LongHeaderPacket) { var header: LongHeaderPacket = header; length: VariableLengthInteger; +@if SPICY_VERSION >= 10800 + payload: skip bytes &size=self.length.result; +@else payload: bytes &size=self.length.result; +@endif }; From ed32433dc3d58c695b4d401a5fc68111999c1979 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Fri, 6 Oct 2023 18:06:51 +0200 Subject: [PATCH 17/17] decrypt_crypto: Move back to passing hilti::rt::Bytes This removes the iterator usage but removes the explicit copy into std::vector<> in favor of using the hilti::rt::Bytes::data() content directly. Hide the reinterpret_cast<> behind a small helper function. And further feedback from Benjamin. --- analyzer/QUIC.spicy | 136 +++++++++++++++++++------------------ analyzer/decrypt_crypto.cc | 90 +++++++++++++----------- 2 files changed, 121 insertions(+), 105 deletions(-) diff --git a/analyzer/QUIC.spicy b/analyzer/QUIC.spicy index 60d8fef..bd040ae 100644 --- a/analyzer/QUIC.spicy +++ b/analyzer/QUIC.spicy @@ -5,7 +5,7 @@ import zeek; # The interface to the C++ code that handles the decryption of the INITIAL packet payload using well-known keys public function decrypt_crypto_payload( - packet: iterator, + all_data: bytes, connection_id: bytes, encrypted_offset: uint64, payload_offset: uint64, @@ -359,7 +359,6 @@ type CryptoBuffer = unit() { # A UDP datagram contains one or more QUIC packets. ############## type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { - var header_form: HeaderForm; var decrypted_data: bytes; var full_packet: bytes; var start: iterator; @@ -374,24 +373,29 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { context.ssl_handle = zeek::protocol_handle_get_or_create("SSL"); } @else - if ( ! context.did_ssl_begin ) - { + if ( ! context.did_ssl_begin ) { zeek::protocol_begin("SSL"); context.did_ssl_begin = True; - } + } @endif self.start = self.input(); } # Peek into the first byte and determine the header type. - : uint8 { - self.header_form = ($$ & 0x80 == 0x80) ? HeaderForm::LONG : HeaderForm::SHORT; + first_byte: bitfield(8) { + header_form: 7 &convert=HeaderForm($$); + }; + + # TODO: Consider bitfield based look-ahead-parsing in the switch below + # to avoid this rewinding here. It's a hack. + : void { self.set_input(self.start); # rewind } + # Depending on the header, parse it and update the src/dest ConnectionID's - switch ( self.header_form ) { + switch ( self.first_byte.header_form ) { HeaderForm::SHORT -> short_header: ShortHeader(context.client_cid_len); HeaderForm::LONG -> long_header: LongHeaderPacket { # For now, only allow a change of src/dest ConnectionID's for INITIAL packets. @@ -412,77 +416,77 @@ type Packet = unit(from_client: bool, context: ConnectionIDInfo&) { context.did_ssl_begin = False; @endif } + } + }; - if ( can_decrypt(self.long_header, context, from_client) ) { - - # Initialize sink/buffer for accumulating CRYPTO frames. - # - # TODO: Attach this to the context instead of unit. - self.crypto_buffer = new CryptoBuffer(); - self.crypto_sink.connect(self.crypto_buffer); - - if ( from_client ) { - context.server_cid_len = self.long_header.dest_conn_id_len; - context.client_cid_len = self.long_header.src_conn_id_len; - - # This means that here, we can try to decrypt the initial packet! - # All data is accessible via the `long_header` unit - - self.decrypted_data = decrypt_crypto_payload( - self.start, - self.long_header.dest_conn_id, - self.long_header.encrypted_offset, - self.long_header.payload_length, - from_client); - - # Set this to be the seed for the decryption - if ( |context.initial_destination_conn_id| == 0 ) { - context.initial_destination_conn_id = self.long_header.dest_conn_id; - } - - } else { - context.server_cid_len = self.long_header.src_conn_id_len; - context.client_cid_len = self.long_header.dest_conn_id_len; - - # Assuming that the client set up the connection, this can be considered the first - # received Initial from the client. So disable change of ConnectionID's afterwards - self.decrypted_data = decrypt_crypto_payload( - self.start, - context.initial_destination_conn_id, - self.long_header.encrypted_offset, - self.long_header.payload_length, - from_client); - } - - # We attempted decryption, but it failed. Just reject the - # input and assume Zeek will disable the analyzer for this - # connection. - if ( |self.decrypted_data| == 0 ) - throw "decryption failed"; - } + # Slurp in the whole packet if we determined we have a chance to decrypt. + all_data: bytes &parse-at=self.start &eod if ( self?.long_header && can_decrypt(self.long_header, context, from_client) ) { + self.crypto_buffer = new CryptoBuffer(); + self.crypto_sink.connect(self.crypto_buffer); + + if ( from_client ) { + context.server_cid_len = self.long_header.dest_conn_id_len; + context.client_cid_len = self.long_header.src_conn_id_len; + + # This means that here, we can try to decrypt the initial packet! + # All data is accessible via the `long_header` unit + self.decrypted_data = decrypt_crypto_payload( + self.all_data, + self.long_header.dest_conn_id, + self.long_header.encrypted_offset, + self.long_header.payload_length, + from_client + ); - # If it's a reply from the server and it's not a REPLY, we assume the keys are restablished and decryption is no longer possible - # TODO: verify if this is actually correct per RFC - if (self.long_header.first_byte.packet_type != LongPacketType::RETRY && ! from_client) { - context.server_initial_processed = True; - context.client_initial_processed = True; - } + # Set this to be the seed for the decryption + if ( |context.initial_destination_conn_id| == 0 ) { + context.initial_destination_conn_id = self.long_header.dest_conn_id; + } + + } else { + context.server_cid_len = self.long_header.src_conn_id_len; + context.client_cid_len = self.long_header.dest_conn_id_len; + + # Assuming that the client set up the connection, this can be considered the first + # received Initial from the client. So disable change of ConnectionID's afterwards + self.decrypted_data = decrypt_crypto_payload( + self.all_data, + context.initial_destination_conn_id, + self.long_header.encrypted_offset, + self.long_header.payload_length, + from_client + ); } - }; + + # We attempted decryption, but it failed. Just reject the + # input and assume Zeek will disable the analyzer for this + # connection. + if ( |self.decrypted_data| == 0 ) + throw "decryption failed"; + + # If this was a reply from the server and it's not a RETRY, we assume the keys + # are restablished and decryption is no longer possible + # + # TODO: verify if this is actually correct per RFC + if ( self.long_header.first_byte.packet_type != LongPacketType::RETRY && ! from_client ) { + context.server_initial_processed = True; + context.client_initial_processed = True; + } + } # Depending on the type of header and whether we were able to decrypt # some of it, parse the remaining payload. - : ShortPacketPayload if (self.header_form == HeaderForm::SHORT); - : EncryptedLongPacketPayload if (self.header_form == HeaderForm::LONG && |self.decrypted_data| == 0); + : ShortPacketPayload if (self.first_byte.header_form == HeaderForm::SHORT); + : EncryptedLongPacketPayload if (self.first_byte.header_form == HeaderForm::LONG && |self.decrypted_data| == 0); # If this was packet with a long header and decrypted data exists, attempt # to parse the plain QUIC frames from it. - frames: Frame(self.long_header, from_client, self.crypto_sink)[] &parse-from=self.decrypted_data if (self.header_form == HeaderForm::LONG && |self.decrypted_data| > 0); + frames: Frame(self.long_header, from_client, self.crypto_sink)[] &parse-from=self.decrypted_data if (self.first_byte.header_form == HeaderForm::LONG && |self.decrypted_data| > 0); # Once the Packet is fully parsed, pass the accumulated CRYPTO frames # to the SSL analyzer as handshake data. on %done { - # print "packet done", zeek::is_orig(), self.header_form, |self.decrypted_data|; + # print "packet done", zeek::is_orig(), self.first_byte.header_form, |self.decrypted_data|; if ( self.crypto_buffer != Null && |self.crypto_buffer.buffered| > 0 ) { local handshake_data = self.crypto_buffer.buffered; diff --git a/analyzer/decrypt_crypto.cc b/analyzer/decrypt_crypto.cc index ba82627..e0f034b 100644 --- a/analyzer/decrypt_crypto.cc +++ b/analyzer/decrypt_crypto.cc @@ -35,6 +35,14 @@ struct DecryptionInformation uint8_t packet_number_length; }; +// Return rt::hilti::Bytes::data() value as const uint8_t* +// +// This should be alright: https://stackoverflow.com/a/15172304 +inline const uint8_t* data_as_uint8(const hilti::rt::Bytes& b) + { + return reinterpret_cast(b.data()); + } + /* Constants used in the HKDF functions. HKDF-Expand-Label uses labels such as 'quic key' and 'quic hp'. These labels can obviously be @@ -86,8 +94,7 @@ std::vector hkdf_extract(const hilti::rt::Bytes& connection_id) EVP_PKEY_derive_init(pctx); EVP_PKEY_CTX_hkdf_mode(pctx, EVP_PKEY_HKDEF_MODE_EXTRACT_ONLY); EVP_PKEY_CTX_set_hkdf_md(pctx, digest); - EVP_PKEY_CTX_set1_hkdf_key(pctx, reinterpret_cast(connection_id.data()), - connection_id.size()); + EVP_PKEY_CTX_set1_hkdf_key(pctx, data_as_uint8(connection_id), connection_id.size()); EVP_PKEY_CTX_set1_hkdf_salt(pctx, INITIAL_SALT_V1.data(), INITIAL_SALT_V1.size()); EVP_PKEY_derive(pctx, out_temp.data(), &initial_secret_len); EVP_PKEY_CTX_free(pctx); @@ -120,7 +127,7 @@ that is partially filled */ DecryptionInformation remove_header_protection(const std::vector& client_hp, uint64_t encrypted_offset, - const std::vector& encrypted_packet) + const hilti::rt::Bytes& all_data) { DecryptionInformation decryptInfo; int outlen; @@ -131,10 +138,11 @@ DecryptionInformation remove_header_protection(const std::vector& clien // Passing an 1 means ENCRYPT EVP_CipherInit_ex(ctx, NULL, NULL, client_hp.data(), NULL, 1); - assert(encrypted_packet.size() >= - encrypted_offset + MAXIMUM_PACKET_NUMBER_LENGTH + AEAD_SAMPLE_LENGTH); + static_assert(AEAD_SAMPLE_LENGTH > 0); + assert(all_data.size() >= encrypted_offset + MAXIMUM_PACKET_NUMBER_LENGTH + AEAD_SAMPLE_LENGTH); - const uint8_t* sample = &encrypted_packet[encrypted_offset + MAXIMUM_PACKET_NUMBER_LENGTH]; + const uint8_t* sample = data_as_uint8(all_data) + encrypted_offset + + MAXIMUM_PACKET_NUMBER_LENGTH; std::array mask; EVP_CipherUpdate(ctx, mask.data(), &outlen, sample, AEAD_SAMPLE_LENGTH); @@ -142,7 +150,7 @@ DecryptionInformation remove_header_protection(const std::vector& clien // To determine the actual packet number length, // we have to remove the mask from the first byte - uint8_t first_byte = encrypted_packet[0]; + uint8_t first_byte = data_as_uint8(all_data)[0]; if ( first_byte & 0x80 ) { @@ -157,9 +165,8 @@ DecryptionInformation remove_header_protection(const std::vector& clien int recovered_packet_number_length = (first_byte & 0x03) + 1; // .. and use this to reconstruct the (partially) unprotected header - std::vector unprotected_header(encrypted_packet.begin(), - - encrypted_packet.begin() + encrypted_offset + + std::vector unprotected_header(data_as_uint8(all_data), + data_as_uint8(all_data) + encrypted_offset + recovered_packet_number_length); uint32_t decoded_packet_number = 0; @@ -183,17 +190,12 @@ DecryptionInformation remove_header_protection(const std::vector& clien Calculate the nonce for the AEAD by XOR'ing the CLIENT_IV and the decoded packet number, and returns the nonce */ -std::vector calculate_nonce(const std::vector& client_iv, uint64_t packet_number) +std::vector calculate_nonce(std::vector client_iv, uint64_t packet_number) { - std::vector nonce = client_iv; - for ( int i = 0; i < 8; ++i ) - { - nonce[AEAD_IV_LEN - 1 - i] ^= (uint8_t)(packet_number >> 8 * i); - } + client_iv[AEAD_IV_LEN - 1 - i] ^= (uint8_t)(packet_number >> 8 * i); - // Return the nonce - return nonce; + return client_iv; } /* @@ -201,18 +203,34 @@ Function that calls the AEAD decryption routine, and returns the decrypted data */ -hilti::rt::Bytes decrypt(const std::vector& client_key, - const std::vector& encrypted_packet, uint64_t payload_length, - const DecryptionInformation& decryptInfo) +hilti::rt::Bytes decrypt(const std::vector& client_key, const hilti::rt::Bytes& all_data, + uint64_t payload_length, const DecryptionInformation& decryptInfo) { int out, out2, res; - const uint8_t* encrypted_payload = &encrypted_packet[decryptInfo.unprotected_header.size()]; + + if ( payload_length < decryptInfo.packet_number_length + AEAD_TAG_LENGTH ) + throw hilti::rt::RuntimeError( + hilti::rt::fmt("payload too small %ld < %ld", payload_length, + decryptInfo.packet_number_length + AEAD_TAG_LENGTH)); + + const uint8_t* encrypted_payload = data_as_uint8(all_data) + + decryptInfo.unprotected_header.size(); + int encrypted_payload_size = payload_length - decryptInfo.packet_number_length - AEAD_TAG_LENGTH; - const uint8_t* tag_to_check = - &encrypted_packet[decryptInfo.unprotected_header.size() + payload_length - - decryptInfo.packet_number_length - AEAD_TAG_LENGTH]; + if ( encrypted_payload_size < 0 ) + throw hilti::rt::RuntimeError( + hilti::rt::fmt("encrypted_payload_size underflow %ld", encrypted_payload_size)); + + if ( all_data.size() < + decryptInfo.unprotected_header.size() + encrypted_payload_size + AEAD_TAG_LENGTH ) + throw hilti::rt::RuntimeError( + hilti::rt::fmt("all_data too short %ld < %ld", all_data.size(), + decryptInfo.unprotected_header.size() + encrypted_payload_size)); + + const void* tag_to_check = all_data.data() + decryptInfo.unprotected_header.size() + + encrypted_payload_size; int tag_to_check_length = AEAD_TAG_LENGTH; std::array decrypt_buffer; @@ -232,7 +250,8 @@ hilti::rt::Bytes decrypt(const std::vector& client_key, EVP_CipherInit_ex(ctx, NULL, NULL, client_key.data(), decryptInfo.nonce.data(), 0); // Set the tag to be validated after decryption - EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_CCM_SET_TAG, tag_to_check_length, (void*)tag_to_check); + EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_CCM_SET_TAG, tag_to_check_length, + const_cast(tag_to_check)); // Setting the second parameter to NULL will pass it as Associated Data EVP_CipherUpdate(ctx, NULL, &out, decryptInfo.unprotected_header.data(), @@ -258,8 +277,7 @@ it stores all the passed data in a global struct and then calls `process_data`, which will eventually return the decrypted data and pass it back to Spicy. */ hilti::rt::Bytes -QUIC_decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterator& packet_stream, - const hilti::rt::Bytes& connection_id, +QUIC_decrypt_crypto_payload(const hilti::rt::Bytes& all_data, const hilti::rt::Bytes& connection_id, const hilti::rt::integer::safe& encrypted_offset, const hilti::rt::integer::safe& payload_length, const hilti::rt::Bool& from_client) @@ -268,15 +286,9 @@ QUIC_decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterator& packet_s if ( payload_length < 20 ) throw hilti::rt::RuntimeError(hilti::rt::fmt("payload too small %ld < 20", payload_length)); - if ( (packet_stream + (encrypted_offset + payload_length - 1)).isEnd() ) - throw hilti::rt::RuntimeError( - hilti::rt::fmt("packet too small %ld", encrypted_offset + payload_length)); - - // Fill in the entire packet bytes - std::vector e_pkt; - hilti::rt::stream::detail::UnsafeConstIterator it{packet_stream}; - while ( ! it.isEnd() ) - e_pkt.push_back(*(it++)); + if ( (all_data.size() < encrypted_offset + payload_length) ) + throw hilti::rt::RuntimeError(hilti::rt::fmt("packet too small %ld %ld", all_data.size(), + encrypted_offset + payload_length)); std::vector initial_secret = hkdf_extract(connection_id); @@ -294,10 +306,10 @@ QUIC_decrypt_crypto_payload(const hilti::rt::stream::SafeConstIterator& packet_s std::vector iv = hkdf_expand(AEAD_IV_LEN, server_client_secret, IV_INFO); std::vector hp = hkdf_expand(AEAD_HP_LEN, server_client_secret, HP_INFO); - DecryptionInformation decryptInfo = remove_header_protection(hp, encrypted_offset, e_pkt); + DecryptionInformation decryptInfo = remove_header_protection(hp, encrypted_offset, all_data); // Calculate the correct nonce for the decryption decryptInfo.nonce = calculate_nonce(iv, decryptInfo.packet_number); - return decrypt(key, e_pkt, payload_length, decryptInfo); + return decrypt(key, all_data, payload_length, decryptInfo); }