From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id WBhDKaj6s1/gWwAA0tVLHw (envelope-from ) for ; Tue, 17 Nov 2020 16:30:32 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id iPAMJaj6s1/2HgAAbx9fmQ (envelope-from ) for ; Tue, 17 Nov 2020 16:30:32 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 3E6F7940546 for ; Tue, 17 Nov 2020 16:30:31 +0000 (UTC) Received: from localhost ([::1]:59652 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kf3sD-000165-Ty for larch@yhetil.org; Tue, 17 Nov 2020 11:30:29 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:37428) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kf3rm-00015w-Jt for guix-patches@gnu.org; Tue, 17 Nov 2020 11:30:02 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:49424) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kf3rm-0007Oa-AO for guix-patches@gnu.org; Tue, 17 Nov 2020 11:30:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kf3rm-0005pS-6W for guix-patches@gnu.org; Tue, 17 Nov 2020 11:30:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#44700] services: setuid: More configurable setuid support. Resent-From: Maxim Cournoyer Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 17 Nov 2020 16:30:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 44700 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: To: Christopher Lemmer Webber Cc: 44700@debbugs.gnu.org Received: via spool by 44700-submit@debbugs.gnu.org id=B44700.160563057422342 (code B ref 44700); Tue, 17 Nov 2020 16:30:02 +0000 Received: (at 44700) by debbugs.gnu.org; 17 Nov 2020 16:29:34 +0000 Received: from localhost ([127.0.0.1]:60968 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kf3rJ-0005oI-Gd for submit@debbugs.gnu.org; Tue, 17 Nov 2020 11:29:33 -0500 Received: from mail-qk1-f182.google.com ([209.85.222.182]:35516) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kf3rG-0005o3-GF for 44700@debbugs.gnu.org; Tue, 17 Nov 2020 11:29:31 -0500 Received: by mail-qk1-f182.google.com with SMTP id v143so20974507qkb.2 for <44700@debbugs.gnu.org>; Tue, 17 Nov 2020 08:29:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=Oy0JDJx+kbxQSB/RvV/1guFDyI9IKIVQ3irPQjUsEx8=; b=HNXfai4TV0wpJsFWJkd2KBz0XQUklYSS7ac1Y2Xcg21x7584H3m9Fu/wci3l6v++Lg 4UJ3A6ZkkBZjb+7xl8HAji+4Io2WBDRbcfOmPgACvvXYKrz6h6/wE7QhHQSMTPYJQO+h I1NYxH1sbozu3yxQf4/qsM7jdQ8Yh+Pwi7+DcbcFQfkb0um7AXZgi2N9ZiAd57RWxUSJ EADlhg7Q28OY47Ia+buPv8kIu92BYi726GWIeZ9JLZ+EPP/UIvxgOcTJ2fJkVD/zhghs 2mcZAlP+zUCK1W9EQ5uFjsP1wfHPjz68J5xCM9tInrz75Y1zR0aC5z7HU68JyJNbKqDb gJEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version; bh=Oy0JDJx+kbxQSB/RvV/1guFDyI9IKIVQ3irPQjUsEx8=; b=ZaD7JAE7hhc0WxrBcF6KNP5kODVQ7Hv6LgK5Jy/l5hqw0QLiSV5DShZ5GB0eMZQUhr WwurT+aIUQZXp+A/l2QmzkbQTI4hkkX5ofOK3R4U6jZbORbnvu4gnUwU5RbhnHEBnAF2 DA9MYkF5WChfNGgqUu/rZbF3+Me0axzzJZhOQ0O+BUezeB0sJV50y6j3kZQOLyV/LJJh 1tTwQiQFvl9ciWenCwFfvrVjY7b4XOk0cbnsWQQkL+vZlmVReeEq/ivL6V35cSrYkmM9 3MycN83kX4PpMQmxcspJRfMi0YpiqOg6kQG/WfX7v3EgDyPTkmB6JlK5/dK5CcAoDTw5 agtQ== X-Gm-Message-State: AOAM530Gc0zg/7rMiavZMJsiL85n9Xr0dvKWzlLknY8aQLIJVktaF+1c +ulYpSVFzL+0mo/7YNdzBtXUfW7x9/L7Aw== X-Google-Smtp-Source: ABdhPJyR9WIfWuc8yioNAyJy/Hg7A0fWF0cCYb+sQBcGE6sBu7aQIb9zhNTFclagmWo0L5VswKECXw== X-Received: by 2002:a37:a783:: with SMTP id q125mr397513qke.10.1605630564616; Tue, 17 Nov 2020 08:29:24 -0800 (PST) Received: from hurd (dsl-205-236-230-198.b2b2c.ca. [205.236.230.198]) by smtp.gmail.com with ESMTPSA id x24sm10276969qkx.23.2020.11.17.08.29.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Nov 2020 08:29:23 -0800 (PST) From: Maxim Cournoyer References: <874klog9tk.fsf@dustycloud.org> Date: Tue, 17 Nov 2020 11:29:23 -0500 In-Reply-To: <874klog9tk.fsf@dustycloud.org> (Christopher Lemmer Webber's message of "Mon, 16 Nov 2020 18:29:11 -0500") Message-ID: <875z64aqvw.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -1.0 (-) X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Scanner: ns3122888.ip-94-23-21.eu Authentication-Results: aspmx1.migadu.com; dkim=fail (headers rsa verify failed) header.d=gmail.com header.s=20161025 header.b=HNXfai4T; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Spam-Score: 0.09 X-TUID: U5XkQBSXFqbb Hello Christopher, Christopher Lemmer Webber writes: > This patch allows for configuring the specific user, group, and whether > to set the setuid and setgid bits. > > See also: > https://lists.gnu.org/archive/html/guix-devel/2020-11/msg00369.html > > But I thought I'd open this here so we could track changes since this is > technically independent of the postfix stuff. Anyway, patch attached. > One change since the last email above is that I added support for > string-based username/groups. > > This also needs documentation, I suppose, so that should be done. > But it would be good to know if this patch looks like it's on the "right > path" or not. > > From eadac673fb22132c555a4e1cee57a6308ecfdad4 Mon Sep 17 00:00:00 2001 > From: Christopher Lemmer Webber > Date: Sun, 15 Nov 2020 16:58:52 -0500 > Subject: [PATCH] services: setuid: More configurable setuid support. > > New record with fields for setting the specific user and > group, as well as specifically selecting the setuid and setgid bits, for a > program within the setuid-program-service. Please make this a full sentence, e.g. "This adds a new record [...]". > > * gnu/services.scm (): New record type. > (setuid-program, make-setuid-program, setuid-program?) > (setuid-program-program, stuid-program-setuid?, setuid-program-setgid?) > (setuid-program-user, setuid-program-group): New variables, export them. > (setuid-program-entry): New variable, a procedure used for the > service-extension of activation-service-type as set up by > setuid-program-service-type. Unpacks the record, > handing off within the gexp to activate-setuid-programs. > (setuid-program-service-type): Make use of setuid-program-entry. > * gnu/build/activation.scm (activate-setuid-programs): Update to expect a > ftagged list for each program entry, pre-unpacked from the ^tagged > record before being handed to this procedure. The doc needs to be updated, as well as the current uses in the code base. > --- > gnu/build/activation.scm | 46 +++++++++++++++++++++---------------- > gnu/services.scm | 49 +++++++++++++++++++++++++++++++++++++--- > 2 files changed, 73 insertions(+), 22 deletions(-) > > diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm > index 4b67926e88..fd17ce0434 100644 > --- a/gnu/build/activation.scm > +++ b/gnu/build/activation.scm > @@ -229,13 +229,6 @@ they already exist." > (define (activate-setuid-programs programs) > "Turn PROGRAMS, a list of file names, into setuid programs stored under > %SETUID-DIRECTORY." > - (define (make-setuid-program prog) > - (let ((target (string-append %setuid-directory > - "/" (basename prog)))) > - (copy-file prog target) > - (chown target 0 0) > - (chmod target #o6555))) > - I think it'd be nicer to keep that procedure here and extend it with the logic added below, for readability. > (format #t "setting up setuid programs in '~a'...~%" > %setuid-directory) > (if (file-exists? %setuid-directory) > @@ -247,18 +240,33 @@ they already exist." > string (mkdir-p %setuid-directory)) > > - (for-each (lambda (program) > - (catch 'system-error > - (lambda () > - (make-setuid-program program)) > - (lambda args > - ;; If we fail to create a setuid program, better keep going > - ;; so that we don't leave %SETUID-DIRECTORY empty or > - ;; half-populated. This can happen if PROGRAMS contains > - ;; incorrect file names: . > - (format (current-error-port) > - "warning: failed to make '~a' setuid-root: ~a~%" > - program (strerror (system-error-errno args)))))) > + (for-each (match-lambda > + [('setuid-program src-path setuid? setgid? user group) ^ There's a convention to not use square brackets in the Guix code base, for uniformity. > + (let ((uid (match user > + [(? string?) (passwd:uid (getpwnam user))] > + [(? integer?) user])) > + (gid (match group > + [(? string?) (group:gid (getgrnam user))] > + [(? integer?) group]))) The above code raise an un-handled exception, for example if the user or group used doesn't exist. It should be moved to the above MAKE-SETUID-PROGRAM procedure and called inside the guard. > + (catch 'system-error > + (lambda () > + (let ((target (string-append %setuid-directory > + "/" (basename src-path))) > + (mode (+ #o0555 ; base permissions > + (if setuid? #o4000 0) ; setuid bit > + (if setgid? #o2000 0)))) ; setgid bit > + (copy-file src-path target) > + (chown target uid gid) > + (chmod target mode))) > + (lambda args > + ;; If we fail to create a setuid program, better keep going > + ;; so that we don't leave %SETUID-DIRECTORY empty or > + ;; half-populated. This can happen if PROGRAMS contains > + ;; incorrect file names: . > + (format (current-error-port) > + "warning: failed to make '~a' setuid-root: ~a~%" The above message should be adapted to say "failed to make ~s setuid/setgid: ~a~%" > + (setuid-program-program program) > + (strerror (system-error-errno args))))))]) > programs)) > > (define (activate-special-files special-files) > diff --git a/gnu/services.scm b/gnu/services.scm > index 4b30399adc..a5b4734152 100644 > --- a/gnu/services.scm > +++ b/gnu/services.scm > @@ -87,6 +87,14 @@ > ambiguous-target-service-error-service > ambiguous-target-service-error-target-type > > + setuid-program > + setuid-program? > + setuid-program-program > + setuid-program-setuid? > + setuid-program-setgid? > + setuid-program-user > + setuid-program-group > + > system-service-type > provenance-service-type > sexp->system-provenance > @@ -773,13 +781,48 @@ directory." > FILES must be a list of name/file-like object pairs." > (service etc-service-type files)) > > +(define-record-type* setuid-program make-setuid-program > + setuid-program? > + ;; Path to program to link with setuid permissions > + (program setuid-program-program) ;string > + ;; Whether to set user setuid bit > + (setuid? setuid-program-setuid? ;boolean > + (default #t)) > + ;; Whether to set user setgid bit > + (setgid? setuid-program-setgid? ;boolean > + (default #t)) This departs from the previous default (not setgid was set). It's probably more explicit to be set to #f as default, since the service is still named 'setuid-program-service-type', so having it do gid stuff by default could come as a surprise. > + ;; The user this should be set to (defaults to root) > + (user setuid-program-user ;integer or string > + (default 0)) > + ;; Group we want to set this to (defaults to root) > + (group setuid-program-group ;integer or string > + (default 0))) > +(define (setuid-program-entry programs) > + #~(activate-setuid-programs > + ;; convert into a tagged list structure as expected by > + ;; activate-setuid-programs > + (list #$@(map (match-lambda > + [(? setuid-program? sp) > + #~(list 'setuid-program > + #$(setuid-program-program sp) > + #$(setuid-program-setuid? sp) > + #$(setuid-program-setgid? sp) > + #$(setuid-program-user sp) > + #$(setuid-program-group sp))] > + ;; legacy, non- structure > + [program > + ;; TODO: Spit out a warning here? A deprecation message should be printed, urging the users to use the new interface, yes. > + #~(list 'setuid-program > + #$program > + #t #t 0 0)]) > + programs)))) > + > (define setuid-program-service-type > (service-type (name 'setuid-program) > (extensions > (list (service-extension activation-service-type > - (lambda (programs) > - #~(activate-setuid-programs > - (list #$@programs)))))) > + setuid-program-entry))) > (compose concatenate) > (extend append) > (description With the above comments, this looks like a good change to me! I haven't tested it yet, but intend to do so when I have a chance! Thank you for working on it, Maxim