Skip to content

buildSupport: add helpers for verifying code signatures #43233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xeji wants to merge 5 commits into from
Closed

buildSupport: add helpers for verifying code signatures #43233

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2738727
build-support: add fetchpgpkey
xeji Jul 7, 2018
317ece1
build-support/setup-hooks: add verifySignatureHook
xeji Jul 8, 2018
376eeee
samba4: verify source tarball signature
xeji Jul 8, 2018
403e8a1
tor-browser-bundle-bin: verify tarball signatures
xeji Jul 8, 2018
8f961de
1password: check binary code signature during build
xeji Jun 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion pkgs/applications/misc/1password/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{ stdenv, fetchzip }:
{ stdenv, fetchzip, fetchpgpkey, verifySignatureHook }:

stdenv.mkDerivation rec {
name = "1password-${version}";
Expand All @@ -24,6 +24,19 @@ stdenv.mkDerivation rec {
}
else throw "Architecture not supported";

nativeBuildInputs = [ verifySignatureHook ];

signaturePublicKey = fetchpgpkey {
url = https://keybase.io/1password/pgp_keys.asc;
Copy link
Member

@Mic92 Mic92 Dec 14, 2018

Choose a reason for hiding this comment

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

What if they update their key?

Copy link
Member

@Mic92 Mic92 Dec 14, 2018

Choose a reason for hiding this comment

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

Can we fetch their key from a key-server using the current keyid instead?

Copy link
Member

@Ekleog Ekleog May 12, 2019

Choose a reason for hiding this comment

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

Keyservers are just one of many ways of fetching OpenPGP keys, I don't think it would make sense to force downloads through them (also, we'd want to fetch using the fingerprint, not the keyid).

Having a fetchFromSks wrapper of fetchpgpkey that fetches from the sks-keyservers network might be a meaningful addition, but I don't think it's required for the first iteration

fingerprint = "3FEF9748469ADBE15DA7CA80AC2D62742012EA22";
sha256 = "1v9gic59a3qim3fcffq77jrswycww4m1rd885lk5xgwr0qnqr019";
};

doCheck = true;
checkPhase = ''
verifySignature op.sig op
'';

installPhase = ''
install -D op $out/bin/op
'';
Expand Down
24 changes: 24 additions & 0 deletions pkgs/applications/networking/browsers/tor-browser-bundle-bin/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{ stdenv
, fetchurl
, fetchpgpkey
, verifySignatureHook
, makeDesktopItem

# Common run-time dependencies
Expand Down Expand Up @@ -119,13 +121,34 @@ let
sha256 = "1s0k82ch7ypjyc5k5rb4skb9ylnp7b9ipvf8gb7pdhb8m4zjk461";
};
};

srcSignatures = {
"x86_64-linux" = fetchurl {
url = "https://dist.torproject.org/torbrowser/${version}/tor-browser-linux64-${version}_${lang}.tar.xz.asc";
sha256 = "008r1k3cpwjnvmyywwr3m3rl9bqmynasbzrrzm4kplaisqfg9wkn";
};

"i686-linux" = fetchurl {
url = "https://dist.torproject.org/torbrowser/${version}/tor-browser-linux32-${version}_${lang}.tar.xz.asc";
sha256 = "000binc825nmapwi255g511yva9mrxlqygj4b1kfk36clmnim5zm";
};
};
in

stdenv.mkDerivation rec {
name = "tor-browser-bundle-bin-${version}";
inherit version;

src = srcs."${stdenv.system}" or (throw "unsupported system: ${stdenv.system}");
srcSignature = srcSignatures."${stdenv.system}" or (throw "unsupported system: ${stdenv.system}");

signaturePublicKey = fetchpgpkey {
url = https://sks-keyservers.net/pks/lookup?op=get&search=0x4E2C6E8793298290;
sha256 = "0lnms0cixpqirphp3wkd6dqfxzm3yhs8d3xsaf1a9rmh839r4xi5";
fingerprint = "EF6E286DDA85EA2A4BA7DE684E2C6E8793298290";
};

nativeBuildInputs = [ verifySignatureHook ];

preferLocalBuild = true;
allowSubstitutes = false;
Expand All @@ -146,6 +169,7 @@ stdenv.mkDerivation rec {
interp=$(< $NIX_CC/nix-support/dynamic-linker)

# Unpack & enter
verifySrcSignature
mkdir -p "$TBB_IN_STORE"
tar xf "${src}" -C "$TBB_IN_STORE" --strip-components=2
pushd "$TBB_IN_STORE"
Expand Down
29 changes: 29 additions & 0 deletions pkgs/build-support/fetchpgpkey/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# This function downloads a PGP public key and verifies its fingerprint
# Because it is based on fetchurl, it will still require a sha256
# in addition to the fingerprint

{ lib, fetchurl, gnupg }:

{
fingerprint
, ... } @ args:

lib.overrideDerivation (fetchurl ({

name = "pubkey-${fingerprint}";

postFetch =
Copy link
Member

@Ekleog Ekleog May 12, 2019

Choose a reason for hiding this comment

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

I think we could reduce issues when a key gets an additional signature (eg. for when fetching from sks-keyservers) by stripping it of all UIDs, signatures, etc. before the sha256 check.

''
# extract fingerprint
fpr=$(cat "$downloadedFile" | gpg --homedir . --import --import-options show-only --with-colons 2>/dev/null | grep -m 1 '^fpr' | cut -d: -f 10)
# verify
if [ "$fpr" == "${fingerprint}" ]; then
echo "key fingerprint $fpr verified"
else
echo "key fingerprint mismatch: got $fpr, expected ${fingerprint}"
exit 1
fi
'';

} // removeAttrs args [ "fingerprint" ] ))
(x: {nativeBuildInputs = x.nativeBuildInputs++ [gnupg];})
45 changes: 45 additions & 0 deletions pkgs/build-support/setup-hooks/verify-signature.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Helper functions for verifying (detached) PGP signatures

# importPublicKey
# Add PGP public key contained in ${publicKey} to the keyring.
# All imported keys will be trusted by verifySig
_importPublicKey() {
if [ -z "$signaturePublicKey" ]; then
echo "error: verifySignatureHook requires signaturePublicKey" >&2
exit 1
fi
gpg -q --import "$signaturePublicKey"
}


# verifySignature SIGFILE DATAFILE UNCOMPRESS
# verify the signature SIGFILE for the file DATAFILE
# if DATAFILE is omitted, it is derived from SIGFILE by dropping the .asc or .sig suffix
# if UNCOMPRESS is set, uncompress DATAFILE before verification
verifySignature() {
if [ -z "$3" ]; then
gpgv --keyring pubring.kbx "$1" "$2" || exit 1
Copy link
Member

@Mic92 Mic92 Dec 14, 2018

Choose a reason for hiding this comment

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

Can you make gpg ignoring expire dates? Otherwise it will become a problem when somebody tries to build an old nixpkgs revision. We could print a warning if a gpg key is expired.

Copy link
Member

@Mic92 Mic92 Dec 14, 2018

Choose a reason for hiding this comment

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

The warning should be good enough for the TOFU case.

else
gunzip -c "$2" | gpgv --keyring pubring.kbx "$1" - || exit 1
fi
}


# verifySrcSignature
# verify the signature $srcSignature for source file $src
verifySrcSignature() {
_importPublicKey
[ -z "$srcSignature" ] && return
verifySignature "$srcSignature" "$src" "$signatureUncompressed"
}


# setup

# create temporary gpg homedir
export GNUPGHOME=$(readlink -f .gnupgtmp)
rm -rf $GNUPGHOME # make sure it's a fresh empty dir
mkdir -p -m 700 $GNUPGHOME

# automatically check the signature before unpack if srcSignature is set
preUnpackHooks+=(verifySrcSignature)
17 changes: 17 additions & 0 deletions pkgs/servers/samba/4.x.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{ lib, stdenv, fetchurl, python, pkgconfig, perl, libxslt, docbook_xsl
, fetchpatch
, fetchpgpkey
, verifySignatureHook
, docbook_xml_dtd_42, docbook_xml_dtd_45, readline, talloc
, popt, iniparser, libbsd, libarchive, libiconv, gettext
, krb5Full, zlib, openldap, cups, pam, avahi, acl, libaio, fam, libceph, glusterfs
Expand Down Expand Up @@ -29,6 +31,20 @@ stdenv.mkDerivation rec {
sha256 = "0vkxqp3wh7bpn1fd45lznmrpn2ma1fq75yq28vi08rggr07y7v8y";
};

srcSignature = fetchurl {
url = "mirror://samba/pub/samba/stable/${name}.tar.asc";
sha256 = "0wpcbwbs1bj1y0amhn0z29v55f2hhmzc5p8n7sbwg9kaf0hc5mz5";
};
signatureUncompressed = true;

signaturePublicKey = fetchpgpkey {
url = https://download.samba.org/pub/samba/samba-pubkey.asc;
sha256 = "1fndhq0c34va34z137gvsl9gpwjv30b06makfx8cq5vrmgiax1x1";
fingerprint = "52FBC0B86D954B0843324CDC6F33915B6568B7EA";
};

nativeBuildInputs = [ verifySignatureHook ];
Copy link
Member

@matthewbauer matthewbauer Jul 8, 2018

Choose a reason for hiding this comment

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

Does this work? We aren't downloading binaries here so I have no idea where the signature would be coming from. This should be not needed at all for source based builds.

Copy link
Contributor Author

@xeji xeji Jul 8, 2018

Choose a reason for hiding this comment

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

It works by convention: The hook assumes that srcSignature contains the signature file, and signaturePublicKey the corresponding key. If they're present, srcSignature is checked against src. Of course you have to fetchurl the signature file and fetchpgpkey the key for the hook to work.

Copy link
Contributor Author

@xeji xeji Jul 8, 2018

Choose a reason for hiding this comment

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

And yes, it can be used for source base builds if upstream provides a signature for the source tarball. It obviously doesn't work with fetchFromGitHub, but that would be a case for verifying signed git commits.

Copy link
Member

@matthewbauer matthewbauer Jul 8, 2018

Choose a reason for hiding this comment

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

Ok did not realize that on tarball signatures! I wonder if we could put the source-based part of the hook in fetchurl then? Just have an optional attribute that is verified for us. This would also make it possible to support multiple-src derivations.

Copy link
Contributor Author

@xeji xeji Jul 8, 2018

Choose a reason for hiding this comment

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

Sure we can do that. I only decided against modifying fetchurl for now to avoid a rebuild-everything change. It would take several extra attributes: URL and hash of the signature file, and the public key which I would still download separately with fetchpgpgkey as it may be re-used for multiple projects or sources.

Copy link
Member

@matthewbauer matthewbauer Jul 8, 2018

Choose a reason for hiding this comment

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

Actually it shouldn't though! fetchurl is fixed output so you can make any changes you want to it.


outputs = [ "out" "dev" "man" ];

patches =
Expand All @@ -41,6 +57,7 @@ stdenv.mkDerivation rec {
})
];


buildInputs =
[ python pkgconfig perl libxslt docbook_xsl docbook_xml_dtd_42 /*
docbook_xml_dtd_45 */ readline talloc popt iniparser
Expand Down
6 changes: 6 additions & 0 deletions pkgs/top-level/all-packages.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ with pkgs;
{ substitutions = { gnu_config = gnu-config;}; }
../build-support/setup-hooks/update-autotools-gnu-config-scripts.sh;

verifySignatureHook = makeSetupHook
{ name = "verify-signature-hook"; deps = [ gnupg ]; }
../build-support/setup-hooks/verify-signature.sh;

gogUnpackHook = makeSetupHook {
name = "gog-unpack-hook";
deps = [ innoextract file-rename ]; }
Expand Down Expand Up @@ -172,6 +176,8 @@ with pkgs;

fetchpatch = callPackage ../build-support/fetchpatch { };

fetchpgpkey = callPackage ../build-support/fetchpgpkey { };

fetchs3 = callPackage ../build-support/fetchs3 { };

fetchsvn = callPackage ../build-support/fetchsvn {
Expand Down