From a96156c58f94165e8dc61981ab9fa6e97a963e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sol=C3=A8ne=20Rapenne?= Date: Wed, 14 Dec 2022 14:01:29 +0100 Subject: warnings: enhance the case of untrusted substituter for untrusted user --- src/libstore/daemon.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/libstore/daemon.cc') diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 12596ba49..3731b21b8 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -222,7 +222,8 @@ struct ClientSettings else if (!hasSuffix(s, "/") && trusted.count(s + "/")) subs.push_back(s + "/"); else - warn("ignoring untrusted substituter '%s'", s); + warn("ignoring untrusted substituter '%s', you are not a trusted user.\n" + "More information about 'trusted-substituters' option in nix.conf man page", s); res = subs; return true; }; -- cgit v1.2.3 From 64951d9125fc223bbeb939b1c774533a8c6ded98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sol=C3=A8ne=20Rapenne?= Date: Tue, 3 Jan 2023 15:35:28 +0100 Subject: Update src/libstore/daemon.cc Co-authored-by: Valentin Gagarin --- src/libstore/daemon.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/daemon.cc') diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 3731b21b8..e2a7dab35 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -223,7 +223,7 @@ struct ClientSettings subs.push_back(s + "/"); else warn("ignoring untrusted substituter '%s', you are not a trusted user.\n" - "More information about 'trusted-substituters' option in nix.conf man page", s); + "Run `man nix.conf` for more information on the `substituters` configuration option.", s); res = subs; return true; }; -- cgit v1.2.3 From 895c525d04901022b7e0c4586ee12774fb4f8c6f Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Thu, 2 Feb 2023 06:03:45 -0500 Subject: daemon: Warn on old clients passing unexpected plugin-files. The setting itself was already ignored due to exception trying to set pluginFiles. --- src/libstore/daemon.cc | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/libstore/daemon.cc') diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index e2a7dab35..d6621ec0b 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -236,6 +236,10 @@ struct ClientSettings // the daemon, as that could cause some pretty weird stuff if (parseFeatures(tokenizeString(value)) != settings.experimentalFeatures.get()) debug("Ignoring the client-specified experimental features"); + } else if (name == settings.pluginFiles.name) { + if (tokenizeString(value) != settings.pluginFiles.get()) + warn("Ignoring the client-specified plugin-files.\n" + "The client specifying plugins to the daemon never made sense, and was removed in Nix >=2.14."); } else if (trusted || name == settings.buildTimeout.name -- cgit v1.2.3 From 479c0117840a5dc710019db006c5940b29d98dcc Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 2 Feb 2023 12:02:03 -0500 Subject: Get rid of the `authHook` parameter on `processConnection` This is (morally) dead code. As @edolstra pointed out in https://github.com/NixOS/nix/pull/5226#discussion_r1073470813, this is no longer needed. I created this in 8d4162ff9e940ea9e2f97b07f3030a722695901a, so it is fitting that I now destroy it :). --- src/libstore/daemon.cc | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'src/libstore/daemon.cc') diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index d6621ec0b..d1f69fe2d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -985,8 +985,7 @@ void processConnection( FdSource & from, FdSink & to, TrustedFlag trusted, - RecursiveFlag recursive, - std::function authHook) + RecursiveFlag recursive) { auto monitor = !recursive ? std::make_unique(from.fd) : nullptr; @@ -1029,10 +1028,6 @@ void processConnection( try { - /* If we can't accept clientVersion, then throw an error - *here* (not above). */ - authHook(*store); - tunnelLogger->stopWork(); to.flush(); -- cgit v1.2.3 From a47e055e090a85884413dfc7889897f4e3d831fb Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 2 Feb 2023 13:20:44 -0500 Subject: Move `trustedUsers` and `allowedUsers` to separate config struct These settings are not needed for libstore at all, they are just used by the nix daemon *command* for authorization on unix domain sockets. My moving them to a new configuration struct just in that file, we avoid them leaking anywhere else. Also, it is good to break up the mammoth `Settings` struct in general. Issue #5638 tracks this. The message is not changed because I do not want to regress in convenience to the user. Just saying "this connection is not trusted" doesn't tell them out to fix the issue. The ideal thing to do would be to somehow parameterize `processCommand` on how the error should be displayed, so different sorts of connections can display different information to the user based on how authentication is performed for the connection in question. This, however, is a good bit more work, so it is left for the future. This came up with me thinking about the tcp:// store (#5265). The larger project is not TCP *per se*, but the idea that it should be possible for something else to manage access control to services like the Nix Daemon, and those services simply trust or trust the incoming connection as they are told. This is a more capability-oriented way of thinking about trust than "every server implements its own auth separately" as we are used to today. Its very great that libstore itself already implements just this model, and so via this refactor I basically want to "enshrine" that so it continues to be the case. --- src/libstore/daemon.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'src/libstore/daemon.cc') diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index d6621ec0b..591693a64 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -529,7 +529,14 @@ static void performOp(TunnelLogger * logger, ref store, mode = (BuildMode) readInt(from); /* Repairing is not atomic, so disallowed for "untrusted" - clients. */ + clients. + + FIXME: layer violation in this message: the daemon code (i.e. + this file) knows whether a client/connection is trusted, but it + does not how how the client was authenticated. The mechanism + need not be getting the UID of the other end of a Unix Domain + Socket. + */ if (mode == bmRepair && !trusted) throw Error("repairing is not allowed because you are not in 'trusted-users'"); } @@ -546,7 +553,9 @@ static void performOp(TunnelLogger * logger, ref store, mode = (BuildMode) readInt(from); /* Repairing is not atomic, so disallowed for "untrusted" - clients. */ + clients. + + FIXME: layer violation; see above. */ if (mode == bmRepair && !trusted) throw Error("repairing is not allowed because you are not in 'trusted-users'"); -- cgit v1.2.3