Package: guix-patches;
Reported by: Sergey Trofimov <sarg <at> sarg.org.ru>
Date: Mon, 21 Apr 2025 17:52:01 UTC
Severity: normal
Tags: patch
To reply to this bug, email your comments to 77968 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
guix-patches <at> gnu.org:bug#77968; Package guix-patches.
(Mon, 21 Apr 2025 17:52:01 GMT) Full text and rfc822 format available.Sergey Trofimov <sarg <at> sarg.org.ru>:guix-patches <at> gnu.org.
(Mon, 21 Apr 2025 17:52:01 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Sergey Trofimov <sarg <at> sarg.org.ru> To: guix-patches <at> gnu.org Cc: Sergey Trofimov <sarg <at> sarg.org.ru> Subject: [PATCH 0/1] gnu: openssh: Trust store items owned by guix-daemon. Date: Mon, 21 Apr 2025 19:51:18 +0200
After migration to rootless guix-daemon on Guix system, ssh started to refuse operations with the error `Bad owner or permissions on /home/sarg/.ssh/config`. The config is managed with `home-openssh-service-type` and is a symlink to /gnu/store/... The file was previously owned by root which is treated specially in openssh source code. As a solution I suggest to patch ssh to trust config files in /gnu/store/ As a workaround users can for now use `ssh -F ~/.ssh/config` as this would skip ownership checks. Sergey Trofimov (1): gnu: openssh: Trust store items owned by guix-daemon. .../openssh-trust-guix-store-directory.patch | 67 +++++++++++++------ gnu/packages/ssh.scm | 2 +- 2 files changed, 47 insertions(+), 22 deletions(-) base-commit: 7a7eff34613c9b3357adf39813793f607c03629d -- 2.49.0
guix-patches <at> gnu.org:bug#77968; Package guix-patches.
(Mon, 21 Apr 2025 19:00:03 GMT) Full text and rfc822 format available.Message #8 received at 77968 <at> debbugs.gnu.org (full text, mbox):
From: Sergey Trofimov <sarg <at> sarg.org.ru> To: 77968 <at> debbugs.gnu.org Cc: Sergey Trofimov <sarg <at> sarg.org.ru> Subject: [PATCH 1/1] gnu: openssh: Trust store items owned by guix-daemon. Date: Mon, 21 Apr 2025 19:58:25 +0200
* gnu/packages/patches/openssh-trust-guix-store-directory.patch
(openssh): Adjust to trust files in guix store owned by guix-daemon.
* gnu/packages/ssh.scm (openssh): Append ending slash when substituting
STORE_DIRECTORY.
Change-Id: I3bd01f8b9d6406e3b886eea8f4b8c265a51cc72f
---
.../openssh-trust-guix-store-directory.patch | 67 +++++++++++++------
gnu/packages/ssh.scm | 2 +-
2 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/gnu/packages/patches/openssh-trust-guix-store-directory.patch b/gnu/packages/patches/openssh-trust-guix-store-directory.patch
index b3a9c1bdfce..d190740f100 100644
--- a/gnu/packages/patches/openssh-trust-guix-store-directory.patch
+++ b/gnu/packages/patches/openssh-trust-guix-store-directory.patch
@@ -3,20 +3,20 @@ From: Alexey Abramov <levenson <at> mmer.org>
Date: Fri, 22 Apr 2022 11:32:15 +0200
Subject: [PATCH] Trust guix store directory
-To be able to execute binaries defined in OpenSSH configuration, we
-need to tell OpenSSH that we can trust Guix store objects. safe_path
-procedure takes a canonical path and for each component, walking
-upwards, checks ownership and permissions constrains which are: must
-be owned by root, not writable by group or others.
+To be able to execute binaries defined in OpenSSH configuration, we need to
+tell OpenSSH that we can trust Guix store objects. safe_path procedure is
+patched to assume files in Guix store to be safe. Additionally configuration
+file placed in Guix store is assumed to be safe to load.
---
- misc.c | 5 +++++
- 1 file changed, 5 insertions(+)
+ misc.c | 6 ++++++
+ readconf.c | 7 ++++---
+ 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/misc.c b/misc.c
-index 0134d69..7131d5e 100644
+index dd0bd032a..6b866464c 100644
--- a/misc.c
+++ b/misc.c
-@@ -2146,6 +2146,7 @@ int
+@@ -2254,6 +2254,7 @@ int
safe_path(const char *name, struct stat *stp, const char *pw_dir,
uid_t uid, char *err, size_t errlen)
{
@@ -24,17 +24,42 @@ index 0134d69..7131d5e 100644
char buf[PATH_MAX], homedir[PATH_MAX];
char *cp;
int comparehome = 0;
-@@ -2178,6 +2179,10 @@ safe_path(const char *name, struct stat *stp, const char *pw_dir,
- }
- strlcpy(buf, cp, sizeof(buf));
-
-+ /* If we are past the Guix store then we can stop */
-+ if (strcmp(guix_store, buf) == 0)
-+ break;
+@@ -2271,6 +2272,11 @@ safe_path(const char *name, struct stat *stp, const char *pw_dir,
+ snprintf(err, errlen, "%s is not a regular file", buf);
+ return -1;
+ }
++ // the file is trusted when it is located in guix store
++ if (strncmp(buf, guix_store, strlen(guix_store)) == 0) {
++ return 0;
++ }
+
- if (stat(buf, &st) == -1 ||
- (!platform_sys_dir_uid(st.st_uid) && st.st_uid != uid) ||
- (st.st_mode & 022) != 0) {
+ if ((!platform_sys_dir_uid(stp->st_uid) && stp->st_uid != uid) ||
+ (stp->st_mode & 022) != 0) {
+ snprintf(err, errlen, "bad ownership or modes for file %s",
+diff --git a/readconf.c b/readconf.c
+index 7cbe7d2c2..40a5f1ace 100644
+--- a/readconf.c
++++ b/readconf.c
+@@ -2566,6 +2566,7 @@ read_config_file_depth(const char *filename, struct passwd *pw,
+ {
+ FILE *f;
+ char *line = NULL;
++ char errmsg[512];
+ size_t linesize = 0;
+ int linenum;
+ int bad_options = 0;
+@@ -2581,9 +2582,9 @@ read_config_file_depth(const char *filename, struct passwd *pw,
+
+ if (fstat(fileno(f), &sb) == -1)
+ fatal("fstat %s: %s", filename, strerror(errno));
+- if (((sb.st_uid != 0 && sb.st_uid != getuid()) ||
+- (sb.st_mode & 022) != 0))
+- fatal("Bad owner or permissions on %s", filename);
++ if (safe_path(filename, &sb, pw->pw_dir, pw->pw_uid, errmsg, sizeof(errmsg)) != 0) {
++ fatal(errmsg);
++ }
+ }
+
+ debug("Reading configuration data %.200s", filename);
--
-2.34.0
-
+2.49.0
diff --git a/gnu/packages/ssh.scm b/gnu/packages/ssh.scm
index 877d129e918..9e0684e26c8 100644
--- a/gnu/packages/ssh.scm
+++ b/gnu/packages/ssh.scm
@@ -262,7 +262,7 @@ (define-public openssh
(lambda _
(substitute* "misc.c"
(("@STORE_DIRECTORY@")
- (string-append "\"" (%store-directory) "\"")))))
+ (string-append "\"" (%store-directory) "/\"")))))
(add-before 'check 'patch-tests
(lambda _
(substitute* "regress/test-exec.sh"
--
2.49.0
sarg <at> sarg.org.ru, guix-patches <at> gnu.org:bug#77968; Package guix-patches.
(Wed, 23 Apr 2025 14:14:04 GMT) Full text and rfc822 format available.Message #11 received at 77968 <at> debbugs.gnu.org (full text, mbox):
From: Sergey Trofimov <sarg <at> sarg.org.ru> To: 77968 <at> debbugs.gnu.org Cc: Sergey Trofimov <sarg <at> sarg.org.ru> Subject: [PATCH v1] gnu: openssh: Adapt for root-less guix store. Date: Wed, 23 Apr 2025 16:13:10 +0200
* gnu/packages/patches/openssh-trust-guix-store-directory.patch
(openssh): Adjust to trust files in guix store owned by guix-daemon.
* gnu/packages/ssh.scm (openssh): [phases]: Append ending slash when
substituting STORE_DIRECTORY. Use default PRIVSEP_PATH (/var/empty).
Change-Id: I3bd01f8b9d6406e3b886eea8f4b8c265a51cc72f
---
.../openssh-trust-guix-store-directory.patch | 67 +++++++++++++------
gnu/packages/ssh.scm | 14 ++--
2 files changed, 51 insertions(+), 30 deletions(-)
diff --git a/gnu/packages/patches/openssh-trust-guix-store-directory.patch b/gnu/packages/patches/openssh-trust-guix-store-directory.patch
index b3a9c1bdfc..d190740f10 100644
--- a/gnu/packages/patches/openssh-trust-guix-store-directory.patch
+++ b/gnu/packages/patches/openssh-trust-guix-store-directory.patch
@@ -3,20 +3,20 @@ From: Alexey Abramov <levenson <at> mmer.org>
Date: Fri, 22 Apr 2022 11:32:15 +0200
Subject: [PATCH] Trust guix store directory
-To be able to execute binaries defined in OpenSSH configuration, we
-need to tell OpenSSH that we can trust Guix store objects. safe_path
-procedure takes a canonical path and for each component, walking
-upwards, checks ownership and permissions constrains which are: must
-be owned by root, not writable by group or others.
+To be able to execute binaries defined in OpenSSH configuration, we need to
+tell OpenSSH that we can trust Guix store objects. safe_path procedure is
+patched to assume files in Guix store to be safe. Additionally configuration
+file placed in Guix store is assumed to be safe to load.
---
- misc.c | 5 +++++
- 1 file changed, 5 insertions(+)
+ misc.c | 6 ++++++
+ readconf.c | 7 ++++---
+ 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/misc.c b/misc.c
-index 0134d69..7131d5e 100644
+index dd0bd032a..6b866464c 100644
--- a/misc.c
+++ b/misc.c
-@@ -2146,6 +2146,7 @@ int
+@@ -2254,6 +2254,7 @@ int
safe_path(const char *name, struct stat *stp, const char *pw_dir,
uid_t uid, char *err, size_t errlen)
{
@@ -24,17 +24,42 @@ index 0134d69..7131d5e 100644
char buf[PATH_MAX], homedir[PATH_MAX];
char *cp;
int comparehome = 0;
-@@ -2178,6 +2179,10 @@ safe_path(const char *name, struct stat *stp, const char *pw_dir,
- }
- strlcpy(buf, cp, sizeof(buf));
-
-+ /* If we are past the Guix store then we can stop */
-+ if (strcmp(guix_store, buf) == 0)
-+ break;
+@@ -2271,6 +2272,11 @@ safe_path(const char *name, struct stat *stp, const char *pw_dir,
+ snprintf(err, errlen, "%s is not a regular file", buf);
+ return -1;
+ }
++ // the file is trusted when it is located in guix store
++ if (strncmp(buf, guix_store, strlen(guix_store)) == 0) {
++ return 0;
++ }
+
- if (stat(buf, &st) == -1 ||
- (!platform_sys_dir_uid(st.st_uid) && st.st_uid != uid) ||
- (st.st_mode & 022) != 0) {
+ if ((!platform_sys_dir_uid(stp->st_uid) && stp->st_uid != uid) ||
+ (stp->st_mode & 022) != 0) {
+ snprintf(err, errlen, "bad ownership or modes for file %s",
+diff --git a/readconf.c b/readconf.c
+index 7cbe7d2c2..40a5f1ace 100644
+--- a/readconf.c
++++ b/readconf.c
+@@ -2566,6 +2566,7 @@ read_config_file_depth(const char *filename, struct passwd *pw,
+ {
+ FILE *f;
+ char *line = NULL;
++ char errmsg[512];
+ size_t linesize = 0;
+ int linenum;
+ int bad_options = 0;
+@@ -2581,9 +2582,9 @@ read_config_file_depth(const char *filename, struct passwd *pw,
+
+ if (fstat(fileno(f), &sb) == -1)
+ fatal("fstat %s: %s", filename, strerror(errno));
+- if (((sb.st_uid != 0 && sb.st_uid != getuid()) ||
+- (sb.st_mode & 022) != 0))
+- fatal("Bad owner or permissions on %s", filename);
++ if (safe_path(filename, &sb, pw->pw_dir, pw->pw_uid, errmsg, sizeof(errmsg)) != 0) {
++ fatal(errmsg);
++ }
+ }
+
+ debug("Reading configuration data %.200s", filename);
--
-2.34.0
-
+2.49.0
diff --git a/gnu/packages/ssh.scm b/gnu/packages/ssh.scm
index 5f0b3e2a6a..0518d2ee1c 100644
--- a/gnu/packages/ssh.scm
+++ b/gnu/packages/ssh.scm
@@ -267,16 +267,11 @@ (define-public openssh
'()))
#:phases
#~(modify-phases %standard-phases
- (add-after 'configure 'reset-/var/empty
- (lambda _
- (substitute* "Makefile"
- (("PRIVSEP_PATH=/var/empty")
- (string-append "PRIVSEP_PATH=" #$output "/var/empty")))))
(add-after 'configure 'set-store-location
(lambda _
(substitute* "misc.c"
(("@STORE_DIRECTORY@")
- (string-append "\"" (%store-directory) "\"")))))
+ (string-append "\"" (%store-directory) "/\"")))))
(add-before 'check 'patch-tests
(lambda _
(substitute* "regress/test-exec.sh"
@@ -289,9 +284,10 @@ (define-public openssh
(string-append pre post)))))
(replace 'install
(lambda* (#:key (make-flags '()) #:allow-other-keys)
- ;; Install without host keys and system configuration files. This
- ;; will install /var/empty to the store, which is needed by the
- ;; system openssh-service-type.
+ ;; don't create /var/empty
+ (substitute* "Makefile"
+ ((".*MKDIR_P.*PRIVSEP_PATH.*") ""))
+ ;; Install without host keys and system configuration files.
(apply invoke "make" "install-nosysconf" make-flags)
(with-directory-excursion "contrib"
(chmod "ssh-copy-id" #o555)
base-commit: 699ce22ed812cf8cfcdd8d0341829f8fac2c864a
--
2.49.0
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.