Add support for Rpm Package signing #3425

pedro-psb · 2024-02-05T22:11:13Z

This is a draft for implementing Rpm Package support in pulp_rpm.

My plan is (not necessarily in this order):

write test for service registration and validation with sample script (using rpm --addsign)
implement the RpmPackageSigningService model
write unit test for RpmTool
write functional tests with sample script (using rpm --addsign)
implement the API options and add new model fields
implement the signing call logic (I need to find the right spots to call it)
add documentation

other:

Fix tests: rpm-tool requires permission to access to /var/lib/rpm.
Currently, my workaround is to change the permission of this folder, but I believe that's not an ideal approach.
Ship empty RPM package (for signing service validation).
Investigate why the functional test fails on on azure and s3.
Don't build an empty rpm on each validate call.
Decouple key used for signing from a specific signing service, so the same signing service can use different keys.

closes #2986

ipanova · 2024-02-07T17:04:39Z

#closes is wrong ;)

pedro-psb · 2024-02-07T18:22:10Z

Ops

pedro-psb · 2024-03-19T17:45:46Z

pulp_rpm/tests/functional/api/test_package_signing.py

+    }
+    upload_task_href = rpm_package_api.create(**upload_attrs).task
+    package_href = monitor_task(upload_task_href).created_resources[2]
+    package_loc_href = rpm_package_api.read(package_href).location_href


@dralley I'm using rpm_package_api.create direcly here, and it seems to be "leaking" throughout runs (I can see the number of artifacts increasing at each run).

I'm not sure what is the recommended way to keep runs as isolated as possible. Is it using gen_object_with_cleanup? I've tried that but then I receive an empty package.

I could use feedback about the general use of fixtures here as well.

gen_object_with_cleanup is generally used with deletable resources like remotes, repositories, etc. Content isn't considered deletable, it goes through the orphan cleanup process which is indirect. Generally a test that needs completely fresh state (that is, deleting any previous content / artifacts and then re-creating them) will call orphan cleanup first and otherwise we'll just let it be.

It's perfectly OK to let the content / artifacts accumulate unless you need the test to create a new content / artifact during the test run which cannot pre-exist. Running orphan cleanup all the time unnecessarily would slow down the tests quite a bit.

Running orphan cleanup all the time unnecessarily would slow down the tests quite a bit.

Yeah, true. Thanks for the clarification

bersace · 2024-04-12T12:25:36Z

Just a question. Why do you need a new rpm package signing service ? Why core signing service can't do the job ?

pedro-psb · 2024-04-12T12:44:03Z

Just a question. Why do you need a new rpm package signing service ? Why core signing service can't do the job ?

Our signing service models relies on the implementation of a validation method, which assures a registered signing script can really perform a specific type of signing on a file. Currently, the only pulpcore signing service implementation is for detached signatures, so we need another that knows how to validate an rpm package signature, which is "attached" and requires using the rpm cli tool.

https://github.com/pulp/pulpcore/blob/9192c2bf0ccb0e0a2df595fd3efdd0980c80ff34/pulpcore/app/models/content.py#L884

bersace · 2024-04-12T14:03:44Z

@pedro-psb ok, got it. It's like pulp_deb:AptReleaseSigningService, you use it with --class ?

pedro-psb · 2024-04-12T16:08:01Z

Yes, in the pulpcore-manager command you can specify what type of signing service you are registering through the --class option. If ommited, it will use the detached one I've linked.

dralley · 2024-05-14T17:51:15Z

pulp_rpm/app/tasks/signing.py

+    package_signing_service = RpmPackageSigningService.objects.get(pk=signing_service_pk)
+    uploaded_package = PulpTemporaryFile.objects.get(pk=temporary_file_pk)
+    with NamedTemporaryFile(mode="wb", dir=".", delete=False) as final_package:
+        print("*" * 100)


debug prints

dralley · 2024-05-14T17:56:43Z

pulp_rpm/app/viewsets/package.py

+            item
+            for item in (serializer.validated_data.get(key) for key in ("upload", "repository"))
+            if item
+        ]


[serializer.validated_data.get("upload"), serializer.validated_data.get("repository")] might be simpler

dralley · 2024-05-14T17:58:50Z

pulp_rpm/tests/functional/api/test_package_signing.py

+        sign_package=True,
+    )
+    package_a_href = monitor_task(upload_response.task)
+    # import epdb;epdb.serve(port=12345)


dralley · 2024-05-14T18:01:53Z

pulp_rpm/tests/unit/test_rpm_tool.py

+    return file
+
+
+def test_get_empty_rpm_is_valid(tmp_path, monkeypatch):


Probably not necessary - either the empty rpm is there or it's not

dralley · 2024-05-16T15:12:17Z

We should document the limitations and give the feature a tech preview label

ipanova · 2024-05-27T15:14:59Z

CHANGES/2986.feature

@@ -0,0 +1 @@
+Added supported for signing RPM Packages on upload.


Consider adding a sentence that this is in tech preview

ipanova · 2024-05-27T15:19:11Z

pulp_rpm/app/models/content.py

@@ -0,0 +1,72 @@
+import tempfile


hmm, maybe having this in modes/repository.py or models/signing_service.py will be better?

I've followed pulpcore, which defines the base SigningService under content. But I agree, models/signing_service.py feels better (and its what pulp_deb does also).

ipanova · 2024-05-27T15:38:34Z

staging_docs/admin/guides/add-signing-services.md

+
+1. Create a signing script capable of signing an RPM Package.
+    - The script receives a file path as its first argument.
+    - The script should return a json-formatted. No signature is required, since its embedded.


...a json-formatted output.

ipanova · 2024-05-27T15:39:44Z

staging_docs/admin/guides/add-signing-services.md

+1. Register it with `pulpcore-manager add-signing-service`.
+    - The `--class` should be `rpm:RpmPackageSigningService`.
+    - The key provided here serves only for validating the script.
+      The signing fingerprint are provided dynamically, as [on upload signing](site:pulp_rpm/docs/user/guides/06-sign-packages/#on-upload).


ipanova · 2024-05-27T15:41:17Z

staging_docs/user/guides/06-sign-packages.md

+### Example
+
+```bash
+# Create a Repository w/ required porams


s/porams/params

ipanova · 2024-05-27T15:47:30Z

staging_docs/user/guides/06-sign-packages.md

+pulp rpm content upload \
+  --repository ${REPOSITORY} \
+  --file ${FILE} \
+  --sign-package True


Do I read this correctly that one need to specify on upload per package whether to sign it or not? Meaning that in repo there could be mixture of signed/unsigned content?
Have we considered to automatically sign on upload if the package signing service was defined on the repo?In the similar fashion as we do with metadata signing service?

Yes, you understand correctly.
I remember we have discussed the possibility of doing something similar to this, I'm not sure why I didn't commit to this design.

I'll do like that, really looks more consistent with the usecase we want to support.

pedro-psb · 2024-06-10T16:30:04Z

@ipanova The last commit modifies the way signing is triggered. Now if there is an associated repository and it contains the package_signing_service field, the upload process will try to sign the uploaded package.

I will rebase that commit after approval.

ipanova · 2024-06-11T15:36:51Z

This change looks good to me, thanks for adding it!

What this does: - Create RpmPackageSigningService - Create RpmTool utility - Add fields to Repository model - Add branch on Package upload to sign the package with the associated SigningService and fingerprint. - Add docs - Use presence of SigningService at Repo as signing trigger Closes pulp#2986

pedro-psb · 2024-06-11T18:34:13Z

Rebased

dralley · 2024-06-11T23:53:11Z

@pedro-psb You may merge :)

I'm not sure if it will let you until we get the CI issues fixed though. That's being worked on

github-actions bot added the multi-commit Added when a PR consists of more than one commit label Feb 6, 2024

pedro-psb force-pushed the add-rpm-sign-support branch 5 times, most recently from 5807490 to d3f921c Compare March 4, 2024 16:14

pedro-psb force-pushed the add-rpm-sign-support branch 4 times, most recently from 9cdd600 to 9299ed4 Compare March 14, 2024 20:34

pedro-psb commented Mar 19, 2024

View reviewed changes

pedro-psb force-pushed the add-rpm-sign-support branch 6 times, most recently from ef49b8b to d833e55 Compare April 25, 2024 20:04

pedro-psb force-pushed the add-rpm-sign-support branch from fe2849f to 220629e Compare May 6, 2024 21:43

dralley reviewed May 14, 2024

View reviewed changes

pedro-psb force-pushed the add-rpm-sign-support branch from 220629e to 0956ff6 Compare May 15, 2024 21:58

pedro-psb force-pushed the add-rpm-sign-support branch from 0956ff6 to 9de4d87 Compare May 22, 2024 21:37

github-actions bot removed the multi-commit Added when a PR consists of more than one commit label May 22, 2024

pedro-psb force-pushed the add-rpm-sign-support branch 3 times, most recently from a1b4c25 to af6855c Compare May 22, 2024 21:46

pedro-psb marked this pull request as ready for review May 22, 2024 21:47

pedro-psb force-pushed the add-rpm-sign-support branch from af6855c to 0b3505e Compare May 23, 2024 13:35

ipanova reviewed May 27, 2024

View reviewed changes

github-actions bot added the multi-commit Added when a PR consists of more than one commit label May 28, 2024

pedro-psb force-pushed the add-rpm-sign-support branch 2 times, most recently from 2492b1c to 243c56f Compare May 29, 2024 16:09

ipanova approved these changes Jun 11, 2024

View reviewed changes

pedro-psb force-pushed the add-rpm-sign-support branch from 243c56f to 0aa467b Compare June 11, 2024 18:33

github-actions bot removed the multi-commit Added when a PR consists of more than one commit label Jun 11, 2024

dralley approved these changes Jun 11, 2024

View reviewed changes

pedro-psb merged commit cab0c83 into pulp:main Jun 13, 2024
16 checks passed

pedro-psb deleted the add-rpm-sign-support branch June 13, 2024 14:11

pedro-psb mentioned this pull request Jun 13, 2024

Support for signing RPM Packages on upload (previously: Native obs-signd support for signing PULP hosted content) #2986

Closed

pedro-psb mentioned this pull request Jun 24, 2024

Optimized on-upload package signing #3638

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Rpm Package signing #3425

Add support for Rpm Package signing #3425

pedro-psb commented Feb 5, 2024 •

edited

Loading

ipanova commented Feb 7, 2024

pedro-psb commented Feb 7, 2024

pedro-psb Mar 19, 2024

dralley Mar 19, 2024 •

edited

Loading

pedro-psb Mar 19, 2024

bersace commented Apr 12, 2024

pedro-psb commented Apr 12, 2024 •

edited

Loading

bersace commented Apr 12, 2024

pedro-psb commented Apr 12, 2024

dralley May 14, 2024

dralley May 14, 2024

dralley May 14, 2024

dralley May 14, 2024

dralley commented May 16, 2024

ipanova May 27, 2024

ipanova May 27, 2024

pedro-psb May 27, 2024

ipanova May 27, 2024

ipanova May 27, 2024

ipanova May 27, 2024

ipanova May 27, 2024

pedro-psb May 27, 2024

pedro-psb commented Jun 10, 2024

ipanova commented Jun 11, 2024

pedro-psb commented Jun 11, 2024

dralley commented Jun 11, 2024

		return file


		def test_get_empty_rpm_is_valid(tmp_path, monkeypatch):

		@@ -0,0 +1 @@
		Added supported for signing RPM Packages on upload.

Add support for Rpm Package signing #3425

Add support for Rpm Package signing #3425

Conversation

pedro-psb commented Feb 5, 2024 • edited Loading

ipanova commented Feb 7, 2024

pedro-psb commented Feb 7, 2024

Choose a reason for hiding this comment

dralley Mar 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bersace commented Apr 12, 2024

pedro-psb commented Apr 12, 2024 • edited Loading

bersace commented Apr 12, 2024

pedro-psb commented Apr 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley commented May 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedro-psb commented Jun 10, 2024

ipanova commented Jun 11, 2024

pedro-psb commented Jun 11, 2024

dralley commented Jun 11, 2024

pedro-psb commented Feb 5, 2024 •

edited

Loading

dralley Mar 19, 2024 •

edited

Loading

pedro-psb commented Apr 12, 2024 •

edited

Loading