Skip to content
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.

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

Commit Signature Verification #8848

Merged
merged 3 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion doc/manual/src/release-notes/rl-next.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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).
1 change: 1 addition & 0 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -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)];

Expand Down
10 changes: 8 additions & 2 deletions src/libexpr/flake/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "fetchers.hh"
#include "finally.hh"
#include "fetch-settings.hh"
#include "value-to-json.hh"

namespace nix {

Expand Down Expand Up @@ -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
}
Expand Down
56 changes: 56 additions & 0 deletions src/libexpr/primops/fetchTree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "registry.hh"
#include "tarball.hh"
#include "url.hh"
#include "value-to-json.hh"

#include <ctime>
#include <iomanip>
Expand Down Expand Up @@ -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)));
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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).
Expand Down
5 changes: 5 additions & 0 deletions src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

}
9 changes: 9 additions & 0 deletions src/libfetchers/fetchers.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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>&);

}
104 changes: 100 additions & 4 deletions src/libfetchers/git.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand All @@ -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;
}

Expand Down Expand Up @@ -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";
Expand All @@ -445,6 +531,8 @@ struct GitInputScheme : InputScheme
{"type", cacheType},
{"name", name},
{"rev", input.getRev()->gitRev()},
{"verifyCommit", verifyCommit},
{"publicKeys", publicKeys_to_string(publicKeys)},
});
};

Expand All @@ -467,19 +555,24 @@ 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);
}
}

Attrs unlockedAttrs({
{"type", cacheType},
{"name", name},
{"url", actualUrl},
{"verifyCommit", verifyCommit},
{"publicKeys", publicKeys_to_string(publicKeys)},
});

Path repoDir;
Expand Down Expand Up @@ -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);
Expand Down
11 changes: 9 additions & 2 deletions src/libutil/experimental-features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions src/libutil/experimental-features.hh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ enum struct ExperimentalFeature
ParseTomlTimestamps,
ReadOnlyLocalStore,
ConfigurableImpureEnv,
VerifiedFetches,
};

/**
Expand Down
Loading
Loading