Skip to content

Commit

Permalink
[antlir2][rpms] improved gpg key handling
Browse files Browse the repository at this point in the history
Summary:
I discovered some pretty nasty bugs where dnf's gpg key checking actually
depends on what is installed in the build appliance image.
When trying to minimize the size of the BA, I removed some unused rpms from
`fb-runtime`, then suddenly I was no longer able to build any images that
installed rpms from `fb-runtime`!
Even more confusing, `dnf` sometimes does not import all the keys used in a
repo - if there are multiple public keys in a single file, dnf will only import
the first one, causing weird package installation failures later on.

This diff moves more GPG key behavior directly into antlir2:
All GPG keys for a repo are now imported.
Unsigned packages are blocked if the repo they belong to has one or more GPG
keys.

Test Plan:
```
❯ buck2 test fbcode//antlir/antlir2/test_images/rpms/sig:
Buck UI: https://www.internalfb.com/buck2/ab7a8eb4-53b3-430a-b471-eab2d8a40ce7
Test UI: https://www.internalfb.com/intern/testinfra/testrun/11540474050704033
Network: Up: 1.1MiB  Down: 0B  (reSessionID-a315b0d4-baef-484b-9f21-b434a172669b)
Jobs completed: 147. Time elapsed: 21.5s.
Cache hits: 0%. Commands: 87 (cached: 0, remote: 0, local: 87)
Tests finished: Pass 14. Fail 0. Fatal 0. Skip 0. Build failure 0
```

Failing images:
```
❯ buck2 test -c antlir2.rpm_sig_broken_images=1 fbcode//antlir/antlir2/test_images/rpms/sig:
Buck UI: https://www.internalfb.com/buck2/1c58eaf5-226a-48b2-9113-dd4cf4608c0e
Test UI: https://www.internalfb.com/intern/testinfra/testrun/7318349583684467

...
2023-10-09T19:55:34.845020Z TRACE compile:rpms: rpm: dnf-driver: GpgError { package: Package { name: "unsigned", epoch: 0, version: "0", release: "0", arch: "noarch" }, error: "RPM is not signed" }
...

...
2023-10-09T19:55:34.901547Z TRACE compile:rpms: rpm: dnf-driver: GpgError { package: Package { name: "signed-with-wrong-key", epoch: 0, version: "0", release: "0", arch: "noarch" }, error: "public key not available" }
...

Failed to build 'fbcode//antlir/antlir2/test_images/rpms/sig:install-unsigned (ovr_config//platform/linux:x86_64-fbcode-platform010-clang15#fdd3fedc5f835a2f)'
Failed to build 'fbcode//antlir/antlir2/test_images/rpms/sig:install-signed-with-wrong-key (ovr_config//platform/linux:x86_64-fbcode-platform010-clang15#fdd3fedc5f835a2f)'
```

Reviewed By: epilatow

Differential Revision: D50094060

fbshipit-source-id: a710ae5f1a8cecd0d0d63acfa0fea641fba5eaf8
  • Loading branch information
vmagro authored and facebook-github-bot committed Oct 10, 2023
1 parent 034cefb commit 9fcec20
Show file tree
Hide file tree
Showing 6 changed files with 392 additions and 19 deletions.
106 changes: 88 additions & 18 deletions antlir/antlir2/dnf/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,21 @@
# /usr/bin/dnf itself uses /usr/libexec/platform-python, so by using that we can
# ensure that we're using the same python that dnf itself is using

import argparse
import base64
import importlib.util
import json
import os
import subprocess
import sys
import threading
from collections import defaultdict
from typing import List
from urllib.parse import urlparse

import dnf
import hawkey
import libdnf
import rpm as librpm
from dnf.i18n import ucd

spec = importlib.util.spec_from_file_location(
Expand Down Expand Up @@ -293,6 +297,28 @@ def _try_get_repoid(p):
json.dump({"tx_error": str(e)}, o)


def _parse_gpg_keys(rawkey: str) -> List[bytes]:
rawkey = rawkey.replace("\r\n", "\n").replace("\r", "\n")
keys = []
inblock = False
block = ""
for line in rawkey.splitlines():
if line == "-----BEGIN PGP PUBLIC KEY BLOCK-----":
inblock = True
block = ""
elif inblock:
if line == "-----END PGP PUBLIC KEY BLOCK-----":
inblock = False
keys.append(base64.b64decode(block))
continue
if line.startswith("Version:"):
continue
if line.startswith("="):
continue
block += line.strip()
return keys


def driver(spec) -> None:
out = LockedOutput(sys.stdout)
mode = spec["mode"]
Expand Down Expand Up @@ -344,7 +370,32 @@ def driver(spec) -> None:

# Check the GPG signatures for all the to-be-installed packages before doing
# the transaction
gpg_errors = {}
gpg_errors = defaultdict(list)

# Import all the GPG keys for repos that we're installing packages from
import_keys = defaultdict(list)
for pkg in base.transaction.install_set:
# @commandline repo (local files) does not have a config and so an
# attempt to access 'pkg.repo' raises a KeyError
if pkg.reponame == hawkey.CMDLINE_REPO_NAME:
continue
# Import all the GPG keys for this repo
for keyfile in pkg.repo.gpgkey:
import_keys[keyfile].append(pkg)
for keyfile, pkgs in import_keys.items():
uri = urlparse(keyfile)
keyfile = os.path.abspath(os.path.join(uri.netloc, uri.path))
with open(keyfile, "r") as f:
keytext = f.read()
keys = _parse_gpg_keys(keytext)
for key in keys:
ret = base._ts.pgpImportPubkey(key)
if ret != 0:
for pkg in pkgs:
gpg_errors[pkg].append(
f"failed to import pubkey ({keyfile}): {ret}"
)

for pkg in base.transaction.install_set:
# If the package comes from a repo without a GPG key, don't bother
# checking its signature. If the repo is @commandline (aka, a local
Expand All @@ -353,26 +404,45 @@ def driver(spec) -> None:
if pkg.reponame == hawkey.CMDLINE_REPO_NAME or not pkg.repo.gpgkey:
continue

# Always make sure that the key is imported
# reading the header will cause rpm to do a gpg check
try:
base.package_import_key(pkg)
except Exception:
pass

code, error = base.package_signature_check(pkg)
if code == 0:
continue # gpg check is ok!
else:
gpg_errors[pkg] = error
with open(pkg.localPkg(), "rb") as f:
base._ts.hdrFromFdno(f.fileno())
except librpm.error as e:
msg = str(e)
if "key" in msg:
gpg_errors[pkg].append(msg)
else:
raise e

# If the rpm is unsigned but there are gpg keys for the repo, block the installation
if pkg.repo.gpgkey:
stdout = subprocess.run(
[
"rpmkeys",
"--checksig",
"--verbose",
"--define=_pkgverify_level signature",
pkg.localPkg(),
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
encoding="utf8",
universal_newlines=True,
check=False,
).stdout.lower()
if ("key id" not in stdout) or ("signature" not in stdout):
gpg_errors[pkg].append("RPM is not signed")

if gpg_errors:
with out as out:
for pkg, error in gpg_errors.items():
json.dump(
{"gpg_error": {"package": package_struct(pkg), "error": error}},
out,
)
out.write("\n")
for pkg, errors in gpg_errors.items():
for error in errors:
json.dump(
{"gpg_error": {"package": package_struct(pkg), "error": error}},
out,
)
out.write("\n")
sys.exit(1)

# setting base.args will record a comment in the history db
Expand Down
59 changes: 58 additions & 1 deletion antlir/antlir2/test_images/rpms/sig/BUCK
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ rpm(
repo(
name = "repo",
compress = "none",
gpg_keys = ["key.pub"],
gpg_keys = [
"key.pub",
"unused-key.pub",
],
rpms = [
":unsigned",
":signed",
Expand Down Expand Up @@ -137,3 +140,57 @@ image_rust_test(
crate_root = "test_signatures.rs",
layer = ":test-signatures-layer",
)

# This layer always builds because it's a nicely behaved rpm signed with the
# correct key for this repo.
image.layer(
name = "install-signed",
dnf_available_repos = [":repo"],
features = [
feature.rpms_install(rpms = ["signed"]),
],
flavor = "//antlir/antlir2/test_images:test-image-flavor",
)

image.layer(
name = "test-install-signed-imported-keys-layer",
features = [
feature.layer_mount(
mkdir = True,
mountpoint = "/layer",
source = ":install-signed",
),
],
flavor = ":install-signed[flavor]",
parent_layer = ":install-signed[build_appliance]",
)

image_rust_test(
name = "test-install-signed-imported-keys",
srcs = ["test_install_signed_imported_keys.rs"],
crate_root = "test_install_signed_imported_keys.rs",
layer = ":test-install-signed-imported-keys-layer",
)

# TODO(T166070409) These images correctly fail to build, but until that task is
# implemented, we can't write a proper test for it so just have to test manually
# by passing `-c antlir2.rpm_sig_broken_images=1` and verifying that the build
# fails.
[
image.layer(
name = "install-signed-with-wrong-key",
dnf_available_repos = [":repo"],
features = [
feature.rpms_install(rpms = ["signed-with-wrong-key"]),
],
flavor = "//antlir/antlir2/test_images:test-image-flavor",
),
image.layer(
name = "install-unsigned",
dnf_available_repos = [":repo"],
features = [
feature.rpms_install(rpms = ["unsigned"]),
],
flavor = "//antlir/antlir2/test_images:test-image-flavor",
),
] if read_config("antlir2", "rpm_sig_broken_images", False) else None
56 changes: 56 additions & 0 deletions antlir/antlir2/test_images/rpms/sig/key.pub
Original file line number Diff line number Diff line change
@@ -1,3 +1,59 @@
# This is an unused key, the real key we sign with is located below but I want
# to make sure that multiple keys are parsed from pubkey files like this

-----BEGIN PGP PUBLIC KEY BLOCK-----

mQINBGUkY4oBEAD4/lKFYSas9NMas59rABSmYjNbteEpwTDaQq25zjoBUTwTxsnv
UcKZtu+I3ohrVIMUar1GSGhYb5Xzavg5Ez8BCi81cmMNRr1UlbkxR/mReibuLuyz
F0FloyJ4MiXtR3QUdkQ0GsLL4GgKacCp/JloBfG1HGCm477CleMxz/Mvb0Vm8Eh+
XkgQkdQgG+FszIva7c4lnAyu/okOX9vpqs/xa8mTnqiP1vP5G9oSwnykBAIe/dTD
ArNdo2o5wicSXj5K+gz86Yrz/m4cQng+3uD4h+X6Ixipdo/nNWH5Seh9Z813HtL+
T02At4A8sEQHgWp6jBz3vv60RGfkfeQWBxQg7u18xQ0zR7HIR1nz5TcfmshS3dxN
j4/2HiMVcoA31upAL6F71Dbb+rYPwIebhkFnSxLSDlJU8hoNJ2ol1yBRoRAwcQXy
ClcDk96FYmq5wzSs3a7Eupu6nwH9YSi7WbPx1EuQePDbs/IvBF1SrtUtdG/mM4te
0en0nbjKH0Swti2eWbuUzpsDuCYsfaXHtg6Vi9etmdw/wAmUB9ITPyWgEuOdxwrj
T/tEbKW3OEwsqF+cUvY0ZRKlU00iZCIl9Kvw4H3Ol2zmrTcr3I2jhHPQCuOrxdW7
uWXpE/0l8p6uqGfZHLKkEfuREQZlio84oV0j22WNhj/WUWdsIgcEFbjo0QARAQAB
tBVhbnRsaXIyLXVudXNlZC1jb25jYXSJAlIEEwEIADwWIQTin7iJ8CLEghpFHQdA
frms77AxCAUCZSRjigIbAwULCQgHAgMiAgEGFQoJCAsCBBYCAwECHgcCF4AACgkQ
QH65rO+wMQjbRA//cq1An5AgBc8kaMh1rlrBxURpyvh0Dw2BfSjcI7LL6OtWN3Mj
LuEAuRZYNTQEciwgeOnwFtDRgZp5rGEmaudNJz7MS4dpCrqaq8zjhXKrdoIiDDPm
i+ksGiNlKzN6UkiyZxwIwct4ysOpvPC2WLiDMy0g2nLSCI29e3hw5TgDgjgaUYbP
O7LgeEvvFmZJV2rJ4rvG7bei9iI7pnH5Mup7evJrId90uBJk8TGgHFqBTh4ZQiY4
hJS9REqOKz61YS6hGaSws+NE5RqFdesIvoIWDmegwsitgDKk6rJdjU3s/7RDWXTJ
ZwjmrhYXJ4Htv/vQldjnmbgHlEB/G24ZMNrEXD+4dYJfTeeflAr2qF3Ld+v4Irj0
+chtV4Di4R61OUWSc0IicVaG/ji3tGLNU6IzlAldUttIlNbZEwqiSkiX3V9KZmAL
QMF1E3rG43EFD/u4/X2wIsjNXlzoSa5CI7oaBY5CLDo3ih9i9Gu/75USNJqzvmng
Ryd1/iN9P3xLoaiwAUQ0gv72SCDab2uAzycqbOdjOFWW9ePF75CVziiMH7kGEbz0
tgyEgZw+Q648lZBJRY74Nl6duoQZ0hQoo7cxobAJ4zF75IOaWW/N5guUxBdkz14L
7yE0AGpmedmmyM0pOJ5GLS1HlHiyL+9Bj+wq6NqJSmhtGiYKAQ/atByJ2u25Ag0E
ZSRjigEQAPHm1SzxXb/Vao7TgSTsN8QWT8fSwqCY8MuH4yLZgqfWHn8vQtGd3Oba
nMeJosWjXfaAzyTOMDeyu4dpak3sbirzOoEGrwU3me7DhAMKlGSjslA9VxkqpQlK
yOcHIvcYlV64sI0q/kJKaVFwQuNrCd7wxakj3G3MpY5+zcb3+MwLuM3REPv8cl5r
8MqAiF364YZoRfo6SBZ9w1bkkMgPv/oam624sZttn1/UUvL2PAIJ3NhWYlvMqdVt
/KMNVTZP4YQ+RQbadPSWdZyYWl6YODMSf/LVlM/O0DXXEGMZ6CzPUDKcgsryX25q
tOo3qoqiralgMKEMtz6OVecjcsJZw/+OWpaidLwxqU1XSUHlk/z0pS3jr96Dcx7R
XJ7QaWBp0DIk7tmqFKwB7MeGFWAkWw1oJbf1iCKKJe+UyArknWHUeB0rCBE6LAQR
URDHDrwVr0kAqZcIwfoDVVamdgaT+4WXx7jeLs5XcNf3IbbLRy3t/VcA9z65FA3+
1ogAagn7D/xsRBGfD8hu3fvBO7XfNPrwJghaPE3DLBe3MBb8KPykl1nYeX1PlE1D
c5uF/+N41DadFC6buPltQF+HKiAh7d+Kn0Ccmxf4T6zgG6fFTeZL4hObM0unvR9q
TwwhV/FlmKQdlpB4TkxRFuBw12AXBiHSItjY7izgGcKszCaeoqLdABEBAAGJAjYE
GAEIACAWIQTin7iJ8CLEghpFHQdAfrms77AxCAUCZSRjigIbDAAKCRBAfrms77Ax
CID0EACoOXqr8DMSBzOlwfGmr4+FqxI7O6JeYzcxnqJm35KF13uLt5rSjFy1tlwP
FkzAgvDOsGEZciDKxkfdF82Cn+OEsIJXZMqjR38V6oy7syOseCzfxN4DSrDb8w42
rTM8vC4dKj2d5uxjZ24ahwvhGqCJKxqjgVkqgP9lMIri36oORh53CVRmYVLO5IbW
gAMLw63cQGZvCTFUuwRgpeFHemqXkHyQkwDVUE2n2Z2pqZt6h7Xbf0sNvPDxx2vT
RjGGxCItacOtJhSWOLmdWJVlim1nH29V8rJXiiOZUwgMlYr1VvSLlQkEG9Xb00Ph
tin9vflRmzHfZtXdillVS5rA/nZoEExyWe1Bi9hspk1acVY9qRwxYXPEJ0PEMmdq
YG0bfYuY/J4ejPunn7sdyRLRsHq93bScNJ7Mzmt/DJFUocEZENSX/DsjaKmoM3U9
p/7YRsVwDsYck0khh6Ou+5A0IvARO4ap6o4rNztd3KNwK7KmOifhVo7oPYtgn1I4
xP2pKFrfC/WstkNCOu5OwOpcpXI1kvnWU070kneyfGNd4vTadm2BpJo6tSCTWeZ3
nfFmCyfOWPDLg3YtxaU1fYFzNeRNmMCuxerXnhdLUoVjrX+x++ravQ5sgUqxUEfp
e8Rj8Xx/HWqoaMlQGyCBuH9bxY0A5uKNksYiNL8Nalq1fungcw==
=qAQa
-----END PGP PUBLIC KEY BLOCK-----


-----BEGIN PGP PUBLIC KEY BLOCK-----

mQINBGUkMZ0BEADaxpin2gjwNVL0Cv+hkYOz5R1WeByXwS5sRSkT/FPmSV7h/gT2
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

use std::collections::HashSet;
use std::process::Command;

#[test]
fn installed_keys() {
let out = Command::new("rpm")
.arg("--root")
.arg("/layer")
.arg("-q")
.arg("gpg-pubkey")
.output()
.expect("failed to run cmd");
let stdout = String::from_utf8(out.stdout).expect("cmd output not utf8");
assert!(!stdout.is_empty());
let keys: HashSet<_> = stdout.lines().map(|l| l.trim()).collect();
assert_eq!(
keys,
HashSet::from([
// key that 'signed' is signed with
"gpg-pubkey-bf8dba69-6524319d",
// 'unused' key that is also set as trusted for the test repo
"gpg-pubkey-22b685ee-652452bf",
// unused key prepended to key.pub but not used to sign any packages
"gpg-pubkey-efb03108-6524638a",
])
);
}
Loading

0 comments on commit 9fcec20

Please sign in to comment.