Merge pull request #8848 from BootRhetoric/flake-authentication

2023-11-04 00:23:28 +01:00 · 2023-11-04 00:23:28 +01:00 · 8e222fbb12
parent 66cb364f58 271932782d
commit 8e222fbb12
11 changed files with 268 additions and 9 deletions
--- 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).
--- 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)];

--- 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
            }
--- 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 <ctime>
 #include <iomanip>
@ -125,6 +126,10 @@ static void fetchTree(
                attrs.emplace(state.symbols[attr.name], Explicit<bool>{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 = "<public key>";
+          type = "<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).
--- a/src/libfetchers/fetchers.cc
+++ b/src/libfetchers/fetchers.cc
@ -360,4 +360,9 @@ std::optional<ExperimentalFeature> InputScheme::experimentalFeature() const
    return {};
 }

+std::string publicKeys_to_string(const std::vector<PublicKey>& publicKeys)
+{
+    return ((nlohmann::json) publicKeys).dump();
+}
+
 }
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@ -182,4 +182,13 @@ void registerInputScheme(std::shared_ptr<InputScheme> && 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<PublicKey>&);
+
 }
--- a/src/libfetchers/git.cc
+++ b/src/libfetchers/git.cc
@ -143,6 +143,69 @@ struct WorkdirInfo
    bool hasHead = false;
 };

+std::vector<PublicKey> getPublicKeys(const Attrs & attrs) {
+    std::vector<PublicKey> publicKeys;
+    if (attrs.contains("publicKeys")) {
+        nlohmann::json publicKeysJson = nlohmann::json::parse(getStrAttr(attrs, "publicKeys"));
+        ensureType(publicKeysJson, nlohmann::json::value_t::array);
+        publicKeys = publicKeysJson.get<std::vector<PublicKey>>();
+    }
+    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<PublicKey>& 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("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);
+        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("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<bool> { value == "1" });
            else
                url2.query.emplace(name, value);
@ -306,14 +369,26 @@ struct GitInputScheme : InputScheme
            "name",
            "dirtyRev",
            "dirtyShortRev",
+            "verifyCommit",
+            "keytype",
+            "publicKey",
+            "publicKeys",
        };
    }

    std::optional<Input> 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<PublicKey> 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);
--- 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<ExperimentalFeatureDetails, 15> xpFeatureDetails = {{
+constexpr std::array<ExperimentalFeatureDetails, 16> xpFeatureDetails = {{
    {
        .tag = Xp::CaDerivations,
        .name = "ca-derivations",
@ -227,7 +227,14 @@ constexpr std::array<ExperimentalFeatureDetails, 15> 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(
--- a/src/libutil/experimental-features.hh
+++ b/src/libutil/experimental-features.hh
@ -32,6 +32,7 @@ enum struct ExperimentalFeature
    ParseTomlTimestamps,
    ReadOnlyLocalStore,
    ConfigurableImpureEnv,
+    VerifiedFetches,
 };

 /**
--- a/tests/functional/fetchGitVerification.sh
+++ 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" <<EOF
+{
+  inputs.test = {
+    type = "git";
+    url = "file://$repo";
+    flake = false;
+    publicKeys = [
+      { type = "ssh-rsa"; key = "$publicKey2"; }
+    ];
+  };
+
+  outputs = { test, ... }: { test = test.outPath; };
+}
+EOF
+nix build --out-link "$flakeDir/result" "$flakeDir#test"
+[[ $(cat "$flakeDir/result/text") = 'hello world' ]]
+
+cat > "$flakeDir/flake.nix" <<EOF
+{
+  inputs.test = {
+    type = "git";
+    url = "file://$repo";
+    flake = false;
+    publicKey= "$publicKey1";
+  };
+
+  outputs = { test, ... }: { test = test.outPath; };
+}
+EOF
+out=$(nix build "$flakeDir#test" 2>&1) || status=$?
+[[ $status == 1 ]]
+[[ $out =~ 'No principal matched.' ]]
--- 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 \