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

Add Store::isTrustedClient() #7515

Merged
merged 1 commit into from
Apr 7, 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
6 changes: 6 additions & 0 deletions doc/manual/src/release-notes/rl-next.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,9 @@

* `man nix-store-<operation>` and `man nix-env-<operation>`
* `nix-store --help --<operation>` and `nix-env --help --<operation>`.

* Nix when used as a client now checks whether the store (the server) trusts the client.
(The store always had to check whether it trusts the client, but now the client is informed of the store's decision.)
This is useful for scripting interactions with (non-legacy-ssh) remote Nix stores.

`nix store ping` and `nix doctor` now display this information.
5 changes: 4 additions & 1 deletion src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,9 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo

virtual void addBuildLog(const StorePath & path, std::string_view log) override
{ unsupported("addBuildLog"); }

std::optional<TrustedFlag> isTrustedClient() override
{ return NotTrusted; }
};


Expand Down Expand Up @@ -1467,7 +1470,7 @@ void LocalDerivationGoal::startDaemon()
FdSink to(remote.get());
try {
daemon::processConnection(store, from, to,
daemon::NotTrusted, daemon::Recursive);
NotTrusted, daemon::Recursive);
debug("terminated daemon connection");
} catch (SysError &) {
ignoreException();
Expand Down
9 changes: 9 additions & 0 deletions src/libstore/daemon.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,15 @@ void processConnection(
if (GET_PROTOCOL_MINOR(clientVersion) >= 33)
to << nixVersion;

if (GET_PROTOCOL_MINOR(clientVersion) >= 35) {
// We and the underlying store both need to trust the client for
// it to be trusted.
auto temp = trusted
? store->isTrustedClient()
: std::optional { NotTrusted };
worker_proto::write(*store, to, temp);
}

/* Send startup error messages to the client. */
tunnelLogger->startWork();

Expand Down
1 change: 0 additions & 1 deletion src/libstore/daemon.hh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

namespace nix::daemon {

enum TrustedFlag : bool { NotTrusted = false, Trusted = true };
enum RecursiveFlag : bool { NotRecursive = false, Recursive = true };

void processConnection(
Expand Down
8 changes: 8 additions & 0 deletions src/libstore/dummy-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store
callback(nullptr);
}

/**
* The dummy store is incapable of *not* trusting! :)
*/
virtual std::optional<TrustedFlag> isTrustedClient() override
{
return Trusted;
}

static std::set<std::string> uriSchemes() {
return {"dummy"};
}
Expand Down
12 changes: 12 additions & 0 deletions src/libstore/http-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,18 @@ class HttpBinaryCacheStore : public virtual HttpBinaryCacheStoreConfig, public v
}});
}

/**
* This isn't actually necessary read only. We support "upsert" now, so we
* have a notion of authentication via HTTP POST/PUT.
*
* For now, we conservatively say we don't know.
*
* \todo try to expose our HTTP authentication status.
*/
std::optional<TrustedFlag> isTrustedClient() override
{
return std::nullopt;
}
};

static RegisterStoreImplementation<HttpBinaryCacheStore, HttpBinaryCacheStoreConfig> regHttpBinaryCacheStore;
Expand Down
9 changes: 9 additions & 0 deletions src/libstore/legacy-ssh-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,15 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor
return conn->remoteVersion;
}

/**
* The legacy ssh protocol doesn't support checking for trusted-user.
* Try using ssh-ng:// instead if you want to know.
*/
std::optional<TrustedFlag> isTrustedClient() override
{
return std::nullopt;
}

void queryRealisationUncached(const DrvOutput &,
Callback<std::shared_ptr<const Realisation>> callback) noexcept override
// TODO: Implement
Expand Down
4 changes: 4 additions & 0 deletions src/libstore/local-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public
return paths;
}

std::optional<TrustedFlag> isTrustedClient() override
{
return Trusted;
}
};

void LocalBinaryCacheStore::init()
Expand Down
5 changes: 5 additions & 0 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1685,6 +1685,11 @@ unsigned int LocalStore::getProtocol()
return PROTOCOL_VERSION;
}

std::optional<TrustedFlag> LocalStore::isTrustedClient()
{
return Trusted;
}


#if defined(FS_IOC_SETFLAGS) && defined(FS_IOC_GETFLAGS) && defined(FS_IMMUTABLE_FL)

Expand Down
2 changes: 2 additions & 0 deletions src/libstore/local-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ public:

unsigned int getProtocol() override;

std::optional<TrustedFlag> isTrustedClient() override;

void vacuumDB();

void repairPath(const StorePath & path) override;
Expand Down
46 changes: 46 additions & 0 deletions src/libstore/remote-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,40 @@ void write(const Store & store, Sink & out, const StorePath & storePath)
}


std::optional<TrustedFlag> read(const Store & store, Source & from, Phantom<std::optional<TrustedFlag>> _)
{
auto temp = readNum<uint8_t>(from);
switch (temp) {
case 0:
return std::nullopt;
case 1:
return { Trusted };
case 2:
return { NotTrusted };
default:
throw Error("Invalid trusted status from remote");
Copy link
Member

Choose a reason for hiding this comment

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

Saving it for a potential follow-up (and fairly low-prio), but it could be nicer to just warn and return std::nullopt in that case. Just to make potential future evolutions of the protocol nicer.

Copy link
Member

Choose a reason for hiding this comment

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

I had thought about that, but I think it's just better to bunl the protocol version on that case? Maybe I'm forgetting, but I couldn't think of another time we tried to do this sort of week forward compat before.

Copy link
Member

Choose a reason for hiding this comment

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

We currently have more or less one forward compat mechanism, which is the protocol version handshake.
If we're going to improve the forward compat mechanism, it would make more sense to do it structurally, with a self-describing or schema based serialization where we can add fields as we please.

}
}

void write(const Store & store, Sink & out, const std::optional<TrustedFlag> & optTrusted)
{
if (!optTrusted)
out << (uint8_t)0;
else {
switch (*optTrusted) {
case Trusted:
out << (uint8_t)1;
break;
case NotTrusted:
out << (uint8_t)2;
break;
default:
assert(false);
};
}
}


ContentAddress read(const Store & store, Source & from, Phantom<ContentAddress> _)
{
return parseContentAddress(readString(from));
Expand Down Expand Up @@ -226,6 +260,13 @@ void RemoteStore::initConnection(Connection & conn)
conn.daemonNixVersion = readString(conn.from);
}

if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 35) {
conn.remoteTrustsUs = worker_proto::read(*this, conn.from, Phantom<std::optional<TrustedFlag>> {});
} else {
// We don't know the answer; protocol to old.
conn.remoteTrustsUs = std::nullopt;
}

auto ex = conn.processStderr();
if (ex) std::rethrow_exception(ex);
}
Expand Down Expand Up @@ -1082,6 +1123,11 @@ unsigned int RemoteStore::getProtocol()
return conn->daemonVersion;
}

std::optional<TrustedFlag> RemoteStore::isTrustedClient()
{
auto conn(getConnection());
return conn->remoteTrustsUs;
}

void RemoteStore::flushBadConnections()
{
Expand Down
3 changes: 3 additions & 0 deletions src/libstore/remote-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,16 @@ public:

unsigned int getProtocol() override;

std::optional<TrustedFlag> isTrustedClient() override;

void flushBadConnections();

struct Connection
{
FdSink to;
FdSource from;
unsigned int daemonVersion;
std::optional<TrustedFlag> remoteTrustsUs;
std::optional<std::string> daemonNixVersion;
std::chrono::time_point<std::chrono::steady_clock> startTime;

Expand Down
10 changes: 10 additions & 0 deletions src/libstore/s3-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,16 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual
return paths;
}

/**
* For now, we conservatively say we don't know.
*
* \todo try to expose our S3 authentication status.
*/
std::optional<TrustedFlag> isTrustedClient() override
{
return std::nullopt;
}

static std::set<std::string> uriSchemes() { return {"s3"}; }

};
Expand Down
12 changes: 12 additions & 0 deletions src/libstore/store-api.hh
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ const uint32_t exportMagic = 0x4558494e;


enum BuildMode { bmNormal, bmRepair, bmCheck };
enum TrustedFlag : bool { NotTrusted = false, Trusted = true };

struct BuildResult;

Expand Down Expand Up @@ -815,6 +816,17 @@ public:
return 0;
};

/**
* @return/ whether store trusts *us*.
*
* `std::nullopt` means we do not know.
*
* @note This is the opposite of the StoreConfig::isTrusted
* store setting. That is about whether *we* trust the store.
*/
virtual std::optional<TrustedFlag> isTrustedClient() = 0;


virtual Path toRealPath(const Path & storePath)
{
return storePath;
Expand Down
3 changes: 2 additions & 1 deletion src/libstore/worker-protocol.hh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace nix {
#define WORKER_MAGIC_1 0x6e697863
#define WORKER_MAGIC_2 0x6478696f

#define PROTOCOL_VERSION (1 << 8 | 34)
#define PROTOCOL_VERSION (1 << 8 | 35)
#define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00)
#define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff)

Expand Down Expand Up @@ -103,6 +103,7 @@ MAKE_WORKER_PROTO(, DerivedPath);
MAKE_WORKER_PROTO(, Realisation);
MAKE_WORKER_PROTO(, DrvOutput);
MAKE_WORKER_PROTO(, BuildResult);
MAKE_WORKER_PROTO(, std::optional<TrustedFlag>);

MAKE_WORKER_PROTO(template<typename T>, std::vector<T>);
MAKE_WORKER_PROTO(template<typename T>, std::set<T>);
Expand Down
13 changes: 13 additions & 0 deletions src/nix/doctor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ bool checkFail(const std::string & msg) {
return false;
}

void checkInfo(const std::string & msg) {
notice(ANSI_BLUE "[INFO] " ANSI_NORMAL + msg);
}

}

struct CmdDoctor : StoreCommand
Expand Down Expand Up @@ -63,6 +67,7 @@ struct CmdDoctor : StoreCommand
success &= checkProfileRoots(store);
}
success &= checkStoreProtocol(store->getProtocol());
checkTrustedUser(store);

if (!success)
throw Exit(2);
Expand Down Expand Up @@ -138,6 +143,14 @@ struct CmdDoctor : StoreCommand

return checkPass("Client protocol matches store protocol.");
}

void checkTrustedUser(ref<Store> store)
{
std::string_view trusted = store->isTrustedClient()
? "trusted"
: "not trusted";
checkInfo(fmt("You are %s by store uri: %s", trusted, store->getUri()));
}
};

static auto rCmdDoctor = registerCommand<CmdDoctor>("doctor");
5 changes: 5 additions & 0 deletions src/nix/ping-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,20 @@ struct CmdPingStore : StoreCommand, MixJSON
store->connect();
if (auto version = store->getVersion())
notice("Version: %s", *version);
if (auto trusted = store->isTrustedClient())
notice("Trusted: %s", *trusted);
} else {
nlohmann::json res;
Finally printRes([&]() {
logger->cout("%s", res);
});

res["url"] = store->getUri();
store->connect();
if (auto version = store->getVersion())
res["version"] = *version;
if (auto trusted = store->isTrustedClient())
res["trusted"] = *trusted;
}
}
};
Expand Down
4 changes: 4 additions & 0 deletions tests/legacy-ssh-store.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
source common.sh

# Check that store ping trusted doesn't yet work with ssh://
nix --store ssh://localhost?remote-store=$TEST_ROOT/other-store store ping --json | jq -e 'has("trusted") | not'
3 changes: 3 additions & 0 deletions tests/local-store.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,6 @@ PATH2=$(nix path-info --store "$PWD/x" $CORRECT_PATH)

PATH3=$(nix path-info --store "local?root=$PWD/x" $CORRECT_PATH)
[ $CORRECT_PATH == $PATH3 ]

# Ensure store ping trusted works with local store
nix --store ./x store ping --json | jq -e '.trusted'
1 change: 1 addition & 0 deletions tests/local.mk
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ nix_tests = \
ca/gc.sh \
gc.sh \
remote-store.sh \
legacy-ssh-store.sh \
lang.sh \
experimental-features.sh \
fetchMercurial.sh \
Expand Down
11 changes: 11 additions & 0 deletions tests/remote-store.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,19 @@ clearStore
# Ensure "fake ssh" remote store works just as legacy fake ssh would.
nix --store ssh-ng://localhost?remote-store=$TEST_ROOT/other-store doctor

# Ensure that store ping trusted works with ssh-ng://
nix --store ssh-ng://localhost?remote-store=$TEST_ROOT/other-store store ping --json | jq -e '.trusted'

startDaemon

if isDaemonNewer "2.15pre0"; then
# Ensure that ping works trusted with new daemon
nix store ping --json | jq -e '.trusted'
else
# And the the field is absent with the old daemon
nix store ping --json | jq -e 'has("trusted") | not'
fi

# Test import-from-derivation through the daemon.
[[ $(nix eval --impure --raw --expr '
with import ./config.nix;
Expand Down