OpenPGP key support #5133

mdellweg · 2024-03-14T13:57:38Z

This adds a repository type as a keyring and content types to handle keys, keyids and key signatures.

dralley · 2024-03-15T15:13:32Z

pulpcore/app/openpgp.py

+}
+
+
+def packet_iter(data):


So, this appears to be a de-novo implementation of PGP packet parsing that we'd be on the hook to maintain. Is that 100% necessary? And if so, why?

Is it at all possible to use an existing maintained library such as sequoia-pgp (via pysequoia)?

I think i looked for such things and none really ticked all the boxes (It's been a while though).

About pysequoia (from their pypi representation):
Note: This is a work in progress. The API is not stable!
Note that since pysequoia is implemented largely in Rust, a [Rust toolchain](https://rustup.rs/) is necessary for the installation to succeed.

Note that since pysequoia is implemented largely in Rust, a Rust toolchain is necessary for the installation to succeed.

This part at least would be nothing new for us, several of our deps already check this box. (py)cryptography and json-stream-rs-tokenizer (transitive dep via json-stream) for instance. I think there were maybe one or two others.

Note: This is a work in progress. The API is not stable!

True, but this note is also a few years old, and the API doesn't seem to be particularly unstable. In a tradeoff between occasionally needing to tweak a few lines vs. maintaining a parser implementation for cryptography... my personal preference would be the former.

rpm uses sequoia-pgp (though not via Python) so the quality of the underlying implementation is already trusted so far as Red Hat is concerned. One of the primary authors is the maintainer of the crypto subsystem of the linux kernel.

What i still don't see (from looking at their README) is a way to disassemble the bytestream into openpgp packets which we want to store individually. But i should take a closer look.

I cannot see how pysequoia is providing the raw parsing data we require here.
Having said that, I believe the rfc will not change much giving us not too bad of a maintenance burden. Also let me state that very clear here. We only act on openpgp keys as a content type here. We parse them to find the individual key/signature packets and extract some metadata. We do not perform anything cryptographic here.

Sequoia has it AFAICT, but pysequoia doesn't have it exposed.

@mdellweg Could you enumerate anything I missed here? wiktor-k/pysequoia#32

At this point it's mainly splitting the key into subpackages, because that way we can collect signatures for keys from separate uploads. Also we need the fingerprints. The rest of the data is mainly conveniently there too. And I don't know what else we want to add later. It's really hard to say what we want/need from this new content type in the future.
Quite honestly i'm not ready add another dependency (let alone one that is not pure python) to "just" parse a simple file format that is probably not going to change. If this was about doing actual cryptography I would say the exact opposite.

stale · 2024-06-13T21:42:04Z

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

stale · 2024-09-13T10:54:42Z

This pull request has been marked 'stale' due to lack of recent activity. If there is no further activity, the PR will be closed in another 30 days. Thank you for your contribution!

dralley · 2024-10-02T04:06:37Z

pulpcore/app/models/openpgp.py

+
+    @property
+    def key_expired(self):
+        # TODO handle more cases


Does this need fixing?

I think we can wait until someone complains. Removing the comment.
It is a threestate anyway: Yes, No, Don't know. (True/False/None).

dralley · 2024-10-02T04:07:08Z

pulpcore/app/models/openpgp.py

+    class Meta:
+        default_related_name = "%(app_label)s_%(model_name)s"
+        permissions = [
+            # ("sync_openpgpkeyring", "Can start a sync task"),


dralley · 2024-10-02T04:08:48Z

pulpcore/app/openpgp.py

+        "name": "EdDSA",
+        "format": "om",
+    },
+}


All of these options are in the PGP standard, but that doesn't mean we want to accept all of them. DSA is long-obsolete, as is MD5 below. How do we want to handle that.

Actually, I think we just take and organize whatever we are presented with.
This is just meant to be a keyring for public key data. Even for verifying anything, we call out to gpg. And the gpg installed should decline working with deprecated algorithms.
Even more so, we should be able to report on these formats, so a user can make an informed decision.

dralley · 2024-10-02T04:09:12Z

pulpcore/app/openpgp.py

+        raise NotImplementedError("Version 3 signatures are not implemented.")
+    elif version in [4, 5]:
+        signature_type = data[1]
+        # pubkey_algorithm = PUBKEY_ALGORITHMS.get(data[2])


This is mainly for documentation purpose.

dralley · 2024-10-02T04:09:47Z

pulpcore/app/openpgp.py

+        unhashed_size = (data[6 + hashed_size] << 8) + data[7 + hashed_size]
+        unhashed_data = data[8 + hashed_size : 8 + hashed_size + unhashed_size]
+        canary = data[8 + hashed_size + unhashed_size : 10 + hashed_size + unhashed_size]
+        # signature = data[10 + hashed_size + unhashed_size :]


dralley · 2024-10-02T04:10:01Z

pulpcore/app/openpgp.py

+                raise ValueError("Out of band key signature.")
+            hash_payload = b""
+        else:
+            # TODO:


dralley · 2024-10-02T04:10:31Z

pulpcore/app/openpgp.py

+        n = extract_mpi(data[8:])
+        e = extract_mpi(data[8 + len(n["raw"])])
+        fingerprint = hashlib.md5(n["body"] + e["body"]).hexdigest()
+        # expiration = int.from_bytes(data[5:7], "big")


dralley · 2024-10-02T04:10:39Z

pulpcore/app/openpgp.py

+                    pos += len(oid["raw"])
+                elif item_type == "k":
+                    # KDF
+                    # TODO more analysis


mdellweg · 2024-10-02T11:14:41Z

I removed the comments related to synching, as we don't support that.
Also improved the openpgp "library" module for readability.

dralley

To the extent that I can reasonably review this (not having much subject matter expertise on PGP), it looks fine.

One thing though, it kinda feels like the 3 migrations should be merged into one if possible.

dralley · 2024-10-11T03:24:49Z

@mdellweg Is it possible to merge those migrations together?

This adds a repository type as a keyring and content types to handle keys, keyids and key signatures. fixes pulp#3024

mdellweg · 2024-10-11T10:39:52Z

Is it possible to merge those migrations together?

Should have done that earlier. It's so hard to constantly rebase them.

mdellweg force-pushed the 3024_keyring branch 2 times, most recently from 188b241 to 4dfa211 Compare March 14, 2024 15:28

dralley reviewed Mar 15, 2024

View reviewed changes

pedro-psb mentioned this pull request Mar 15, 2024

Add support for Rpm Package signing pulp/pulp_rpm#3425

Merged

12 tasks

pedro-psb mentioned this pull request Mar 28, 2024

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

Closed

stale bot added the stale label Jun 13, 2024

mdellweg removed the stale label Jun 14, 2024

stale bot added the stale label Sep 13, 2024

mdellweg removed the stale label Sep 13, 2024

mdellweg force-pushed the 3024_keyring branch 2 times, most recently from b7236bb to 0f39a6c Compare September 26, 2024 10:59

mdellweg marked this pull request as ready for review September 27, 2024 09:34

dralley self-requested a review October 2, 2024 04:04

dralley reviewed Oct 2, 2024

View reviewed changes

pulpcore/app/openpgp.py Outdated

raise ValueError("Out of band key signature.")

hash_payload = b""

else:

# TODO:

Copy link

Contributor

dralley Oct 2, 2024

Choose a reason for hiding this comment

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

^

dralley reviewed Oct 2, 2024

View reviewed changes

pulpcore/app/openpgp.py Outdated

pos += len(oid["raw"])

elif item_type == "k":

# KDF

# TODO more analysis

Copy link

Contributor

dralley Oct 2, 2024

Choose a reason for hiding this comment

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

^

mdellweg force-pushed the 3024_keyring branch 2 times, most recently from 3ff4ce2 to 8a89ff3 Compare October 2, 2024 11:12

mdellweg force-pushed the 3024_keyring branch 2 times, most recently from c48fe0c to 05f5ae5 Compare October 9, 2024 11:03

dralley previously approved these changes Oct 11, 2024

View reviewed changes

OpenPGP key support

2d1192c

This adds a repository type as a keyring and content types to handle keys, keyids and key signatures. fixes pulp#3024

mdellweg dismissed dralley’s stale review via 2d1192c October 11, 2024 08:31

mdellweg force-pushed the 3024_keyring branch from 05f5ae5 to 2d1192c Compare October 11, 2024 08:31

dralley approved these changes Oct 11, 2024

View reviewed changes

mdellweg merged commit b6c1ef3 into pulp:main Oct 12, 2024
12 checks passed

mdellweg deleted the 3024_keyring branch October 12, 2024 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OpenPGP key support #5133

OpenPGP key support #5133

mdellweg commented Mar 14, 2024

dralley Mar 15, 2024 •

edited

Loading

mdellweg Mar 15, 2024

dralley Mar 15, 2024 •

edited

Loading

mdellweg Mar 15, 2024

mdellweg Sep 26, 2024

dralley Oct 2, 2024

dralley Oct 3, 2024

mdellweg Oct 7, 2024

stale bot commented Jun 13, 2024

stale bot commented Sep 13, 2024

dralley Oct 2, 2024

dralley Oct 9, 2024

mdellweg Oct 9, 2024 •

edited

Loading

dralley Oct 2, 2024

dralley Oct 2, 2024

mdellweg Oct 2, 2024

dralley Oct 2, 2024

mdellweg Oct 2, 2024

dralley Oct 2, 2024

dralley Oct 2, 2024

dralley Oct 2, 2024

dralley Oct 2, 2024

mdellweg commented Oct 2, 2024

dralley left a comment

dralley commented Oct 11, 2024

mdellweg commented Oct 11, 2024

OpenPGP key support #5133

OpenPGP key support #5133

Conversation

mdellweg commented Mar 14, 2024

dralley Mar 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley Mar 15, 2024 • edited Loading

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

stale bot commented Jun 13, 2024

stale bot commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdellweg Oct 9, 2024 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdellweg commented Oct 2, 2024

dralley left a comment

Choose a reason for hiding this comment

dralley commented Oct 11, 2024

mdellweg commented Oct 11, 2024

dralley Mar 15, 2024 •

edited

Loading

dralley Mar 15, 2024 •

edited

Loading

mdellweg Oct 9, 2024 •

edited

Loading