#119 write-repo-file: add --gpg-key option
Opened 11 months ago by ktdreyer. Modified 5 months ago
ktdreyer/compose-utils write-repo-gpg  into  master

file modified
+53 -2
@@ -13,7 +13,7 @@ 

  name = {name}

  baseurl = {baseurl}

  enabled = {enabled}

- gpgcheck = 0

+ gpgcheck = {gpgcheck}

  """

  

  CONTENT_TYPES = {
@@ -39,6 +39,35 @@ 

      return False

  

  

+ def get_variant_gpg_key_id(compose, variant):

+     """

+     If all RPMs are signed by a single key ID, return that key ID.

+ 

+     :returns str: a key ID, like "fd431d51" for Red Hat's GA signing key, or

+                   "f21541eb" for Red Hat's beta signing key.

+     :returns None: if any RPMs are unsigned or if RPMs are signed by more than

+                    one key.

+     """

+     key = None

+     # TODO: research the following:

+     # - Is "variant.uid" the right value to use here?

Yes, it is the right value. However, the variant does not necessarily have to be present in the dict (in cases where there are no RPMs in the variant, the variant can still be included, see Silverblue on Fedora Rawhide as an example).

+     # - How does this code relate to debuginfo files?

The emit function that calls this is itself called separately for binary, debug and source packages. This information could be passed to this function and only packages with matching category might be checked.

+     arches_packages = compose.rpms.rpms[variant.uid]

+     for arch_packages in arches_packages.values():

+         for package in arch_packages.values():

+             for rpm in package.values():

+                 if key is None and rpm['sigkey'] is not None:

+                     key = rpm['sigkey']

+                 if key is not None and rpm['sigkey'] is None:

+                     # log.info('%s is unsigned', pkgfile['path'])

+                     return None

+                 if key is not None and key != rpm['sigkey']:

+                     # log.info('multiple keys found: %s and %s', key,

+                     #          pkgfile['sigkey'])

+                     return None

+     return key

+ 

+ 

  def emit(compose, opts, variant, output, content_type="repository"):

      """Print configuration for a single repository into output."""

      paths = getattr(variant.paths, content_type)
@@ -86,7 +115,22 @@ 

          if not is_url(baseurl):

              baseurl = "file://" + baseurl

  

-     content = REPO.format(name=name, enabled=enabled, baseurl=baseurl)

+     # Are all the packages in this variant signed by the provided GPG key?

+     key_url = None

+     gpgcheck = "0"

+     if opts.gpg_key:

+         key_id = get_variant_gpg_key_id(compose, variant)

+         if key_id == opts.gpg_key[0]:

+             gpgcheck = "1"

+             key_url = opts.gpg_key[1]

+ 

+     content = REPO.format(name=name,

+                           enabled=enabled,

+                           baseurl=baseurl,

+                           gpgcheck=gpgcheck)

+     if key_url:

+         content += "gpgkey = %s\n" % key_url

+ 

      print(content, file=output)

  

  
@@ -156,6 +200,13 @@ 

          type=parse_mapping,

      )

      parser.add_argument(

+         "--gpg-key",

+         metavar="KEY_ID,KEY_URL",

+         help="Enable GPG verification. For example: "

+              "fd431d51,file:///etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release",

+         type=parse_mapping,

+     )

+     parser.add_argument(

          "--name-pattern",

          default="{release_short}-{release_version}-{variant}",

          help="Pattern for repository names.",

@@ -39,6 +39,11 @@ 

  generate URLs. The path prefix will be stripped from path, and replaced with

  URL prefix.

  .TP

+ .BR \-\-gpg\-key = \fIKEY_ID,KEY_URL\fR

+ Enable GPG verification. For example, \fB\-\-gpg\-key

+ fd431d51,file:///etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release\fR will set

+ \fBgpgcheck = 1\fR in the repo file if all RPMs are signed with the \fBfd431d51\fR key.

+ .TP

  .BR \-\-name\-pattern = \fIPATTERN\fR

  Customize name for the repositories. These fragments are available:

  .sp

@@ -0,0 +1,7 @@ 

+ [Fedora-Modular-Bikeshed-Server-rpms]

+ name = Fedora-Modular-Bikeshed-Server-rpms

+ baseurl = file:///composes/Fedora-Modular-Bikeshed-20171004.n.0/compose/Server/$basearch/os

+ enabled = 1

+ gpgcheck = 1

+ gpgkey = file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-modularity

+ 

file modified
+22 -1
@@ -12,7 +12,7 @@ 

  import mock

  from six import StringIO

  

- from .helpers import get_compose_path, get_fixture

+ from .helpers import get_compose, get_compose_path, get_fixture

  

  from compose_utils import repo_file

  
@@ -101,7 +101,28 @@ 

              "/Client/$basearch/os does not start with /foo/bar.\n", mock_err.getvalue()

          )

  

+     def test_gpg_key(self):

+         self.success_run(

+             ["--gpg-key", "a3cc4e62,file:///etc/pki/rpm-gpg/RPM-GPG-KEY-fedora-modularity"],

+             "Fedora-Modular-Bikeshed-20171004.n.0",

+             "signed.repo",

+         )

+ 

      def test_is_url(self):

          self.assertTrue(repo_file.is_url('http://example.com/foobar'))

          self.assertTrue(repo_file.is_url('https://example.com/foobar'))

          self.assertFalse(repo_file.is_url('/path/to/foobar'))

+ 

+     def test_get_variant_gpg_key_id_unsigned(self):

+         compose = get_compose("DP-1.0-20160315.t.0")

+         variants = [v for v in compose.info.variants.variants.values()]

+         for variant in variants:

+             result = repo_file.get_variant_gpg_key_id(compose, variant)

+             self.assertIsNone(result)

+ 

+     def test_get_variant_gpg_key_id_signed(self):

+         compose = get_compose("Fedora-Modular-Bikeshed-20171004.n.0")

+         variants = [v for v in compose.info.variants.variants.values()]

+         variant = variants[0]

+         result = repo_file.get_variant_gpg_key_id(compose, variant)

+         self.assertEqual(result, 'a3cc4e62')

Prior to this change, compose-write-repo-file unconditionally wrote .repo files with gpgcheck = 0.

Add a new --gpg-key option to compose-write-repo-file. When a user specifies a comma-separated key ID and key URL, we sanity-check that every every RPM is signed by this key. For each variant with signed RPMs, we set the gpgcheck = 1 and gpgkey options in the .repo file.

Yes, it is the right value. However, the variant does not necessarily have to be present in the dict (in cases where there are no RPMs in the variant, the variant can still be included, see Silverblue on Fedora Rawhide as an example).

The emit function that calls this is itself called separately for binary, debug and source packages. This information could be passed to this function and only packages with matching category might be checked.

In general the change looks fine to me.

I'm not sure if the situation when the key doesn't match is handled well. in what situation do you want the command to silently not use the given key rather than giving you an error about key mismatch?

You're right. After thinking this over, let's fail with a non-zero exit code if the user specifies --gpg-key and any RPM is not signed with that key. This is security-sensitive code, so I think simplicity is better than "failing open" here.

Side note, I just found out Yum/DNF support multiple gpgkey URLs, https://unix.stackexchange.com/questions/215292/create-yum-repo-with-multiple-keys . I'll need to re-think the command-line argument UX here.

pretty please pagure-ci rebuild

10 months ago

(Note to self, subscription-manager can write Yum repo files with multiple GPG keys, and the syntax for multiple GPG keys looks like this:)

gpgkey = file:///etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-release,file:///etc/pki/rpm-gpg/RPM-GPG-KEY-redhat-beta