From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS4713 221.184.0.0/13 X-Spam-Status: No, score=-3.6 required=3.0 tests=AWL,BAYES_00, FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_PASS, UNPARSEABLE_RELAY shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by dcvr.yhbt.net (Postfix) with ESMTP id 4D93E1F9E0 for ; Fri, 24 Apr 2020 10:41:01 +0000 (UTC) Received: from neon.ruby-lang.org (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id 61F90120999; Fri, 24 Apr 2020 19:40:35 +0900 (JST) Received: from o1678948x4.outbound-mail.sendgrid.net (o1678948x4.outbound-mail.sendgrid.net [167.89.48.4]) by neon.ruby-lang.org (Postfix) with ESMTPS id 747E7120996 for ; Fri, 24 Apr 2020 19:40:32 +0900 (JST) Received: by filterdrecv-p3iad2-8ddf98858-z54vx with SMTP id filterdrecv-p3iad2-8ddf98858-z54vx-21-5EA2C232-10 2020-04-24 10:40:50.101194021 +0000 UTC m=+2540000.583833835 Received: from herokuapp.com (unknown) by ismtpd0028p1iad2.sendgrid.net (SG) with ESMTP id Js4eLfcpRYGBGfzny94wiw for ; Fri, 24 Apr 2020 10:40:50.046 +0000 (UTC) Date: Fri, 24 Apr 2020 10:40:50 +0000 (UTC) From: salewski@att.net Message-ID: References: Mime-Version: 1.0 X-Redmine-MailingListIntegration-Message-Ids: 73817 X-Redmine-Project: ruby-master X-Redmine-Issue-Tracker: Bug X-Redmine-Issue-Id: 16787 X-Redmine-Issue-Author: salewski X-Redmine-Sender: salewski X-Mailer: Redmine X-Redmine-Host: bugs.ruby-lang.org X-Redmine-Site: Ruby Issue Tracking System X-Auto-Response-Suppress: All Auto-Submitted: auto-generated X-SG-EID: =?us-ascii?Q?ukLggO+EF8qbI7KFMNUTPbIRPpqU=2FeDDj1=2FvUFO5YnKLNhOhxVxsdko6dpxWRQ?= =?us-ascii?Q?GTY1E=2F=2FiSqLvyimQvSNf0ewZtSn5YpJ50w0cPoS?= =?us-ascii?Q?S8dZp7vLnMmv4j7X0D9KPTj7FSjqWblXMyt+Rv3?= =?us-ascii?Q?rby9vR6nw+l4L1jOfOE2Uu0DwMEJ=2FfrxZ+8tyx+?= =?us-ascii?Q?6gfnL0nybLSLNBk8oCUhyJ7=2FLjR++8blB3QXmTI?= =?us-ascii?Q?M32cv91O+TLgGCajs=3D?= To: ruby-core@ruby-lang.org X-ML-Name: ruby-core X-Mail-Count: 98056 Subject: [ruby-core:98056] [Ruby master Bug#16787] [patch] allow Dir.home to work for non-login procs when $HOME not set X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" Issue #16787 has been updated by salewski (Alan Salewski). ruby -v changed from ruby 2.8.0dev (2020-04-23T10:11:21Z ads/b.r-l.o-issue-.. 5369b67fc8) [x86_64-linux] to ruby 2.8.0dev (2020-04-24T10:02:57Z ads/b.r-l.o-issue-.. 66fa7717ab) [x86_64-linux] File allow-dir.home-for-non-login-procs-v7.patch added Attaching version "v7" of the patch. This version corresponds with the changes I just push on my [ads/b.r-l.o-issue-16787](https://github.com/salewski/ruby/tree/ads/b.r-l.o-issue-16787) branch for [PR 3034](https://github.com/ruby/ruby/pull/3034): * https://github.com/salewski/ruby/commit/66fa7717ab8c2d37042866cddf3fcf38d0095f99 * https://github.com/ruby/ruby/commit/66fa7717ab8c2d37042866cddf3fcf38d0095f99 This one is a candidate for further review and/or merging. This one builds on the earlier version of the patch, and moves the new pwd.h related functions from file.c to process.c This patch has been generated of a branch that was rebased on top of the 'master' branch within the last 30 minutes. @nabu: This variation incorporates the final outstanding recommendation of the feedback you provided (thanks for that!) on 2020-04-17[0][1], which was to look into moving the new functions into process.c rather than putting them in file.c. It turns out that file.c was already including functionality from process.c, so this change does not add a new dependency between the files. It does widen the visibility of the three new functions, as they are now declared in the `internal/process.h` header: VALUE rb_getlogin(void); VALUE rb_getpwdirnam_for_login(VALUE login); VALUE rb_getpwdiruid(void); Note that I changed the signature of the new `rb_getpwdirnam_for_login(...)` to accept the login name as a parameter. The reason is that the single calling location from `rb_default_home_dir(...)` in `file.c has historical behavior of raising an exception with the message: "couldn't find login name -- expanding `~'" While it is a corner case, there is one scenario in which it would still be more appropriate for the code to emit that message than the newly introduced message that mentions the uid: if the attempt to find the login name failed (either because the system doesn't have `getlogin_r()` or `getlogin()`, or because the process is not a descendant of login) **AND** the system (for whatever reason) has `pwd.h` but does not have either `getpwuid_r()` or `getpwuid()`. So yeah, a corner case -- but theoretically possible. Note that this change cannot be cleanly applied to the `ruby_2_7` branch because the `internal/process.h` file does not exist on that branch (looks like it was introduced more recently). However, the three function definitions can easily be lifted out of `internal/process.h` from the branch and added to ruby.h (next to the other process.c functions, such as `rb_last_status_clear(...)`), so it wouldn't be too much work to cherry-pick it with minor modifications. As with prior versions, I've tested these changes with all the various combinations of the six `getlogin*()`, `getpwnam*()`, and `getpwuid*()` functions. I have also tested it with faked-up scenarios of `getlogin_r()` and `getlogin()` returning `NULL` to verify the backward compat code path mentioned above. [0] https://bugs.ruby-lang.org/issues/16787#note-10 [1] https://github.com/ruby/ruby/pull/3034#pullrequestreview-395521033 ---------------------------------------- Bug #16787: [patch] allow Dir.home to work for non-login procs when $HOME not set https://bugs.ruby-lang.org/issues/16787#change-85284 * Author: salewski (Alan Salewski) * Status: Open * Priority: Normal * ruby -v: ruby 2.8.0dev (2020-04-24T10:02:57Z ads/b.r-l.o-issue-.. 66fa7717ab) [x86_64-linux] * Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN ---------------------------------------- The 'Dir.home' method in versions of Ruby 2.x through the latest (2.7.1, released 2020-03-31) is unable to reliably locate the user's home directory when all three of the following are true at the same time: 1. Ruby is running on a Unix-like OS 2. The $HOME environment variable is not set 3. The process is not a descendant of login(1) (or a work-alike) When the above conditions are met, the condition can be triggered simply: $ unset HOME $ ruby -e print "home is: #{Dir.home}\n"; -e:1:in `home': couldn't find login name -- expanding `~' (ArgumentError) from -e:1:in `
' The expectation is that Dir.home should be able to obtain the user's default home directory regardless of whether or not the process is a (grand)child of login(1). This behavior surfaced when running unit tests on GitHub Actions, where the driving process did not use a login session. The unit tests failed due to the different behavior of Dir.home in this scenario, but Dir.home ought to behave the same either way. The actual observed behavior is that Dir.home is able to obtain the user's default home directory only for processes that are (grand)children of login(1). This behavior has been confirmed directly on (at least) the following versions, though it is clear from browsing the code that this is long standing behavior: $ ruby --version ruby 2.5.5p157 (2019-03-15 revision 67260) [x86_64-linux-gnu] $ ruby --version ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-linux] $ ruby --version ruby 2.8.0dev (2020-04-15T07:06:48Z master 69b3e0ac59) [x86_64-linux] On a Unix-like OS, when the $HOME environment variable is not set, Ruby attempts to obtain the user's home directory from the password database, as one would expect. But the mechanism it uses only works for (grand)children of login(1) (or work-alikes). In particular, it uses getlogin(3) to obtain the username, with the intent to then obtain the user's password record (and its 'pw_dir' member) by looking it up by name (getpwnam(3)). That getlogin() call fails, of course, because there is no logged-in user for the process. The attached patch preserves the basic intent of the existing code, but allows it work in the above scenario because the lookup for the user's record in the password database is done directly by uid (getpwuid_r(3)), which is always available, regardless of whether or not the process was launched by a subprocess of login(1). The patch applies cleanly against the HEAD of both 'master' and 'ruby_2_7', and was tested against both on Debian GNU/Linux (buster/bullseye mix). Motivation ---------- This issue [surfaced this past week](https://github.com/heroku/netrc/issues/50) in the [heroku/netrc](https://github.com/heroku/netrc) project when CI builds were first setup for the project using the GitHub Actions service. The process that runs the unit tests there is not a (grand)child of login(1), so failed on unit tests that exercise logic in that library when the $HOME environment variable is not set (changing its value and/or unsetting it are legitimate user activities; the tests were exercising that legitimate code path). How to reproduce ---------------- In order to reproduce the issue you need to get some startup daemon process to launch your ruby program; triggering the issue will not work for the ruby process to be a subprocess of any process that is itself a (grand)child of a login process. The GitHub Actions service happens to run code that way (see above issue link for an example), but it can be simulated locally fairly easily, too, using atd(8). A process that is not a (grand)child of login(1) will not have its 'loginuid' attribute set, so there will discrepancy between the values reported by id(1) and the never-initialized value in '/proc/self/loginuid': $ /usr/bin/id uid=1001(runner) gid=115(docker) groups=115(docker) $ /usr/bin/id --user 1001 /usr/bin/getent passwd 1001 runner:x:1001:115:,,,:/home/runner:/bin/bash $ cat /proc/self/loginuid 4294967295 Note that '4294967295' is the largest unsigned value that will fit in 32 bits, so it's signed value interpretation is '-1'. A 'loginuid' attribute with that value is an indication that it has never been set. In a typical configuration, it would be set as a side effect of the login process by PAM (see pam_loginuid(8)). The out-of-the-box 'atd(8)' configuration on Debian is also configured to have PAM account for the 'loginuid' attribute, but for the purpose of testing the fix for this issue, it can be easily disabled by editing the '/etc/pam.d/atd' file. Find the line that looks like this: session required pam_loginuid.so and comment it out so it looks like this: #session required pam_loginuid.so That change will take effect as soon as you save the file; there is no need to restart any services or anything like that. To test the before and after behaviors, I simply ran a pristine and a patched version of the code side-by-side, indirectly via at(1). $ cat /tmp/algo-doit2 #!/bin/bash - set -x my_log_fpath='/tmp/algo-doit2.log' #RUBY_UNPATCHED='/usr/bin/ruby2.5' RUBY_UNPATCHED='/tmp/aljunk-ruby-from-git/bin/ruby' #RUBY_PATCHED='/tmp/aljunk-ruby-from-git-patched/bin/ruby' RUBY_PATCHED='/tmp/aljunk-ruby-from-git-patched-master/bin/ruby' ( set -x /usr/bin/id /usr/bin/id --user printf '%s\n' $(cat /proc/self/loginuid) : DEBUG 1 unpatched: good "${RUBY_UNPATCHED}" -e 'print "home is: #{Dir.home}\n";' : : DEBUG 2 unpatched: now bad unset HOME "${RUBY_UNPATCHED}" -e 'print "home is: #{Dir.home}\n";' ) 1>> "${my_log_fpath}" 2>&1 ( set -x /usr/bin/id /usr/bin/id --user printf '%s\n' $(cat /proc/self/loginuid) : DEBUG 3 patched: good "${RUBY_PATCHED}" -e 'print "home is: #{Dir.home}\n";' : : DEBUG 4 patched: still good unset HOME "${RUBY_PATCHED}" -e 'print "home is: #{Dir.home}\n";' ) 1>> "${my_log_fpath}" 2>&1 For best results, run 'tail -F' on the output log in the background in your shell: $ tail -F /tmp/algo-doit2.log & With that setup, now each time you run the at(1) command you'll see the output (from the log file) right away: $ at now < /tmp/algo-doit2 warning: commands will be executed using /bin/sh job 17 at Wed Apr 15 06:04:00 2020 + set -x + /usr/bin/id uid=1000(someuser) gid=1000(someuser) groups=1000(someuser) + /usr/bin/id --user 1000 + cat /proc/self/loginuid + printf %s\n 4294967295 4294967295 + : DEBUG 1 unpatched: good + /tmp/aljunk-ruby-from-git/bin/ruby -e print "home is: #{Dir.home}\n"; home is: /home/someuser + : + : DEBUG 2 unpatched: now bad + unset HOME + /tmp/aljunk-ruby-from-git/bin/ruby -e print "home is: #{Dir.home}\n"; -e:1:in `home': couldn't find login name -- expanding `~' (ArgumentError) from -e:1:in `
' + set -x + /usr/bin/id uid=1000(someuser) gid=1000(someuser) groups=1000(someuser) + /usr/bin/id --user 1000 + cat /proc/self/loginuid + printf %s\n 4294967295 4294967295 + : DEBUG 3 patched: good + /tmp/aljunk-ruby-from-git-patched-master/bin/ruby -e print "home is: #{Dir.home}\n"; home is: /home/someuser + : + : DEBUG 4 patched: still good + unset HOME + /tmp/aljunk-ruby-from-git-patched-master/bin/ruby -e print "home is: #{Dir.home}\n"; home is: /home/someuser After testing, be sure to restore your atd(8) PAM configuration. Legal ----- I agree that the code in the attached patch may be distributed and/or modified under Ruby's License. Related Bugs ------------ Bug #12226 seems as if it might be related "in spirit", but that bug is specific to MS Windows, and the current issue (and patch) is specific to Unix-like systems. "Dir.home with valid named user raises ArgumentError on Windows" https://bugs.ruby-lang.org/issues/12226 ---Files-------------------------------- allow-dir.home-for-non-login-procs.patch (2.79 KB) allow-dir.home-for-non-login-procs-v2.patch (4.52 KB) allow-dir.home-for-non-login-procs-v3.patch (16.8 KB) allow-dir.home-for-non-login-procs-v4.patch (11.9 KB) allow-dir.home-for-non-login-procs-v5.patch (12.6 KB) allow-dir.home-for-non-login-procs-v6.patch (11.6 KB) allow-dir.home-for-non-login-procs-v7.patch (14.6 KB) -- https://bugs.ruby-lang.org/