Merge #14984: On POSIX, enforce permissions when reading default .netrc.

4 files changed, 59 insertions, 6 deletions

diff --git a/Doc/library/netrc.rst b/Doc/library/netrc.rst
index 323fd69..c693cea 100644
--- a/Doc/library/netrc.rst
+++ b/Doc/library/netrc.rst
@@ -25,6 +25,12 @@ the Unix :program:`ftp` program and other FTP clients.
    no argument is given, the file :file:`.netrc` in the user's home directory will
    be read.  Parse errors will raise :exc:`NetrcParseError` with diagnostic
    information including the file name, line number, and terminating token.
+   If no argument is specified on a POSIX system, the presence of passwords in
+   the :file:`.netrc` file will raise a :exc:`NetrcParseError` if the file
+   ownership or permissions are insecure (owned by a user other than the user
+   running the process, or accessible for read or write by any other user).
+   This implements security behavior equivalent to that of ftp and other
+   programs that use :file:`.netrc`.
 
 
 .. exception:: NetrcParseError
diff --git a/Lib/netrc.py b/Lib/netrc.py
index 0fd37e3..713e322 100644
--- a/Lib/netrc.py
+++ b/Lib/netrc.py
@@ -2,7 +2,7 @@
 
 # Module and documentation by Eric S. Raymond, 21 Dec 1998
 
-import os, shlex
+import os, stat, shlex, pwd
 
 __all__ = ["netrc", "NetrcParseError"]
 
@@ -21,6 +21,7 @@ class NetrcParseError(Exception):
 
 class netrc:
     def __init__(self, file=None):
+        default_netrc = file is None
         if file is None:
             try:
                 file = os.path.join(os.environ['HOME'], ".netrc")
@@ -29,9 +30,9 @@ class netrc:
         self.hosts = {}
         self.macros = {}
         with open(file) as fp:
-            self._parse(file, fp)
+            self._parse(file, fp, default_netrc)
 
-    def _parse(self, file, fp):
+    def _parse(self, file, fp, default_netrc):
         lexer = shlex.shlex(fp)
         lexer.wordchars += r"""!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~"""
         lexer.commenters = lexer.commenters.replace('#', '')
@@ -88,6 +89,26 @@ class netrc:
                 elif tt == 'account':
                     account = lexer.get_token()
                 elif tt == 'password':
+                    if os.name == 'posix' and default_netrc:
+                        prop = os.fstat(fp.fileno())
+                        if prop.st_uid != os.getuid():
+                            try:
+                                fowner = pwd.getpwuid(prop.st_uid)[0]
+                            except KeyError:
+                                fowner = 'uid %s' % prop.st_uid
+                            try:
+                                user = pwd.getpwuid(os.getuid())[0]
+                            except KeyError:
+                                user = 'uid %s ' % os.getuid()
+                            raise NetrcParseError(
+                                ("~/.netrc file owner (%s) does not match"
+                                 " current user (%s)") % (fowner, user),
+                                file, lexer.lineno)
+                        if (prop.st_mode & (stat.S_IRWXG | stat.S_IRWXO)):
+                            raise NetrcParseError(
+                               "~/.netrc access too permissive: access"
+                               " permissions must restrict access to only"
+                               " the owner", file, lexer.lineno)
                     password = lexer.get_token()
                 else:
                     raise NetrcParseError("bad follower token %r" % tt,
diff --git a/Lib/test/test_netrc.py b/Lib/test/test_netrc.py
index 2795456..4156c53 100644
--- a/Lib/test/test_netrc.py
+++ b/Lib/test/test_netrc.py
@@ -5,9 +5,6 @@ temp_filename = test_support.TESTFN
 
 class NetrcTestCase(unittest.TestCase):
 
-    def tearDown(self):
-        os.unlink(temp_filename)
-
     def make_nrc(self, test_data):
         test_data = textwrap.dedent(test_data)
         mode = 'w'
@@ -15,6 +12,7 @@ class NetrcTestCase(unittest.TestCase):
             mode += 't'
         with open(temp_filename, mode) as fp:
             fp.write(test_data)
+        self.addCleanup(os.unlink, temp_filename)
         return netrc.netrc(temp_filename)
 
     def test_default(self):
@@ -103,6 +101,28 @@ class NetrcTestCase(unittest.TestCase):
             """, '#pass')
 
 
+    @unittest.skipUnless(os.name == 'posix', 'POSIX only test')
+    def test_security(self):
+        # This test is incomplete since we are normally not run as root and
+        # therefore can't test the file ownership being wrong.
+        d = test_support.TESTFN
+        os.mkdir(d)
+        self.addCleanup(test_support.rmtree, d)
+        fn = os.path.join(d, '.netrc')
+        with open(fn, 'wt') as f:
+            f.write("""\
+                machine foo.domain.com login bar password pass
+                default login foo password pass
+                """)
+        with test_support.EnvironmentVarGuard() as environ:
+            environ.set('HOME', d)
+            os.chmod(fn, 0600)
+            nrc = netrc.netrc()
+            self.assertEqual(nrc.hosts['foo.domain.com'],
+                             ('bar', None, 'pass'))
+            os.chmod(fn, 0o622)
+            self.assertRaises(netrc.NetrcParseError, netrc.netrc)
+
 def test_main():
     test_support.run_unittest(NetrcTestCase)
 
diff --git a/Misc/NEWS b/Misc/NEWS
index c4698f4..d9b413d 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -32,6 +32,12 @@ Core and Builtins
 Library
 -------
 
+- Issue #14984: On POSIX systems, when netrc is called without a filename
+  argument (and therefore is reading the user's $HOME/.netrc file), it now
+  enforces the same security rules as typical ftp clients: the .netrc file must
+  be owned by the user that owns the process and must not be readable by any
+  other user.
+
 - Issue #17324: Fix http.server's request handling case on trailing '/'. Patch
   contributed by Vajrasky Kok.