feat: Sign ancillary files in aggregator #2394

Alenar · 2025-03-27T17:33:51Z

Content

This PR enhance the ancillary archive creation by a adding a signed manifest file in the archive.

The manifest:
- Include the list of the ancillary files included alongside their Sha256 hashes
- The signature sign the aggregation of all files path and their hashes
- Is signed using ED25519 (same cryptographic primitive as era markers and genesis signatures)
Aggregator changes:
- Add two tar appender: AppenderData to append data directly from memory, ChainAppender to chain two appender together.
- CompressedArchiveSnapshotter: compute, sign, and append to archive the ancillary manifest
- Add ancillary_files_signer mandatory configuration parameter (infrastructure and e2e runner were modified accordingly)

Pre-submit checklist

Branch
- Tests are provided (if possible)
- Crates versions are updated (if relevant)
- CHANGELOG file is updated (if relevant)
- Commit sequence broadly makes sense
- Key commits have useful messages
PR
- All check jobs of the CI have succeeded
- Self-reviewed the diff
- Useful pull request description
- Reviewer requested
Documentation
- Update documentation website (if relevant)
- No new TODOs introduced

Issue(s)

Relates to #2362

Copilot

Pull Request Overview

This PR adds support for signing ancillary files in the aggregator by introducing a signed manifest that includes file paths and their SHA‐256 hashes, and updating the snapshotter to incorporate the signature into the resulting archive. Key changes include:

Addition of environment variable definitions in the docker-compose files for the ancillary signer.
Introduction of the SignableManifest entity along with its associated cryptographic types and tests in mithril-common.
Updates to the aggregator’s snapshotter, ancillary signer implementation, dependency injection, and configuration to support manifest signing.

Reviewed Changes

Copilot reviewed 21 out of 23 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
mithril-infra/assets/docker/docker-compose-aggregator-noauth.yaml	Added ancillary signer environment variables (with extraneous commas).
mithril-infra/assets/docker/docker-compose-aggregator-noauth-p2p.yaml	Added ancillary signer environment variables (with extraneous commas).
mithril-infra/assets/docker/docker-compose-aggregator-auth.yaml	Added ancillary signer environment variables (with extraneous commas).
mithril-infra/assets/docker/docker-compose-aggregator-auth-p2p.yaml	Added ancillary signer environment variables (with extraneous commas).
mithril-common/src/test_utils/fake_keys.rs	Introduced a new fake key function for signable manifest signatures and updated tests.
mithril-common/src/entities/signable_manifest.rs	New entity definition for a signable manifest including hash computation and verification.
mithril-common/src/entities/mod.rs	Re-exported the new signable_manifest module.
mithril-common/src/crypto_helper/mod.rs & ed25519_alias.rs	Updated exports and added a manifest module for cryptographic types.
mithril-aggregator/src/services/snapshotter/compressed_archive_snapshotter.rs	Updated to build, sign, and include the ancillary manifest in the archive.
mithril-aggregator/src/services/snapshotter/ancillary_signer/{with_secret_key.rs, interface.rs, mod.rs}	Introduced ancillary signer abstractions and an implementation using a secret key.
mithril-aggregator/src/dependency_injection/builder/protocol/artifacts.rs	Updated dependency injection to create an ancillary signer from configuration.
mithril-aggregator/src/configuration.rs	Added configuration support for ancillary signers along with associated tests.
mithril-aggregator/src/artifact_builder/*/	Updated tests and snapshotter usage to include a mock ancillary signer.

Files not reviewed (2)

mithril-aggregator/config/dev.json: Language not supported
mithril-infra/mithril.aggregator.tf: Language not supported

mithril-infra/assets/docker/docker-compose-aggregator-noauth.yaml

mithril-infra/assets/docker/docker-compose-aggregator-noauth-p2p.yaml

mithril-infra/assets/docker/docker-compose-aggregator-auth.yaml

mithril-infra/assets/docker/docker-compose-aggregator-auth-p2p.yaml

github-actions · 2025-03-27T17:45:25Z

Test Results

3 files ± 0 57 suites ±0 11m 29s ⏱️ -2s
1 812 tests +23 1 812 ✅ +23 0 💤 ±0 0 ❌ ±0
2 218 runs +23 2 218 ✅ +23 0 💤 ±0 0 ❌ ±0

Results for commit a4f99fe. ± Comparison against base commit 78c5f58.

This pull request removes 10 and adds 33 tests. Note that renamed tests count towards both.

mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_dir_all_compute_size
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_entries_compute_size_of_its_paths
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_entries_return_error_when_empty_entries
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_entries_return_error_when_file_or_directory_not_exist
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_entries_should_create_archive_only_for_specified_directories_and_files
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_entries_with_duplicate_files_and_directories
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_file_all_compute_size
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_file_should_append_file_to_tar
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_file_should_return_error_if_file_does_not_exist
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_file_should_return_error_if_input_is_not_a_file

mithril-aggregator ‑ configuration::test::serialized_ancillary_files_signer_config_use_snake_case_for_keys_and_kebab_case_for_type_value
mithril-aggregator ‑ services::snapshotter::ancillary_signer::with_secret_key::tests::computed_signature_signs_manifest_hash
mithril-aggregator ‑ services::snapshotter::compressed_archive_snapshotter::tests::snapshot_ancillary::create_archive_fail_if_manifest_signing_fail
mithril-aggregator ‑ services::snapshotter::compressed_archive_snapshotter::tests::snapshot_ancillary::create_archive_generate_sign_and_include_manifest_file
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_data::append_serializable_json
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_data::appended_entry_have_read_write_permissions_and_time_metadata
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_data::compute_uncompressed_size
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_dir_all::compute_uncompressed_size
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_entries::can_append_duplicate_files_and_directories
mithril-aggregator ‑ tools::file_archiver::appender::tests::appender_entries::compute_uncompressed_size_of_its_paths
…

♻️ This comment has been updated with latest results.

Copilot

Pull Request Overview

This PR enhances the ancillary archive creation by including a signed manifest file in the archive. Key changes include:

Adding two new environment variables to the Docker compose files for configuring the ancillary files signer.
Introducing a new signable manifest module with methods to compute and verify file hashes, along with additional tests.
Updating the aggregator’s snapshotter to build, sign, and append the ancillary manifest, and wiring the ancillary signer into dependency injection and configuration.

Reviewed Changes

Copilot reviewed 21 out of 23 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
docker-compose-aggregator-*.yaml	Added ANCILLARY_FILES_SIGNER environment variables.
mithril-common/src/test_utils/fake_keys.rs	Added a new function to return pre-encoded ED25519 signatures for testing.
mithril-common/src/entities/signable_manifest.rs	New module defining SignableManifest and its helper methods.
mithril-common/src/crypto_helper/ed25519_alias.rs	Integrated manifest-related type aliases into ED25519 module.
mithril-aggregator/src/services/snapshotter/compressed_archive_snapshotter.rs	Updated snapshotter to build and sign the ancillary manifest using a chained appender.
mithril-aggregator/src/services/snapshotter/ancillary_signer/*	Added interface and a secret-key based implementation for manifest signing.
mithril-aggregator/src/dependency_injection/builder/protocol/artifacts.rs	Configured ancillary signer retrieval in dependency injection.
mithril-aggregator/src/configuration.rs	Added ancillary files signer configuration and tests.
artifact builder and tests	Updated tests to inject the ancillary signer (using mocks) where needed.

Files not reviewed (2)

mithril-aggregator/config/dev.json: Language not supported
mithril-infra/mithril.aggregator.tf: Language not supported

Comments suppressed due to low confidence (1)

mithril-common/src/test_utils/fake_keys.rs:154

Review the multi-line hex string literal to ensure that no unintended whitespace is introduced when the string is split across lines. Consider using explicit concatenation or a raw string literal to improve clarity and avoid potential parsing issues.

        "ebc0652ffe864970a2ba538eacf7d088e9840e3db883c96d13eb6c5b4c74cfc6e84932e4640ca9e3b5e3de2dd6\

sfauvel

LGTM

mithril-aggregator/src/dependency_injection/builder/protocol/artifacts.rs

mithril-aggregator/src/tools/file_archiver/appender.rs

mithril-aggregator/src/services/snapshotter/ancillary_signer/interface.rs

internal/mithril-cli-helper/src/serde_deserialization.rs

Copilot

Pull Request Overview

This PR enhances the ancillary archive creation by including a signed manifest file that details the list of ancillary files with their SHA256 hashes and a signature over their aggregation. Key changes include:

New modules and crypto types to support signing of the manifest.
Updates to the snapshotter to build, sign, and chain the ancillary manifest into the archive.
Adjustments to configuration, dependency injection, and tests to integrate the ancillary files signer.

Reviewed Changes

Copilot reviewed 32 out of 33 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
mithril-common/src/entities/mod.rs	Adds signable_manifest module and exports its contents to support manifest creation.
mithril-common/src/crypto_helper/mod.rs	Updates to include manifest types from ed25519_alias.
mithril-common/src/crypto_helper/ed25519_alias.rs	Introduces a new module for manifest-specific cryptographic types and aliases.
mithril-common/Cargo.toml	Bump version to 0.5.18.
mithril-aggregator/src/services/snapshotter/mod.rs	Exposes the ancillary_signer module for manifest signing.
mithril-aggregator/src/services/snapshotter/compressed_archive_snapshotter.rs	Updates snapshotter to build/sign the manifest and chain it to the archive; accepts ancillary_signer dependency.
mithril-aggregator/src/services/snapshotter/ancillary_signer/**	Adds a new signer interface and its implementation using a secret key.
mithril-aggregator/src/dependency_injection/builder/protocol/artifacts.rs	Integration of ancillary_signer in dependency building.
mithril-aggregator/src/configuration.rs	Adds ancillary_files_signer configuration with ED25519 support.
mithril-aggregator/src/artifact_builder/{cardano_database_artifacts/immutable.rs, cardano_database.rs}	Updates tests to include ancillary signer and adjust expected archive size.
mithril-aggregator/Cargo.toml	Bump version to 0.7.27.
internal/mithril-cli-helper/**	Updates in serialization helpers and Cargo.toml to support configuration deserialization.
docs/website/root/manual/develop/nodes/mithril-aggregator.md	Documents the new ancillary_files_signer CLI parameter.
CHANGELOG.md	Lists the new signed manifest feature and separates ancillary archive contents.

Files not reviewed (1)

mithril-aggregator/config/dev.json: Language not supported

internal/mithril-cli-helper/src/serde_deserialization.rs

jpraynaud

LGTM 👍

mithril-aggregator/src/configuration.rs

mithril-infra/assets/docker/docker-compose-aggregator-auth-p2p.yaml

CHANGELOG.md

…n reading configurations

+ use `temp_dir_create!` macro Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

- `AppenderData` allow to add raw bytes from memory directly into a tar archive, with a utility ctor to be built from serializable object into json - `ChainAppender` allow to chain two appender together, allowing to pass them at once to a file archiver - Add a `chain` method on the `TarAppender` trait with a blanket implementation to allow easy chaining without needing to import of the `ChainAppender`. Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

`SignableManifest` is a generic data structure to represent manifest files and use Ed25519 for its signature `AncillaryFilesManifest` is a specialization of the former for ancillary files that implement two capabilities: - verifying that the stored files path and hashes in the manifest match files in a given directory - aggregating the filenames and hashes into a single hash ready to be verified against a signature Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

…ashes Else reading huge files (such as the ledger) would have made the application unresponsive. Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

In order to easily create a manifest from a list of file paths, computing their hashes at the same time. Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

- add `ancillary_snapshot` module in `services/snapshotter` (located here because it will be the snapshotter that generate signed manifests) - define `AncillarySigner` trait - add `AncillarySignerWithSecretKey` that implement the trait using a in memory secret key Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

- rename `ancillary_files_signer` to `ancillary_files_signer_config`

…` version * mithril-cli-helper from `0.0.2` to `0.0.3` * mithril-aggregator from `0.7.26` to `0.7.27` * mithril-common from `0.5.17` to `0.5.18` * mithril-end-to-end from `0.4.75` to `0.4.76` * mithril-infra/assets/infra.version from `0.3.15` to `0.3.16`

Alenar requested review from sfauvel, jpraynaud and Copilot March 27, 2025 17:33

Alenar assigned Alenar and dlachaume Mar 27, 2025

Copilot AI reviewed Mar 27, 2025

View reviewed changes

Alenar force-pushed the ensemble/2362/sign_ancillary branch from 21f496e to d59f4da Compare March 27, 2025 17:37

Alenar force-pushed the ensemble/2362/sign_ancillary branch from d59f4da to 9836223 Compare March 27, 2025 17:49

Alenar requested a review from Copilot March 28, 2025 08:40

Copilot AI reviewed Mar 28, 2025

View reviewed changes

sfauvel approved these changes Mar 28, 2025

View reviewed changes

mithril-aggregator/src/dependency_injection/builder/protocol/artifacts.rs Outdated Show resolved Hide resolved

jpraynaud reviewed Mar 28, 2025

View reviewed changes

mithril-aggregator/src/tools/file_archiver/appender.rs Outdated Show resolved Hide resolved

mithril-aggregator/src/tools/file_archiver/appender.rs Outdated Show resolved Hide resolved

mithril-aggregator/src/services/snapshotter/ancillary_signer/interface.rs Outdated Show resolved Hide resolved

Alenar force-pushed the ensemble/2362/sign_ancillary branch 2 times, most recently from 4c611d4 to 0157384 Compare March 28, 2025 15:23

github-advanced-security bot found potential problems Mar 28, 2025

View reviewed changes

internal/mithril-cli-helper/src/serde_deserialization.rs Fixed Show fixed Hide fixed

internal/mithril-cli-helper/src/serde_deserialization.rs Fixed Show fixed Hide fixed

Alenar force-pushed the ensemble/2362/sign_ancillary branch from 0157384 to 80abe83 Compare March 28, 2025 16:07

Alenar temporarily deployed to testing-preview March 28, 2025 16:15 — with GitHub Actions Inactive

Alenar force-pushed the ensemble/2362/sign_ancillary branch 3 times, most recently from 5876e93 to 59c4b3e Compare March 28, 2025 16:29

Alenar temporarily deployed to testing-preview March 28, 2025 16:37 — with GitHub Actions Inactive

Alenar requested a review from Copilot March 28, 2025 16:46

Copilot AI reviewed Mar 28, 2025

View reviewed changes

internal/mithril-cli-helper/src/serde_deserialization.rs Outdated Show resolved Hide resolved

Alenar force-pushed the ensemble/2362/sign_ancillary branch from 59c4b3e to 156cd04 Compare March 28, 2025 16:50

Alenar temporarily deployed to testing-preview March 28, 2025 16:59 — with GitHub Actions Inactive

jpraynaud approved these changes Mar 31, 2025

View reviewed changes

mithril-aggregator/src/configuration.rs Outdated Show resolved Hide resolved

mithril-infra/assets/docker/docker-compose-aggregator-auth-p2p.yaml Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

Alenar and others added 3 commits March 31, 2025 11:11

feat(cli-helper): add deserialize utility to handle map or string whe…

89298a7

…n reading configurations

test(aggregator): group appender tests per implementation

fc3d8cf

+ use `temp_dir_create!` macro Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

Alenar and others added 9 commits March 31, 2025 11:11

refactor(common): use async streaming while computing manifest file h…

0b137bf

…ashes Else reading huge files (such as the ledger) would have made the application unresponsive. Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

refactor(common): simplify AncillaryFilesManifest errors

5d8ba33

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

feat(common): add from_paths to AncillaryFilesManifest

749373f

In order to easily create a manifest from a list of file paths, computing their hashes at the same time. Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

feat(aggregator): sign ancillary archives with an embedded manifest

8c12638

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

feat(aggregator): add configuration for ancillary files signing

f8025d6

Co-authored-by: Damien Lachaume <dlachaume@users.noreply.github.com>

doc: update parameters list in aggregator doc

e3602d9

fix: adjustments following PR review

b3f864f

- rename `ancillary_files_signer` to `ancillary_files_signer_config`

Alenar force-pushed the ensemble/2362/sign_ancillary branch from 156cd04 to 8882ed5 Compare March 31, 2025 09:11

Alenar temporarily deployed to testing-preview March 31, 2025 09:20 — with GitHub Actions Inactive

Alenar added 2 commits March 31, 2025 11:22

chore: update changelog

9ecadcc

Alenar force-pushed the ensemble/2362/sign_ancillary branch from 8882ed5 to a4f99fe Compare March 31, 2025 09:22

Alenar temporarily deployed to testing-preview March 31, 2025 09:32 — with GitHub Actions Inactive

Alenar merged commit 4d12aae into main Mar 31, 2025
38 checks passed

Alenar deleted the ensemble/2362/sign_ancillary branch March 31, 2025 09:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Sign ancillary files in aggregator #2394

feat: Sign ancillary files in aggregator #2394

Uh oh!

Alenar commented Mar 27, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 27, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

sfauvel left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

jpraynaud left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feat: Sign ancillary files in aggregator #2394

feat: Sign ancillary files in aggregator #2394

Uh oh!

Conversation

Alenar commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Content

Pre-submit checklist

Issue(s)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

sfauvel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

jpraynaud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alenar commented Mar 27, 2025 •

edited

Loading

github-actions bot commented Mar 27, 2025 •

edited

Loading