Skip to content

Improve post-quantum signature test coverage - #4542

Open
dralley wants to merge 6 commits into
pulp:mainfrom
dralley:pqc-coverage
Open

Improve post-quantum signature test coverage#4542
dralley wants to merge 6 commits into
pulp:mainfrom
dralley:pqc-coverage

Conversation

@dralley

@dralley dralley commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Add tests for ML-DSA signed RPM packages across upload, sync, and signing service paths.

Extend RepositoryBuilder with an option to build real packages from metadata, and build_from_files() for creating repos with existing RPMs.

New tests:

  • Upload with ML-DSA87+Ed448 key
  • Upload pre-built multi-signed fixture (v4 RSA + v6 ML-DSA87)
  • signing_key filter API with v6 fingerprints
  • Sync of ML-DSA signed packages

Assisted-By: Claude Opus 4.6

@dralley
dralley force-pushed the pqc-coverage branch 2 times, most recently from 69f63b1 to 086057b Compare August 5, 2026 19:04
self,
packages: list[MetaPackage],
base_path: Optional[str] = None,
real_packages: bool = False,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR doesn't actually use this option, I just thought it would be cool to add.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, nice addition :)

rpm_path.write_bytes(requests.get(RPM_UNSIGNED_URL).content)

# TODO: generate a new PQC key in-place to make it truly unique, avoid conflicts with other tests
_sign_package(rpm_path, KEY_V6_MLDSA65_ED25519.private_url)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't do this until my PySequoia PR merges and is released.

@dralley
dralley force-pushed the pqc-coverage branch 3 times, most recently from fbf4bfd to c39bed2 Compare August 6, 2026 01:51
@github-actions github-actions Bot added the multi-commit Added when a PR consists of more than one commit label Aug 6, 2026
@dralley
dralley marked this pull request as draft August 6, 2026 02:43
@dralley

dralley commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Note: this is ready for review now, but isn't ready to merge yet, since I'm writing tests that rely on pulp/pulpcore#7946

Also relies on wiktor-k/pysequoia#84

@github-actions github-actions Bot removed the multi-commit Added when a PR consists of more than one commit label Aug 6, 2026
@github-actions github-actions Bot added the multi-commit Added when a PR consists of more than one commit label Aug 18, 2026
@dralley

dralley commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Some more fun notes:

  • It's much faster to generate a package using rpm_rs than it is to download from fixtures.pulpproject.org, about 28 milliseconds vs 220
  • It's much faster to generate an ed25519 or ML-DSA signing key locally than it is to download from fixtures.pulpproject.org / github, single-digit milliseconds vs 100+ms. Not so with RSA keys which are expensive to generate.

@dralley
dralley force-pushed the pqc-coverage branch 2 times, most recently from 0fb5071 to d5fdba0 Compare August 19, 2026 01:40
raise RuntimeError("Server does not have domains enabled.")
kwargs["pulp_domain"] = pulp_domain

return gen_object_with_cleanup(rpm_package_api, **upload_attrs, **kwargs)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason do not keep cleaning up the temporary files here? (as with NamedTemporaryFile did).
It's already uploaded to Pulp's storage and apparently we don't ever need that tmp file again.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably happened just because we gave the temp file a name.

strict=True,
reason="add-signing-service uses GPG internally,"
" which cannot handle ML-DSA / v6 keys",
),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's see if I get it.
pulpcore has merged this PR adding support for pysquoia, but the pqc support was just merged into pysequoia now. So once pulpcore requires the appropriate pysequoia version Pulp will be able to handle PQC signing services. Is that right? If yes, will this PR wait for the chain of events to finish?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially, yes.

self,
packages: list[MetaPackage],
base_path: Optional[str] = None,
real_packages: bool = False,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, nice addition :)

nevra = Nevra(name="upload-signed-test", epoch=0, version="1.0", release="1", arch="noarch")
rpm_path = tmp_path / f"{nevra.to_nvra()}.rpm"
build_rpm(nevra, rpm_path)
_sign_package(rpm_path, KEY_V4_RSA4K.private_url)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Verifies that signing_keys contains both v4 and v6 prefixed fingerprints.
"""
rpm_path = tmp_path / "multi-signed.rpm"
rpm_path.write_bytes(requests.get(RPM_FIXTURE_MULTI_SIGNED).content)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we also multi-sign on the fly? Just curious

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely could but we do probably want to use rpmbuild-built packages in at least some tests and I figured this was an important enough case to do so.

assert synced_package.version == nevra.version

# signing_keys is None after sync (known limitation, see synchronizing.py:1470-1471)
assert synced_package.signing_keys is None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

def pqc_package_signing_service(tmp_path, has_rpmv6_support, pulpcore_bindings):
"""Create a PQC (ML-DSA) package signing service using Sequoia."""
if not has_rpmv6_support:
pytest.skip("rpmsign --rpmv6 not available")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a legit environment we expect this to not be available? If there isn't, I would prefer this to be an assert than a skip.
I assume the rpm v6 is provided in centos stream 10, which is well established now, right? At least on the branches where we are need PQC support.

@dralley dralley Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RHEL 9 / CS9

@dralley
dralley force-pushed the pqc-coverage branch 2 times, most recently from 2d2d92e to cdbf164 Compare August 31, 2026 16:33
Add tests for ML-DSA signed RPM packages across upload, sync, and
signing service paths.

Extend RepositoryBuilder with an option to build real packages from
metadata, and build_from_files() for creating repos with existing RPMs.

New tests:
- Upload with ML-DSA87+Ed448 key
- Upload pre-built multi-signed fixture (v4 RSA + v6 ML-DSA87)
- signing_key filter API with v6 fingerprints
- Sync of ML-DSA signed packages (documents signing_keys=None limitation)
- Using a signing service with ML-DSA keys for package signing (xfail)
- Using a signing service with ML-DSA keys for metadata signing (xfail)

Assisted-By: Claude Opus 4.6
The pattern we were using did not consistently raise errors on 404
@dralley
dralley marked this pull request as ready for review September 1, 2026 16:53
@dralley
dralley requested a review from pedro-psb September 1, 2026 16:54
@dralley
dralley force-pushed the pqc-coverage branch 2 times, most recently from b6995c0 to 5659db9 Compare September 1, 2026 17:42
Use rpm-rs to build (and optionally sign) RPMs in the test suite rather
than downloading fixture packages. Local generation is faster and lets
tests use unique per-test packages, so many upload/signing tests no
longer need delete_orphans_pre and can run in parallel.

- build_rpm() gains an optional signer to produce signed packages
- rpm_package_factory/rpm_artifact_factory default to local generation
- drop now-unused fixtures/constants (signed_artifact, LEGACY_SIGNING_KEY,
  RPM_*_URL2, RPM_PACKAGE_FILENAME2)
- downloading key assets is avoided where possible

Assisted-By: Claude Opus 4.8
Fetch each public/private signing-key file once per session instead of
once per test, and parameterize the signer and verifier fixtures over
any FixtureKey via rpm_signer_factory and rpm_verifier_factory.

Assisted-By: Claude Opus 4.8
@dralley
dralley force-pushed the pqc-coverage branch 3 times, most recently from 430db08 to 509e932 Compare September 2, 2026 01:51
Override rpm's gpg signing command (%__gpg_sign_cmd) with a shim that
shells out to `sq`, which works on both old and new rpm. Also fix the
script's JSON output quoting and assert the signing subkey fingerprint
(signatures are issued by the subkey, not the primary key).

Assisted-By: Claude Opus 4.8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

multi-commit Added when a PR consists of more than one commit no-changelog no-issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants