Bazaar

Merge lp:~joke/bzr/file_permissions_authentication.conf into lp:bzr

  1. file_permissions_authentication.conf
  2. Merge into bzr.dev
Proposed by John A Meinel
Status: Work in progress
Proposed branch: lp:~joke/bzr/file_permissions_authentication.conf
Merge into: lp:bzr
Diff against target: 52 lines (+19/-0) (has conflicts)
1 file modified
bzrlib/config.py (+19/-0)
Text conflict in bzrlib/config.py
To merge this branch: bzr merge lp:~joke/bzr/file_permissions_authentication.conf
Reviewer Review Type Date Requested Status
Vincent Ladeuil Needs Information
Review via email: mp+58666@code.launchpad.net

Commit message

Bug #475501, create authentication.conf with mode 0600

Description of the change

We currently don't do much fancy with authentication.conf permissions. Joke de Buhr reasonably pointed out that we could create the file as 600 and avoid defaulting to having your passwords accessible by other users on the system.

I haven't looked over the patch at all, just trying to get it out of the In Progress queue and get a decision on it.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

I wonder how this will do on Windows? It seems to rely on Unix modes, and it would be nice to add some tests.

Reply
Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 5/3/2011 1:45 AM, Jelmer Vernooij wrote:
> I wonder how this will do on Windows? It seems to rely on Unix modes, and it would be nice to add some tests.

The attributes are all available on Windows. Probably the big concern is
that it is a bit hard to set the modes reliably, and then the warning
will occur repeatedly.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2/8IgACgkQJdeBCYSNAAONQwCfYs+M6SRAQ23cjkMe51+7EMTD
wRsAn0rJ1SE+hDQMK7XlKWmGkfc6MhSk
=vrH1
-----END PGP SIGNATURE-----

Reply
Revision history for this message
Vincent Ladeuil (vila) wrote :

The approach I've seen used in other contexts for such sensible informations seems to be:
- create the file with user-only persmissions,
- refuse to use it if others (even group) can read it

That's not exactly the approach taken here.

Now, I think windows doesn't provide this (user-only access) easily so we may just don't check there.

So what do others think ? Should we:

- create with 0600,
- warn if group or other have read access (not on windows)

or

- create with 0600
- refuse to use the file if group or other have read access (not on windows)

Something else ?

I'm fine with finishing this proposal if we reach a consensus, in the mean time, I'll mark this as wip.

review: Needs Information
Reply
Revision history for this message
Vincent Ladeuil (vila) wrote :

I forgot:

27 + if not GlobalConfig().get_user_option_as_bool(
28 + 'no_insecure_permissions_warning') :

We have a suppress_warnings config option that should be used here instead of introducing a new one (it's a list of warning names that should not be displayed, see bzrlib.config.Config.suppress_warning.

Reply

Unmerged revisions

4791. By Joke de Buhr

Don't emit warning message if the option 'no_insecure_permissions_warning'
is set to 'True' in bazaar.conf [DEFAULT]

4790. By Joke de Buhr

Emit warning message if an authentication.conf file hase insecure file permissions.

4789. By Joke de Buhr

create authentication.conf with umask 166 (permissions: u=rw,go=)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bzrlib/config.py'
--- bzrlib/config.py 2011-04-09 20:01:11 +0000
+++ bzrlib/config.py 2011-04-21 13:18:39 +0000
@@ -65,6 +65,7 @@
65import os65import os
66import string66import string
67import sys67import sys
68import stat
6869
69from bzrlib import commands70from bzrlib import commands
70from bzrlib.decorators import needs_write_lock71from bzrlib.decorators import needs_write_lock
@@ -1591,6 +1592,7 @@
1591 if _file is None:1592 if _file is None:
1592 self._filename = authentication_config_filename()1593 self._filename = authentication_config_filename()
1593 self._input = self._filename = authentication_config_filename()1594 self._input = self._filename = authentication_config_filename()
1595 self._check_permissions()
1594 else:1596 else:
1595 # Tests can provide a string as _file1597 # Tests can provide a string as _file
1596 self._filename = None1598 self._filename = None
@@ -1611,15 +1613,32 @@
1611 raise errors.ParseConfigError(e.errors, e.config.filename)1613 raise errors.ParseConfigError(e.errors, e.config.filename)
1612 return self._config1614 return self._config
16131615
1616 def _check_permissions(self):
1617 """Check permission of auth file are user read/write able only."""
1618 mode = stat.S_IMODE(os.stat(self._filename).st_mode)
1619 if not GlobalConfig().get_user_option_as_bool(
1620 'no_insecure_permissions_warning') :
1621 if ( ( stat.S_IXOTH | stat.S_IWOTH | stat.S_IROTH | stat.S_IXGRP |
1622 stat.S_IWGRP | stat.S_IRGRP ) & mode ) > 0 :
1623 trace.warning("The file '" + self._filename + "' has insecure "
1624 "file permissions. Saved passwords may be accessible "
1625 "by other users.")
1626
1614 def _save(self):1627 def _save(self):
1615 """Save the config file, only tests should use it for now."""1628 """Save the config file, only tests should use it for now."""
1616 conf_dir = os.path.dirname(self._filename)1629 conf_dir = os.path.dirname(self._filename)
1617 ensure_config_dir_exists(conf_dir)1630 ensure_config_dir_exists(conf_dir)
1631<<<<<<< TREE
1618 f = file(self._filename, 'wb')1632 f = file(self._filename, 'wb')
1619 try:1633 try:
1620 self._get_config().write(f)1634 self._get_config().write(f)
1621 finally:1635 finally:
1622 f.close()1636 f.close()
1637=======
1638 old_umask = os.umask(0177)
1639 self._get_config().write(file(self._filename, 'wb'))
1640 os.umask(old_umask)
1641>>>>>>> MERGE-SOURCE
16231642
1624 def _set_option(self, section_name, option_name, value):1643 def _set_option(self, section_name, option_name, value):
1625 """Set an authentication configuration option"""1644 """Set an authentication configuration option"""