From 6df32889a51510dff44c776fa312b7ba61ab8edf Mon Sep 17 00:00:00 2001 From: BootRhetoric <110117466+BootRhetoric@users.noreply.github.com> Date: Fri, 20 Oct 2023 21:16:56 +0200 Subject: [PATCH 1/3] Add git commit verification input attributes This implements the git input attributes `verifyCommit`, `keytype`, `publicKey` and `publicKeys` as experimental feature `verified-fetches`. `publicKeys` should be a json string. This representation was chosen because all attributes must be of type bool, int or string so they can be included in flake uris (see definition of fetchers::Attr). --- src/libfetchers/fetchers.cc | 5 ++ src/libfetchers/fetchers.hh | 9 +++ src/libfetchers/git.cc | 104 +++++++++++++++++++++++++-- src/libutil/experimental-features.cc | 11 ++- src/libutil/experimental-features.hh | 1 + 5 files changed, 124 insertions(+), 6 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 92692d23a..895515327 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -360,4 +360,9 @@ std::optional InputScheme::experimentalFeature() const return {}; } +std::string publicKeys_to_string(const std::vector& publicKeys) +{ + return ((nlohmann::json) publicKeys).dump(); +} + } diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 7d768bac1..a056c8939 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -182,4 +182,13 @@ void registerInputScheme(std::shared_ptr && fetcher); nlohmann::json dumpRegisterInputSchemeInfo(); +struct PublicKey +{ + std::string type = "ssh-ed25519"; + std::string key; +}; +NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(PublicKey, type, key) + +std::string publicKeys_to_string(const std::vector&); + } diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index d625fe01e..51e551879 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -143,6 +143,69 @@ struct WorkdirInfo bool hasHead = false; }; +std::vector getPublicKeys(const Attrs & attrs) { + std::vector publicKeys; + if (attrs.contains("publicKeys")) { + nlohmann::json publicKeysJson = nlohmann::json::parse(getStrAttr(attrs, "publicKeys")); + ensureType(publicKeysJson, nlohmann::json::value_t::array); + publicKeys = publicKeysJson.get>(); + } + else { + publicKeys = {}; + } + if (attrs.contains("publicKey")) + publicKeys.push_back(PublicKey{maybeGetStrAttr(attrs, "keytype").value_or("ssh-ed25519"),getStrAttr(attrs, "publicKey")}); + return publicKeys; +} + +void doCommitVerification(const Path repoDir, const Path gitDir, const std::string rev, const std::vector& publicKeys) { + // Create ad-hoc allowedSignersFile and populate it with publicKeys + auto allowedSignersFile = createTempFile().second; + std::string allowedSigners; + for (const PublicKey& k : publicKeys) { + if (k.type != "ssh-dsa" + && k.type != "ssh-ecdsa" + && k.type != "ssh-ecdsa-sk" + && k.type != "ssh-ed25519" + && k.type != "ssh-ed25519-sk" + && k.type != "ssh-rsa") + warn("Unknow keytype: %s\n" + "Please use one of\n" + "- ssh-dsa\n" + "- ssh-ecdsa\n" + "- ssh-ecdsa-sk\n" + "- ssh-ed25519\n" + "- ssh-ed25519-sk\n" + "- ssh-rsa", k.type); + allowedSigners += "* " + k.type + " " + k.key + "\n"; + } + writeFile(allowedSignersFile, allowedSigners); + + // Run verification command + auto [status, output] = runProgram(RunOptions { + .program = "git", + .args = {"-c", "gpg.ssh.allowedSignersFile=" + allowedSignersFile, "-C", repoDir, + "--git-dir", gitDir, "verify-commit", rev}, + .mergeStderrToStdout = true, + }); + + /* Evaluate result through status code and checking if public key fingerprints appear on stderr + * This is neccessary because the git command might also succeed due to the commit being signed by gpg keys + * that are present in the users key agent. */ + std::string re = R"(Good "git" signature for \* with .* key SHA256:[)"; + for (const PublicKey& k : publicKeys){ + // Calculate sha256 fingerprint from public key and escape the regex symbol '+' to match the key literally + auto fingerprint = trim(hashString(htSHA256, base64Decode(k.key)).to_string(nix::HashFormat::Base64, false), "="); + auto escaped_fingerprint = std::regex_replace(fingerprint, std::regex("\\+"), "\\+" ); + re += "(" + escaped_fingerprint + ")"; + } + re += "]"; + if (status == 0 && std::regex_search(output, std::regex(re))) + printTalkative("Commit signature verification on commit %s succeeded", rev); + else + throw Error("Commit signature verification on commit %s failed: \n%s", rev, output); +} + // Returns whether a git workdir is clean and has commits. WorkdirInfo getWorkdirInfo(const Input & input, const Path & workdir) { @@ -272,9 +335,9 @@ struct GitInputScheme : InputScheme attrs.emplace("type", "git"); for (auto & [name, value] : url.query) { - if (name == "rev" || name == "ref") + if (name == "rev" || name == "ref" || name == "keytype" || name == "publicKey" || name == "publicKeys") attrs.emplace(name, value); - else if (name == "shallow" || name == "submodules" || name == "allRefs") + else if (name == "shallow" || name == "submodules" || name == "allRefs" || name == "verifyCommit") attrs.emplace(name, Explicit { value == "1" }); else url2.query.emplace(name, value); @@ -306,14 +369,26 @@ struct GitInputScheme : InputScheme "name", "dirtyRev", "dirtyShortRev", + "verifyCommit", + "keytype", + "publicKey", + "publicKeys", }; } std::optional inputFromAttrs(const Attrs & attrs) const override { + for (auto & [name, _] : attrs) + if (name == "verifyCommit" + || name == "keytype" + || name == "publicKey" + || name == "publicKeys") + experimentalFeatureSettings.require(Xp::VerifiedFetches); + maybeGetBoolAttr(attrs, "shallow"); maybeGetBoolAttr(attrs, "submodules"); maybeGetBoolAttr(attrs, "allRefs"); + maybeGetBoolAttr(attrs, "verifyCommit"); if (auto ref = maybeGetStrAttr(attrs, "ref")) { if (std::regex_search(*ref, badGitRefRegex)) @@ -336,6 +411,15 @@ struct GitInputScheme : InputScheme if (auto ref = input.getRef()) url.query.insert_or_assign("ref", *ref); if (maybeGetBoolAttr(input.attrs, "shallow").value_or(false)) url.query.insert_or_assign("shallow", "1"); + if (maybeGetBoolAttr(input.attrs, "verifyCommit").value_or(false)) + url.query.insert_or_assign("verifyCommit", "1"); + auto publicKeys = getPublicKeys(input.attrs); + if (publicKeys.size() == 1) { + url.query.insert_or_assign("keytype", publicKeys.at(0).type); + url.query.insert_or_assign("publicKey", publicKeys.at(0).key); + } + else if (publicKeys.size() > 1) + url.query.insert_or_assign("publicKeys", publicKeys_to_string(publicKeys)); return url; } @@ -425,6 +509,8 @@ struct GitInputScheme : InputScheme bool shallow = maybeGetBoolAttr(input.attrs, "shallow").value_or(false); bool submodules = maybeGetBoolAttr(input.attrs, "submodules").value_or(false); bool allRefs = maybeGetBoolAttr(input.attrs, "allRefs").value_or(false); + std::vector publicKeys = getPublicKeys(input.attrs); + bool verifyCommit = maybeGetBoolAttr(input.attrs, "verifyCommit").value_or(!publicKeys.empty()); std::string cacheType = "git"; if (shallow) cacheType += "-shallow"; @@ -445,6 +531,8 @@ struct GitInputScheme : InputScheme {"type", cacheType}, {"name", name}, {"rev", input.getRev()->gitRev()}, + {"verifyCommit", verifyCommit}, + {"publicKeys", publicKeys_to_string(publicKeys)}, }); }; @@ -467,12 +555,15 @@ struct GitInputScheme : InputScheme auto [isLocal, actualUrl_] = getActualUrl(input); auto actualUrl = actualUrl_; // work around clang bug - /* If this is a local directory and no ref or revision is given, + /* If this is a local directory, no ref or revision is given and no signature verification is needed, allow fetching directly from a dirty workdir. */ if (!input.getRef() && !input.getRev() && isLocal) { auto workdirInfo = getWorkdirInfo(input, actualUrl); if (!workdirInfo.clean) { - return fetchFromWorkdir(store, input, actualUrl, workdirInfo); + if (verifyCommit) + throw Error("Can't fetch from a dirty workdir with commit signature verification enabled."); + else + return fetchFromWorkdir(store, input, actualUrl, workdirInfo); } } @@ -480,6 +571,8 @@ struct GitInputScheme : InputScheme {"type", cacheType}, {"name", name}, {"url", actualUrl}, + {"verifyCommit", verifyCommit}, + {"publicKeys", publicKeys_to_string(publicKeys)}, }); Path repoDir; @@ -637,6 +730,9 @@ struct GitInputScheme : InputScheme ); } + if (verifyCommit) + doCommitVerification(repoDir, gitDir, input.getRev()->gitRev(), publicKeys); + if (submodules) { Path tmpGitDir = createTempDir(); AutoDelete delTmpGitDir(tmpGitDir, true); diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 74af9aae0..47edca3a5 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -12,7 +12,7 @@ struct ExperimentalFeatureDetails std::string_view description; }; -constexpr std::array xpFeatureDetails = {{ +constexpr std::array xpFeatureDetails = {{ { .tag = Xp::CaDerivations, .name = "ca-derivations", @@ -227,7 +227,14 @@ constexpr std::array xpFeatureDetails = {{ .description = R"( Allow the use of the [impure-env](@docroot@/command-ref/conf-file.md#conf-impure-env) setting. )", - } + }, + { + .tag = Xp::VerifiedFetches, + .name = "verified-fetches", + .description = R"( + Enables verification of git commit signatures through the [`fetchGit`](@docroot@/language/builtins.md#builtins-fetchGit) built-in. + )", + }, }}; static_assert( diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index e02f8353e..f005cc9ee 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -32,6 +32,7 @@ enum struct ExperimentalFeature ParseTomlTimestamps, ReadOnlyLocalStore, ConfigurableImpureEnv, + VerifiedFetches, }; /** From 098f0615c9401414a76e66653fbf4c9dd30d55a7 Mon Sep 17 00:00:00 2001 From: BootRhetoric <110117466+BootRhetoric@users.noreply.github.com> Date: Fri, 20 Oct 2023 21:17:14 +0200 Subject: [PATCH 2/3] fetchGit and flake: add publicKeys list input This adds publicKeys as an optional fetcher input attribute to flakes and builtins.fetchGit to provide a nix interface for the json-encoded `publicKeys` attribute of the git fetcher. Co-authored-by: Valentin Gagarin --- doc/manual/src/release-notes/rl-next.md | 3 +- src/libexpr/flake/flake.cc | 10 ++++- src/libexpr/primops/fetchTree.cc | 56 +++++++++++++++++++++++++ src/libfetchers/git.cc | 14 +++---- 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 3cfb53998..8cd69f8fd 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -16,7 +16,6 @@ - `builtins.fetchTree` is now marked as stable. - - The interface for creating and updating lock files has been overhauled: - [`nix flake lock`](@docroot@/command-ref/new-cli/nix3-flake-lock.md) only creates lock files and adds missing inputs now. @@ -29,3 +28,5 @@ - The flake-specific flags `--recreate-lock-file` and `--update-input` have been removed from all commands operating on installables. They are superceded by `nix flake update`. + +- Commit signature verification for the [`builtins.fetchGit`](@docroot@/language/builtins.md#builtins-fetchGit) is added as the new [`verified-fetches` experimental feature](@docroot@/contributing/experimental-features.md#xp-feature-verified-fetches). diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 70ae7b584..ded132695 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -8,6 +8,7 @@ #include "fetchers.hh" #include "finally.hh" #include "fetch-settings.hh" +#include "value-to-json.hh" namespace nix { @@ -140,8 +141,13 @@ static FlakeInput parseFlakeInput(EvalState & state, attrs.emplace(state.symbols[attr.name], (long unsigned int)attr.value->integer); break; default: - throw TypeError("flake input attribute '%s' is %s while a string, Boolean, or integer is expected", - state.symbols[attr.name], showType(*attr.value)); + if (attr.name == state.symbols.create("publicKeys")) { + experimentalFeatureSettings.require(Xp::VerifiedFetches); + NixStringContext emptyContext = {}; + attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, *attr.value, pos, emptyContext).dump()); + } else + throw TypeError("flake input attribute '%s' is %s while a string, Boolean, or integer is expected", + state.symbols[attr.name], showType(*attr.value)); } #pragma GCC diagnostic pop } diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 767f559be..3717b9022 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -7,6 +7,7 @@ #include "registry.hh" #include "tarball.hh" #include "url.hh" +#include "value-to-json.hh" #include #include @@ -125,6 +126,10 @@ static void fetchTree( attrs.emplace(state.symbols[attr.name], Explicit{attr.value->boolean}); else if (attr.value->type() == nInt) attrs.emplace(state.symbols[attr.name], uint64_t(attr.value->integer)); + else if (state.symbols[attr.name] == "publicKeys") { + experimentalFeatureSettings.require(Xp::VerifiedFetches); + attrs.emplace(state.symbols[attr.name], printValueAsJSON(state, true, *attr.value, pos, context).dump()); + } else state.debugThrowLastTrace(TypeError("fetchTree argument '%s' is %s while a string, Boolean or integer is expected", state.symbols[attr.name], showType(*attr.value))); @@ -427,6 +432,42 @@ static RegisterPrimOp primop_fetchGit({ With this argument being true, it's possible to load a `rev` from *any* `ref` (by default only `rev`s from the specified `ref` are supported). + - `verifyCommit` (default: `true` if `publicKey` or `publicKeys` are provided, otherwise `false`) + + Whether to check `rev` for a signature matching `publicKey` or `publicKeys`. + If `verifyCommit` is enabled, then `fetchGit` cannot use a local repository with uncommitted changes. + Requires the [`verified-fetches` experimental feature](@docroot@/contributing/experimental-features.md#xp-feature-verified-fetches). + + - `publicKey` + + The public key against which `rev` is verified if `verifyCommit` is enabled. + Requires the [`verified-fetches` experimental feature](@docroot@/contributing/experimental-features.md#xp-feature-verified-fetches). + + - `keytype` (default: `"ssh-ed25519"`) + + The key type of `publicKey`. + Possible values: + - `"ssh-dsa"` + - `"ssh-ecdsa"` + - `"ssh-ecdsa-sk"` + - `"ssh-ed25519"` + - `"ssh-ed25519-sk"` + - `"ssh-rsa"` + Requires the [`verified-fetches` experimental feature](@docroot@/contributing/experimental-features.md#xp-feature-verified-fetches). + + - `publicKeys` + + The public keys against which `rev` is verified if `verifyCommit` is enabled. + Must be given as a list of attribute sets with the following form: + ```nix + { + key = ""; + type = ""; # optional, default: "ssh-ed25519" + } + ``` + Requires the [`verified-fetches` experimental feature](@docroot@/contributing/experimental-features.md#xp-feature-verified-fetches). + + Here are some examples of how to use `fetchGit`. - To fetch a private repository over SSH: @@ -501,6 +542,21 @@ static RegisterPrimOp primop_fetchGit({ } ``` + - To verify the commit signature: + + ```nix + builtins.fetchGit { + url = "ssh://git@github.com/nixos/nix.git"; + verifyCommit = true; + publicKeys = [ + { + type = "ssh-ed25519"; + key = "AAAAC3NzaC1lZDI1NTE5AAAAIArPKULJOid8eS6XETwUjO48/HKBWl7FTCK0Z//fplDi"; + } + ]; + } + ``` + Nix will refetch the branch according to the [`tarball-ttl`](@docroot@/command-ref/conf-file.md#conf-tarball-ttl) setting. This behavior is disabled in [pure evaluation mode](@docroot@/command-ref/conf-file.md#conf-pure-eval). diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 51e551879..72fba0582 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -169,14 +169,14 @@ void doCommitVerification(const Path repoDir, const Path gitDir, const std::stri && k.type != "ssh-ed25519" && k.type != "ssh-ed25519-sk" && k.type != "ssh-rsa") - warn("Unknow keytype: %s\n" + warn("Unknown keytype: %s\n" "Please use one of\n" "- ssh-dsa\n" - "- ssh-ecdsa\n" - "- ssh-ecdsa-sk\n" - "- ssh-ed25519\n" - "- ssh-ed25519-sk\n" - "- ssh-rsa", k.type); + " ssh-ecdsa\n" + " ssh-ecdsa-sk\n" + " ssh-ed25519\n" + " ssh-ed25519-sk\n" + " ssh-rsa", k.type); allowedSigners += "* " + k.type + " " + k.key + "\n"; } writeFile(allowedSignersFile, allowedSigners); @@ -201,7 +201,7 @@ void doCommitVerification(const Path repoDir, const Path gitDir, const std::stri } re += "]"; if (status == 0 && std::regex_search(output, std::regex(re))) - printTalkative("Commit signature verification on commit %s succeeded", rev); + printTalkative("Signature verification on commit %s succeeded", rev); else throw Error("Commit signature verification on commit %s failed: \n%s", rev, output); } From 271932782dd3d44e0e238bd3234ca1e97996cfea Mon Sep 17 00:00:00 2001 From: BootRhetoric <110117466+BootRhetoric@users.noreply.github.com> Date: Fri, 20 Oct 2023 21:18:01 +0200 Subject: [PATCH 3/3] fetchGit and flake: add commit signature verification tests This adds simple tests of the commit signature verification mechanism of fetchGit and its flake input wrapper. OpenSSH is added to the build dependencies since it's needed to create a key when testing the functionality. It is neither a built- nor a runtime dependency. --- flake.nix | 1 + tests/functional/fetchGitVerification.sh | 76 ++++++++++++++++++++++++ tests/functional/local.mk | 1 + 3 files changed, 78 insertions(+) create mode 100644 tests/functional/fetchGitVerification.sh diff --git a/flake.nix b/flake.nix index 7cc4ed7fe..51d818423 100644 --- a/flake.nix +++ b/flake.nix @@ -185,6 +185,7 @@ buildPackages.git buildPackages.mercurial # FIXME: remove? only needed for tests buildPackages.jq # Also for custom mdBook preprocessor. + buildPackages.openssh # only needed for tests (ssh-keygen) ] ++ lib.optionals stdenv.hostPlatform.isLinux [(buildPackages.util-linuxMinimal or buildPackages.utillinuxMinimal)]; diff --git a/tests/functional/fetchGitVerification.sh b/tests/functional/fetchGitVerification.sh new file mode 100644 index 000000000..4d9209498 --- /dev/null +++ b/tests/functional/fetchGitVerification.sh @@ -0,0 +1,76 @@ +source common.sh + +requireGit +[[ $(type -p ssh-keygen) ]] || skipTest "ssh-keygen not installed" # require ssh-keygen + +enableFeatures "verified-fetches" + +clearStore + +repo="$TEST_ROOT/git" + +# generate signing keys +keysDir=$TEST_ROOT/.ssh +mkdir -p "$keysDir" +ssh-keygen -f "$keysDir/testkey1" -t ed25519 -P "" -C "test key 1" +key1File="$keysDir/testkey1.pub" +publicKey1=$(awk '{print $2}' "$key1File") +ssh-keygen -f "$keysDir/testkey2" -t rsa -P "" -C "test key 2" +key2File="$keysDir/testkey2.pub" +publicKey2=$(awk '{print $2}' "$key2File") + +git init $repo +git -C $repo config user.email "foobar@example.com" +git -C $repo config user.name "Foobar" +git -C $repo config gpg.format ssh + +echo 'hello' > $repo/text +git -C $repo add text +git -C $repo -c "user.signingkey=$key1File" commit -S -m 'initial commit' + +out=$(nix eval --impure --raw --expr "builtins.fetchGit { url = \"file://$repo\"; keytype = \"ssh-rsa\"; publicKey = \"$publicKey2\"; }" 2>&1) || status=$? +[[ $status == 1 ]] +[[ $out =~ 'No principal matched.' ]] +[[ $(nix eval --impure --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; publicKey = \"$publicKey1\"; } + \"/text\")") = 'hello' ]] + +echo 'hello world' > $repo/text +git -C $repo add text +git -C $repo -c "user.signingkey=$key2File" commit -S -m 'second commit' + +[[ $(nix eval --impure --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; publicKeys = [{key = \"$publicKey1\";} {type = \"ssh-rsa\"; key = \"$publicKey2\";}]; } + \"/text\")") = 'hello world' ]] + +# Flake input test +flakeDir="$TEST_ROOT/flake" +mkdir -p "$flakeDir" +cat > "$flakeDir/flake.nix" < "$flakeDir/flake.nix" <&1) || status=$? +[[ $status == 1 ]] +[[ $out =~ 'No principal matched.' ]] \ No newline at end of file diff --git a/tests/functional/local.mk b/tests/functional/local.mk index 3679349f8..fe0d0c4ed 100644 --- a/tests/functional/local.mk +++ b/tests/functional/local.mk @@ -55,6 +55,7 @@ nix_tests = \ secure-drv-outputs.sh \ restricted.sh \ fetchGitSubmodules.sh \ + fetchGitVerification.sh \ flakes/search-root.sh \ readfile-context.sh \ nix-channel.sh \