Treat OHTTP non-final zero-length chunks as errors.

Per the recent [chunked OHTTP draft update](https://www.ietf.org/archive/id/draft-ietf-ohai-chunked-ohttp-07.html#section-6-6), a non-final chunk MUST NOT contain a zero-length plaintext and must be treated as a decryption error upon receipt.

PiperOrigin-RevId: 862088253

Author: Rickyp

quiche/oblivious_http/buffers/oblivious_http_request.cc

@@ -165,7 +165,8 @@ absl::StatusOr<std::string> ObliviousHttpRequest::EncryptChunk(
     absl::string_view plaintext_payload, const Context& context,
     bool is_final_chunk) {
   if (plaintext_payload.empty() && !is_final_chunk) {
-    return absl::InvalidArgumentError("Invalid input.");
+    return absl::InvalidArgumentError(
+        "A non-final chunk MUST NOT contain a zero-length plaintext.");
   }
 
   uint8_t* ad = nullptr;
@@ -277,6 +278,12 @@ absl::StatusOr<std::string> ObliviousHttpRequest::DecryptChunk(
     return SslErrorAsStatus("Failed to decrypt.",
                             absl::StatusCode::kInvalidArgument);
   }
+
+  if (decrypted_len == 0 && !is_final_chunk) {
+    return absl::InvalidArgumentError(
+        "Decrypted non-final chunk plaintext is zero-length. A non-final chunk "
+        "MUST NOT contain a zero-length plaintext.");
+  }
   decrypted.resize(decrypted_len);
   return decrypted;
 }

quiche/oblivious_http/buffers/oblivious_http_response.cc

@@ -346,7 +346,7 @@ absl::StatusOr<std::string> ObliviousHttpResponse::EncryptChunk(
   // Empty plaintext_payload is only allowed for the final chunk.
   if (!is_final_chunk && plaintext_payload.empty()) {
     return absl::InvalidArgumentError(
-        "Payload cannot be empty for non-final chunks.");
+        "A non-final chunk MUST NOT contain a zero-length plaintext.");
   }
   if (chunk_nonce.empty()) {
     return absl::InvalidArgumentError("Chunk nonce cannot be empty.");
@@ -405,7 +405,13 @@ absl::StatusOr<std::string> ObliviousHttpResponse::DecryptChunk(
           reinterpret_cast<const uint8_t*>(encrypted_chunk.data()),
           encrypted_chunk.size(), ad, ad_len)) {
     return SslErrorAsStatus(
-        "Failed to decrypt the response with derived AEAD key and nonce.");
+        "Failed to decrypt the response with derived AEAD key and nonce.",
+        absl::StatusCode::kInvalidArgument);
+  }
+  if (decrypted_len == 0 && !is_final_chunk) {
+    return absl::InvalidArgumentError(
+        "Decrypted non-final chunk plaintext is zero-length. A non-final chunk "
+        "MUST NOT contain a zero-length plaintext.");
   }
   decrypted.resize(decrypted_len);
   return decrypted;

quiche/oblivious_http/oblivious_http_client_test.cc

@@ -186,6 +186,41 @@ TEST(ObliviousHttpClient, TestEncryptingMultipleRequestsWithSingleInstance) {
   EXPECT_NE(serialized_ohttp_req_1, serialized_ohttp_req_2);
 }
 
+TEST(ChunkedObliviousHttpClient, DecryptZeroLengthNonFinalResponseChunkFails) {
+  TestChunkHandler chunk_handler;
+  // Contains the nonce and a zero-length
+  // encrypted response chunk.
+  std::string nonFinalZeroLengthEncryptedResponseChunk;
+  EXPECT_TRUE(absl::HexStringToBytes(
+      "a58cfdb3f69b5cb9ae328e25516dfed2109ac5a0b0ce59b9ff5bf6fe1ab2274715",
+      &nonFinalZeroLengthEncryptedResponseChunk));
+
+  // The seed used for generating the response context.
+  std::string seed;
+  EXPECT_TRUE(absl::HexStringToBytes(
+      "52c4a758a802cd8b936eceea314432798d5baf2d7e9235dc084ab1b9cfa2f736",
+      &seed));
+
+  absl::StatusOr<ObliviousHttpHeaderKeyConfig> key_config =
+      ObliviousHttpHeaderKeyConfig::Create(
+          /*key_id=*/1, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, EVP_HPKE_HKDF_SHA256,
+          EVP_HPKE_AES_128_GCM);
+  QUICHE_ASSERT_OK(key_config);
+
+  absl::StatusOr<ChunkedObliviousHttpClient> chunked_client =
+      ChunkedObliviousHttpClient::Create(GetHpkePublicKey(), key_config.value(),
+                                         &chunk_handler,
+
+                                         seed);
+
+  QUICHE_ASSERT_OK(chunked_client);
+  EXPECT_EQ(
+      chunked_client
+          ->DecryptResponse(nonFinalZeroLengthEncryptedResponseChunk, false)
+          .code(),
+      absl::StatusCode::kInvalidArgument);
+}
+
 TEST(ObliviousHttpClient, TestInvalidHPKEKey) {
   // Invalid public key.
   EXPECT_EQ(ObliviousHttpClient::Create(
@@ -671,7 +706,7 @@ TEST(ChunkedObliviousHttpClient, DecryptResponseWithCorruptedNonceFails) {
   corrupted_response[0] ^= 0x01;  // Corrupt first byte of nonce.
   EXPECT_THAT(
       chunk_client->DecryptResponse(corrupted_response, /*end_stream=*/true),
-      StatusIs(absl::StatusCode::kInternal));
+      StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
 TEST(ChunkedObliviousHttpClient, DecryptResponseWithCorruptedChunkFails) {
@@ -713,7 +748,7 @@ TEST(ChunkedObliviousHttpClient, DecryptResponseWithCorruptedChunkFails) {
                                          // 12 bytes nonce + 1 byte len.
   EXPECT_THAT(chunk_client->DecryptResponse(corrupted_chunk_response,
                                             /*end_stream=*/false),
-              StatusIs(absl::StatusCode::kInternal));
+              StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
 TEST(ChunkedObliviousHttpClient, DecryptResponseWithCorruptedFinalChunkFails) {
@@ -755,7 +790,7 @@ TEST(ChunkedObliviousHttpClient, DecryptResponseWithCorruptedFinalChunkFails) {
              // 12 bytes nonce + 1 byte chunk indicator==0.
   EXPECT_THAT(
       chunk_client->DecryptResponse(corrupted_response, /*end_stream=*/true),
-      StatusIs(absl::StatusCode::kInternal));
+      StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
 TEST(ChunkedObliviousHttpClient, DecryptResponseAfterEndStreamReturnsError) {

quiche/oblivious_http/oblivious_http_gateway_test.cc

@@ -283,6 +283,35 @@ TEST(ChunkedObliviousHttpGateway,
             absl::StatusCode::kInternal);
 }
 
+TEST(ChunkedObliviousHttpGateway, DecryptZeroLengthNonFinalRequestChunkFails) {
+  TestChunkHandler chunk_handler;
+  // Contains the initial encapsulated request header and a zero-length
+  // encrypted request chunk.
+  std::string nonFinalZeroLengthEncryptedRequestChunk;
+  ASSERT_TRUE(
+      absl::HexStringToBytes("010020000100013d9dde87c236fcbcfa540087557a8653738"
+                             "0e4731385cdcfcc391f7948b9"
+                             "590b108495352919fa03b19a997bb5621b1480",
+                             &nonFinalZeroLengthEncryptedRequestChunk));
+
+  // The key used to generate the above request.
+  std::string privateKey;
+  ASSERT_TRUE(absl::HexStringToBytes(
+      "b77431ecfa8f4cfc30d6e467aafa06944dffe28cb9dd1409e33a3045f5adc8a1",
+      &privateKey));
+  auto gateway = ChunkedObliviousHttpGateway::Create(
+      privateKey,
+      GetOhttpKeyConfig(
+          /*key_id=*/1, EVP_HPKE_DHKEM_X25519_HKDF_SHA256, EVP_HPKE_HKDF_SHA256,
+          EVP_HPKE_AES_128_GCM),
+      &chunk_handler);
+
+  EXPECT_EQ(
+      gateway->DecryptRequest(nonFinalZeroLengthEncryptedRequestChunk, false)
+          .code(),
+      absl::StatusCode::kInvalidArgument);
+}
+
 TEST(ObliviousHttpGateway, TestDecryptingMultipleRequestsWithSingleInstance) {
   auto instance = ObliviousHttpGateway::Create(
       GetHpkePrivateKey(),