[feature] Improve package signing plugin integration #18527

danimtb · 2025-06-25T08:01:43Z

Changelog: Feature: Improve package signing plugin integration
Docs: missing

Implemented verify in the cache check-integrity command.
Added scoped output to plugin interface.
Added complete test with new functionality and testing different packages' statuses for signature verification.

…ture/sign_plugin

danimtb · 2025-06-25T08:02:09Z

test/integration/test_pkg_signing.py

        import os

-        def sign(ref, artifacts_folder, signature_folder):
+        def sign(ref, artifacts_folder, signature_folder, **kwargs):


According to the documentation: https://docs.conan.io/2/reference/extensions/package_signing.html

danimtb · 2025-06-25T08:03:47Z

test/integration/test_pkg_signing.py

+            # This is to check if the package was signed or not
+            if isinstance(ref, PkgReference):
+                download_file_path = os.path.join(artifacts_folder, "conan_package.tgz")
+            else:
+                download_file_path = os.path.join(artifacts_folder, "conanfile.py")
+
+            if not os.path.isfile(download_file_path) and not os.path.isfile(signature):
+                output.warning("Could not verify unsigned package")
+                return


This is maybe the last part to improve, as this is the way I found to check if the package was signed or not (it was built but never "uploaded")

Yes, it seems the system needs to support the handling and reporting of packages not signed, probably with some opt-in/opt-out logic to error or not

conan/internal/cache/integrity_check.py

conan/internal/rest/pkg_sign.py

danimtb · 2025-06-26T08:18:33Z

conan/internal/rest/pkg_sign.py

            return

        def _sign(ref, files, folder):
+            output = ConanOutput(scope=f"{ref.repr_notime()}")


This was introduced to improve the output as reported in #18528

test/integration/test_pkg_signing.py

memsharded · 2025-06-29T10:25:58Z

conan/internal/cache/integrity_check.py

-    def __init__(self, cache):
+    def __init__(self, cache, home_folder):
        self._cache = cache
+        self._pkg_signatures_plugin = PkgSignaturesPlugin(cache, home_folder)


Does this check means that when the integrity check is called from conan upload it will try to verify the package signatures before the upload? Is this an intended use case or not?
What should a plugin do if the package that is uploading is already signed? Skip the signature, add its own?

AFAIK, the integrity checker is only used in the cache check-integrity command, so the upload is not using it. In any case, the integrity checker only verifies the signature, it does not sign the packages.

If the upload finds that the package is signed, it will not verify nor resign it at this moment.

Now, with the ability to verify the packages without installing (conan cache check-integrity), I believe the user has the tools to perform the verification whenever they want to verify the package is signed with the appropriate signature.

Not really. The upload does a cache integrity check:

def _upload_pkglist(pkglist, subtitle=lambda _: None): if check_integrity: subtitle("Checking integrity of cache packages") self.conan_api.cache.check_integrity(pkglist)

So this will check signatures, calling verify() before calling sign() and trying to sign them. So this is happening, but maybe it is a reasonable flow?

I'd like to see a test that covers this, checking that the package is already signed before re-signing it, and explaining it a bit on the docs.

memsharded · 2025-07-14T08:17:15Z

conan/internal/cache/integrity_check.py

-    def __init__(self, cache):
+    def __init__(self, cache, home_folder):
        self._cache = cache
+        self._pkg_signatures_plugin = PkgSignaturesPlugin(cache, home_folder)


Not really. The upload does a cache integrity check:

def _upload_pkglist(pkglist, subtitle=lambda _: None): if check_integrity: subtitle("Checking integrity of cache packages") self.conan_api.cache.check_integrity(pkglist)

So this will check signatures, calling verify() before calling sign() and trying to sign them. So this is happening, but maybe it is a reasonable flow?

I'd like to see a test that covers this, checking that the package is already signed before re-signing it, and explaining it a bit on the docs.

conan/internal/cache/integrity_check.py

conan/internal/rest/pkg_sign.py

memsharded

Lets do a sync about the product design aspects:

Concepts of "this package has been signed by XXX with YYYY" checks by users plugin code, need some built-in support
Management of unsigned packages
The return interface of the verify() method

memsharded · 2025-07-16T07:53:37Z

conan/internal/cache/integrity_check.py

+        files = os.listdir(folder)
+        try:
+            self._pkg_signatures_plugin.verify(ref, folder, files)
+        except (ConanException, AssertionError) as e:


And what if the plugin code raises Exception? I think it should capture everything. Or maybe it is the PkgSignaturesPlugin.verify implementation the one doing the try-except to capture it and provide users of the plugin (download, check-integrity) with a boolean True/False return interface?

There is also a use case not implemented yet in the interface: What happens with packages not signed? Is it an error? A warning? It sounds that the package signing system would like at least to report on that:

- 34 packages signed OK - 22 packages not signed

memsharded · 2025-07-16T07:58:09Z

test/integration/test_pkg_signing.py

+            # This is to check if the package was signed or not
+            if isinstance(ref, PkgReference):
+                download_file_path = os.path.join(artifacts_folder, "conan_package.tgz")
+            else:
+                download_file_path = os.path.join(artifacts_folder, "conanfile.py")
+
+            if not os.path.isfile(download_file_path) and not os.path.isfile(signature):
+                output.warning("Could not verify unsigned package")
+                return


We need a better way to check if a package was signed, it is not possible to decide if it was signed based on "signature.asc" existence, as other signing methods will not have it. It seems that this part of the signing needs to be standardized in the PkgSignaturesPlugin built-in part, formalizing the concepts: "this package has been signed", and "this package has been signed by XXX" provider and "with YYYY" method. It seems leaving this to user plugin implementations will be problematic

memsharded · 2025-07-16T08:00:00Z

test/integration/test_pkg_signing.py

+            # This is to check if the package was signed or not
+            if isinstance(ref, PkgReference):
+                download_file_path = os.path.join(artifacts_folder, "conan_package.tgz")
+            else:
+                download_file_path = os.path.join(artifacts_folder, "conanfile.py")
+
+            if not os.path.isfile(download_file_path) and not os.path.isfile(signature):
+                output.warning("Could not verify unsigned package")
+                return


Yes, it seems the system needs to support the handling and reporting of packages not signed, probably with some opt-in/opt-out logic to error or not

czoido · 2025-07-16T09:52:55Z

conan/internal/rest/pkg_sign.py

        self._plugin_verify_function(ref, artifacts_folder=folder, signature_folder=metadata_sign,
-                                     files=files)
+                                     files=files, output=output)
+        output.info("Package signature verification: ok")


I think we can't assume that if we reach this point the verification is ok, maybe the verify function did not raise an error but the verification was not completed when executed.

czoido · 2025-07-16T09:55:51Z

conan/internal/rest/pkg_sign.py

    def verify(self, ref, folder, files):
        if self._plugin_verify_function is None:
            return
+        output = ConanOutput(scope=f"{ref.repr_notime()} [Package-signing plugin]")


Here I would try to use a scope that is the same to the conanfile's one that already outputs the revision in some places?
I get an output like this:

======== Computing dependency graph ======== zlib/1.3.1: Not found in local cache, looking in remotes... zlib/1.3.1: Checking remote: conancenter Connecting to remote 'conancenter' anonymously zlib/1.3.1#b8bc2603263cf7eccbd6e17e66b0ed76 [Package-signing plugin]: Package signature verification: ok zlib/1.3.1: Downloaded recipe revision b8bc2603263cf7eccbd6e17e66b0ed76

I'm also not sure if I would add [Package-signing plugin] to the scope... maybe it's not necessary?

package revision is required to differentiate, escpecially when using the cache -check-integrity command. The package signing header was suggested some comments back #18527 (comment)

I see, yes, in the case of conan cache check-integrity it's important to know the revision and it does not appear anywhere else. Maybe we could start the cache integrity command saying: "verifying ref: ref_with_revision" or something like that? having the full ref in all the lines is a bit noisy and the lines are really long

yes, it is a bit long. We probably have to remove the additional title of the plugin. However, the complete reference is in line with the changes already introduced in the check-integrity command: #18544

czoido · 2025-07-16T10:00:41Z

conan/internal/rest/pkg_sign.py

-                    _sign(pref, pkg_bundle["files"], self._cache.pkg_layout(pref).download_package())
+                    _sign(pref, pkg_bundle["files"], self._cache.pkg_layout(pref).download_package(),
+                          output)
+            output.info("Package signature creation: ok")


For the same reason that I explained for the verify, I think we can't assume a successful sign just for reaching this point.

danimtb · 2025-08-15T10:17:49Z

Closing this PR in favour of the new one: #18785

danimtb added 5 commits June 5, 2025 16:04

add verify plugin to integrity check command

2f37e50

Merge branch 'develop2' of github.com:conan-io/conan into danimtb/fea…

062fc1c

…ture/sign_plugin

test

2118287

add integrity check test and outout

44bf83a

fix

d42f00c

danimtb commented Jun 25, 2025

View reviewed changes

memsharded self-assigned this Jun 25, 2025

fix tests

b3a488d

memsharded reviewed Jun 25, 2025

View reviewed changes

conan/internal/cache/integrity_check.py Outdated Show resolved Hide resolved

conan/internal/rest/pkg_sign.py Outdated Show resolved Hide resolved

danimtb mentioned this pull request Jun 25, 2025

[feature] Improve cache check-integrity output #18528

Closed

1 task

improve load methods

c02e47a

danimtb commented Jun 26, 2025

View reviewed changes

memsharded reviewed Jun 26, 2025

View reviewed changes

test/integration/test_pkg_signing.py Show resolved Hide resolved

danimtb added 2 commits June 26, 2025 13:30

add test

f775323

Merge branch 'develop2' into danimtb/feature/sign_plugin

51102e7

danimtb requested a review from AbrilRBS June 27, 2025 06:01

memsharded reviewed Jun 29, 2025

View reviewed changes

memsharded reviewed Jul 14, 2025

View reviewed changes

danimtb added 3 commits July 15, 2025 12:10

Merge branch 'develop2' into danimtb/feature/sign_plugin

82fc3b2

fix tests

2a801a0

improved output and catch assertion error too

1a42aff

memsharded reviewed Jul 16, 2025

View reviewed changes

czoido reviewed Jul 16, 2025

View reviewed changes

modify upload --check flow

155cdf8

danimtb closed this Aug 15, 2025

[feature] Improve package signing plugin integration #18527

[feature] Improve package signing plugin integration #18527

Uh oh!

Conversation

danimtb commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Uh oh!

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!

Uh oh!

Uh oh!

memsharded left a comment

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!

danimtb commented Aug 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

danimtb commented Jun 25, 2025 •

edited

Loading