GH-48858: [C++][Parquet] Avoid re-serializing footer for signature verification #48859

pitrou · 2026-01-14T15:16:35Z

Rationale for this change

When reading an encrypted Parquet file with a plaintext footer, the Parquet reader is able to verify footer integrity by comparing the signature in the file with the one computed by encrypting the footer.

However, the way it does this is to first re-serializes the deserialized footer using Thrift. This has several issues:

it's inefficient
it's not obvious that it will always produce the same Thrift encoding as the original, leading to spurious signature verification failures
if the original footer deserializes to invalid enum values, attempting to serialize it again will lead to undefined behavior

Reason 3 is what allowed this to be uncovered by OSS-Fuzz (see https://oss-fuzz.com/testcase-detail/4740205688193024).

This PR switches to reusing the original serialized metadata.

Are these changes tested?

Yes, by existing tests and new fuzz regression file.

Are there any user-facing changes?

No.

GitHub Issue: [C++][Parquet] Plaintext footer signature verification re-serializes footer #48858

pitrou · 2026-01-14T15:29:25Z

cpp/src/parquet/metadata.h

+  PARQUET_DEPRECATED("Deprecated in 24.0.0. Use the two-argument overload instead.")
  bool VerifySignature(const void* signature);


I opted to deprecate this but we might also remove it, as it seems this API is meant for internal use? @adamreeve @EnricoMi

Yes I can't see why this would need to be part of the public API. In which case, the new method should probably be private or internal too.

If we remove it and make the new method internal then this could block someone from upgrading if they had a legitimate use case for this. Maybe we should deprecate it and suggest users make an issue if they need this functionality? Also OK by me if you just want to remove it though, it seems unlikely this would be used.

pitrou · 2026-01-14T15:30:34Z

cpp/src/parquet/metadata.cc

+  bool VerifySignature(std::span<const uint8_t> serialized_metadata,
+                       std::span<const uint8_t> signature) {


I implemented this as a method of FileMetaData but the only member it uses is the FileDecryptor, so perhaps this should be moved elsewhere (or made static?).

pitrou · 2026-01-14T15:45:53Z

Hmm, it looks like we need to wait for #48819 for the R failures.

rok · 2026-01-14T23:27:41Z

If I remember correctly in rust we verify directly on decrypted buffer (skipping reserialization).

pitrou · 2026-01-15T08:40:59Z

If I remember correctly in rust we verify directly on decrypted buffer (skipping reserialization).

I'm not surprised that Rust is saner than C++ :-)

pitrou · 2026-01-15T09:20:34Z

@github-actions crossbow submit -g cpp

github-actions · 2026-01-15T09:23:40Z

Revision: e853dee

Submitted crossbow builds: ursacomputing/crossbow @ actions-f90353001e

Task	Status
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-cpp-tutorial
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-debian-experimental-cpp-gcc-15
test-fedora-42-cpp
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-bundled
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-bundled-offline
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-gcc-14
test-ubuntu-24.04-cpp-minimal-with-formats
test-ubuntu-24.04-cpp-thread-sanitizer

pitrou · 2026-01-19T10:05:39Z

@github-actions crossbow submit fuzz

github-actions · 2026-01-19T10:21:20Z

Revision: 1bc9ff5

Submitted crossbow builds: ursacomputing/crossbow @ actions-35c4966c5a

Task	Status
test-build-cpp-fuzz

pitrou · 2026-01-19T11:13:35Z

CI failures are unrelated.

pitrou · 2026-01-19T16:10:27Z

@adamreeve @EnricoMi @wgtmac Are you available to review this?

adamreeve

Looks good to me thanks @pitrou. I don't feel too strongly either way about how to handle the deprecation and method visibility.

adamreeve · 2026-01-19T21:06:17Z

If I remember correctly in rust we verify directly on decrypted buffer (skipping reserialization).

Yes, it looks like we started out copying the C++ approach and then switched to encrypting the metadata buffer directly for similar reasons: apache/arrow-rs#7459 (comment)

wgtmac · 2026-01-20T06:16:14Z

cpp/src/parquet/metadata.cc

+  bool VerifySignature(std::span<const uint8_t> serialized_metadata,
+                       std::span<const uint8_t> signature) {


cpp/src/parquet/metadata.cc

wgtmac · 2026-01-20T06:20:14Z

cpp/src/parquet/file_reader.cc

    auto file_decryptor = std::make_shared<InternalFileDecryptor>(
        file_decryption_properties, file_aad, algo.algorithm,
        file_metadata_->footer_signing_key_metadata(), properties_.memory_pool());
-    // set the InternalFileDecryptor in the metadata as well, as it's used


If we make verify signature as a static function, we can remove set_file_decryptor as well?

No, because the decryptor is passed down to row group and column metadata, and is necessary to decrypt encrypted column metadata.

EnricoMi · 2026-01-20T09:52:53Z

cpp/src/parquet/metadata.cc

+    std::shared_ptr<Buffer> encrypted_buffer =
+        AllocateBuffer(file_decryptor_->pool(),
+                       aes_encryptor->CiphertextLength(serialized_metadata.size()));
+    int32_t encrypted_len = aes_encryptor->SignedFooterEncrypt(


So we produce the signature by encrypting the serialized metadata? Can't we compute the signature from the encrypted buffer? Or is the signature a property of the encryption process?

AFAIU, the signature is part of the AEAD process.

To quote the spec about plaintext footers:

The footer signing is done by encrypting the serialized FileMetaData structure with the AES GCM algorithm - using a footer signing key, and an AAD constructed according to the instructions of the section 4.4. Only the nonce and GCM tag are stored in the file – as a 28-byte fixed-length array, written right after the footer itself.

So the signature we are talking about is the concatenation of the nonce and the AES-GCM "authentication tag".

How do we obtain the serialized metadata? Does this come from decrypting the encrypted metadata? In that case, we should not need to compute and compare the signature because decryption should fail in the first place if signature does not match.

Ah, this is plaintext footer mode, so we encrypt the footer (metadata) but only store the signature, not the encrypted metadata. To verify integrity, we encrypt again to compare the signatures. There is no encrypted metadata along with the signature, right?

In this context, the footer (i.e. the serialized metadata) is stored in plaintext, not encrypted.

(you're right, when the footer is encrypted, checking the signature is part of the decryption process and we don't need to do it manually)

Oops, looks like we posted our messages at the same time :) So, yes.

…ure verification

pitrou · 2026-01-20T11:48:57Z

I've made the new method static and private, and updated the deprecation message to ask users to contact us if they need the deprecated functionality.

pitrou · 2026-01-20T11:51:45Z

@github-actions crossbow submit -g cpp

github-actions · 2026-01-20T11:54:34Z

Revision: e8fea7e

Submitted crossbow builds: ursacomputing/crossbow @ actions-d1830a6afa

Task	Status
example-cpp-minimal-build-static
example-cpp-minimal-build-static-system-dependency
example-cpp-tutorial
test-build-cpp-fuzz
test-conda-cpp
test-conda-cpp-valgrind
test-debian-12-cpp-amd64
test-debian-12-cpp-i386
test-debian-experimental-cpp-gcc-15
test-fedora-42-cpp
test-ubuntu-22.04-cpp
test-ubuntu-22.04-cpp-20
test-ubuntu-22.04-cpp-bundled
test-ubuntu-22.04-cpp-emscripten
test-ubuntu-22.04-cpp-no-threading
test-ubuntu-24.04-cpp
test-ubuntu-24.04-cpp-bundled-offline
test-ubuntu-24.04-cpp-gcc-13-bundled
test-ubuntu-24.04-cpp-gcc-14
test-ubuntu-24.04-cpp-minimal-with-formats
test-ubuntu-24.04-cpp-thread-sanitizer

github-actions bot added Component: Parquet Component: C++ awaiting review Awaiting review labels Jan 14, 2026

pitrou requested review from EnricoMi and adamreeve January 14, 2026 15:24

pitrou marked this pull request as ready for review January 14, 2026 15:28

pitrou requested a review from wgtmac as a code owner January 14, 2026 15:28

pitrou commented Jan 14, 2026

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 14, 2026

pitrou commented Jan 14, 2026

View reviewed changes

pitrou force-pushed the gh48858-plaintext-verify-signature branch from 35d6c51 to e853dee Compare January 15, 2026 08:45

pitrou force-pushed the gh48858-plaintext-verify-signature branch from e853dee to 1bc9ff5 Compare January 19, 2026 10:05

pitrou mentioned this pull request Jan 19, 2026

GH-48900: [C++] Avoid memory blowup with excessive variadic buffer count in IPC #48901

Open

adamreeve approved these changes Jan 19, 2026

View reviewed changes

wgtmac approved these changes Jan 20, 2026

View reviewed changes

EnricoMi reviewed Jan 20, 2026

View reviewed changes

apacheGH-48858: [C++][Parquet] Avoid re-serializing footer for signat…

e8fea7e

…ure verification

pitrou force-pushed the gh48858-plaintext-verify-signature branch from 1bc9ff5 to e8fea7e Compare January 20, 2026 11:48

		PARQUET_DEPRECATED("Deprecated in 24.0.0. Use the two-argument overload instead.")
		bool VerifySignature(const void* signature);

		bool VerifySignature(std::span<const uint8_t> serialized_metadata,
		std::span<const uint8_t> signature) {

GH-48858: [C++][Parquet] Avoid re-serializing footer for signature verification #48859

Are you sure you want to change the base?

GH-48858: [C++][Parquet] Avoid re-serializing footer for signature verification #48859

Conversation

pitrou commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

Are these changes tested?

Are there any user-facing changes?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamreeve Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pitrou commented Jan 14, 2026

Uh oh!

rok commented Jan 14, 2026

Uh oh!

pitrou commented Jan 15, 2026

Uh oh!

pitrou commented Jan 15, 2026

Uh oh!

github-actions bot commented Jan 15, 2026

Uh oh!

pitrou commented Jan 19, 2026

Uh oh!

github-actions bot commented Jan 19, 2026

Uh oh!

pitrou commented Jan 19, 2026

Uh oh!

pitrou commented Jan 19, 2026

Uh oh!

adamreeve left a comment

Choose a reason for hiding this comment

Uh oh!

adamreeve commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pitrou commented Jan 20, 2026

Uh oh!

pitrou commented Jan 20, 2026

Uh oh!

github-actions bot commented Jan 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

pitrou commented Jan 14, 2026 •

edited

Loading

adamreeve Jan 19, 2026 •

edited

Loading