Merge lp:~florian-dorn/bzr/Base64CredentialStore into lp:bzr

[Florian Dorn]

Implements a base64 credential store (fixes bug #788015), for obfruscating passwords in authentication.conf

[Martin Pool (mbp)]

Hi, Florian, thanks for this contribution.

Could you please complete <http://www.canonical.com/contributors>?

[question] I wonder how people will discover they can use this. It seems to me we ought to update at least the developer docs that say this is not implemented; add it to news and whatsnew; also perhaps add it into the user guide about authentication. Perhaps this should even become the default?

+ __doc__ = """Base64 credential store for the authentication.conf file"""

[fix] A literal string occurring as the first thing in a class or other block is automatically the docstring in Python.

[tweak] Could you please mention here that the point of this class is to obfuscate the passwords stored in the file?

[fix] This ought to be tested; there is a test_decode_password and TestPlainTextCredentialStore in tests/test_config.py. If you're having trouble adding one just ask and we will help or do it.

I wonder how people are going to discover that this exists? I think

review: Needs Fixing

[John A Meinel (jameinel)]

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

On 6/6/2011 4:46 AM, Martin Pool wrote:
> Review: Needs Fixing
> Hi, Florian, thanks for this contribution.
>
> Could you please complete <http://www.canonical.com/contributors>?
>
> [question] I wonder how people will discover they can use this. It seems to me we ought to update at least the developer docs that say this is not implemented; add it to news and whatsnew; also perhaps add it into the user guide about authentication. Perhaps this should even become the default?
>
> + __doc__ = """Base64 credential store for the authentication.conf file"""

>
> [fix] A literal string occurring as the first thing in a class or other block is automatically the docstring in Python.

We use "__doc__" for Command classes because -OO doesn't strip it out.
We might be using it similarly here. If a doc string ends up in 'bzr
help' text, we want to use __doc__ = ...

>
> [tweak] Could you please mention here that the point of this class is to obfuscate the passwords stored in the file?
>
> [fix] This ought to be tested; there is a test_decode_password and TestPlainTextCredentialStore in tests/test_config.py. If you're having trouble adding one just ask and we will help or do it.
>
> I wonder how people are going to discover that this exists? I think

Would it show up under "bzr help authentication"? If not, it probably
should.

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

iEYEARECAAYFAk3s43YACgkQJdeBCYSNAAOKHACeN0ZHUveykAFo9dckb8e0tkMm
RP4AoNiJSYE7DR8dJx1zD+1QB+pGkKjb
=D6By
-----END PGP SIGNATURE-----

[Vincent Ladeuil (vila)]

@Florian: Do you need help here ?

How do you use this store ? You put your encoded credentials into authentication.conf manually ?

review: Needs Information

[Florian Dorn (florian-dorn)]

@Vincent:
the base64 encoded password can be generated with:
echo -n "secret" | base64
and put there manually

[Martin Pool (mbp)]

On 7 June 2011 00:25, John Arbash Meinel <email address hidden> wrote:

>> [question] I wonder how people will discover they can use this.  It seems to me we ought to update at least the developer docs that say this is not implemented; add it to news and whatsnew; also perhaps add it into the user guide about authentication.  Perhaps this should even become the default?
>>
>> +    __doc__ = """Base64 credential store for the authentication.conf file"""
>
>
>>
>> [fix] A literal string occurring as the first thing in a class or other block is automatically the docstring in Python.
>
> We use "__doc__" for Command classes because -OO doesn't strip it out.
> We might be using it similarly here. If a doc string ends up in 'bzr
> help' text, we want to use __doc__ = ...

[aside] Right, but if this is going to be user documentation it ought
to be written in a different way, describing how to use it, not what
the class is. The parent AuthenticationConfig doesn't declare an
explicity __doc__. Perhaps we should move away from that to say
instead help="""..."""

[fix] At the moment there is "bzr help authentication" apparently
taken from a static file, and that ought to be updated to explain how
to use it.

[Vincent Ladeuil (vila)]

> @Vincent:
> the base64 encoded password can be generated with:
> echo -n "secret" | base64
> and put there manually

Great, that's worth adding to the doc (you need to specify password_encoding=base64 too right ?).

I'll put the proposal in the 'Work In Progress' state. When you want it to be reviewed again, just push your new changes to lp and put the proposal back to the 'Needs Review' state.

[Florian Dorn (florian-dorn)]

@All:
I finally have added the test and updated the developer docs. Sorry for resubmitting the bug, I guess changing the status would have done the job.

[Martin Packman (gz)]

This is simple enough that it should just go in config rather than needing a plugin. I'll go ahead and so that when I get the chance.

review: Needs Fixing

=== added directory 'bzrlib/plugins/base64_credential_store'
=== added file 'bzrlib/plugins/base64_credential_store/__init__.py'
--- bzrlib/plugins/base64_credential_store/__init__.py	1970-01-01 00:00:00 +0000
+++ bzrlib/plugins/base64_credential_store/__init__.py	2012-04-12 12:57:23 +0000
@@ -0,0 +1,50 @@
+# Copyright (C) 2008-2011 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+__doc__ = """Base64 credential store for the authentication.conf file"""
+# Since we are a built-in plugin we share the bzrlib version
+from bzrlib import version_info
+
+from bzrlib import (
+    config,
+    lazy_import,
+    )
+
+lazy_import.lazy_import(globals(), """
+import errno
+import base64
+from bzrlib import (
+    errors,
+    )
+""")
+
+class Base64CredentialStore(config.CredentialStore):
+    
+    def decode_password(self, credentials):
+        """See CredentialStore.decode_password."""
+        return base64.decodestring(credentials['password'])
+
+config.credential_store_registry.register_lazy('base64', __name__, 'Base64CredentialStore', help=__doc__)
+
+
+
+def load_tests(basic_tests, module, loader):
+    testmod_names = [
+        'tests',
+        ]
+    basic_tests.addTest(loader.loadTestsFromModuleNames(
+            ["%s.%s" % (__name__, tmn) for tmn in testmod_names]))
+    return basic_tests

=== added directory 'bzrlib/plugins/base64_credential_store/tests'
=== added file 'bzrlib/plugins/base64_credential_store/tests/__init__.py'
--- bzrlib/plugins/base64_credential_store/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ bzrlib/plugins/base64_credential_store/tests/__init__.py	2012-04-12 12:57:23 +0000
@@ -0,0 +1,23 @@
+# Copyright (C) 2008 by Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+def load_tests(basic_tests, module, loader):
+    testmod_names = [
+        'test_base64',
+        ]
+    basic_tests.addTest(loader.loadTestsFromModuleNames(
+            ["%s.%s" % (__name__, tmn) for tmn in testmod_names]))
+    return basic_tests

=== added file 'bzrlib/plugins/base64_credential_store/tests/test_base64.py'
--- bzrlib/plugins/base64_credential_store/tests/test_base64.py	1970-01-01 00:00:00 +0000
+++ bzrlib/plugins/base64_credential_store/tests/test_base64.py	2012-04-12 12:57:23 +0000
@@ -0,0 +1,37 @@
+# Copyright (C) 2008 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+from cStringIO import StringIO
+
+from bzrlib import (
+    config,
+    errors,
+    osutils,
+    tests,
+    )
+
+from bzrlib.plugins import base64_credential_store
+
+
+class TestBase64CredentialStore(tests.TestCase):
+
+    def test_decode_password(self):
+        import base64
+        r = config.credential_store_registry
+        plain_text = r.get_credential_store('base64')
+        decoded = plain_text.decode_password(dict(password=base64.encodestring('secret-pass')))
+        self.assertEquals('secret-pass', decoded)
+

=== modified file 'doc/developers/authentication-ring.txt'
--- doc/developers/authentication-ring.txt	2010-11-12 22:46:28 +0000
+++ doc/developers/authentication-ring.txt	2012-04-12 12:57:23 +0000
@@ -158,7 +158,7 @@
 
 Encoding passwords in ``base64``, while weak, provides protection against
 accidental reading (if an administrator have to look into the file, he will not
-see the passwords in clear).(Not implemented yet).
+see the passwords in clear).
 
 This specification intends to ease the authentication providing, not to secure
 it in the best possible way.
@@ -266,6 +266,7 @@
         scheme=https
         host=home.net
         user=joe
+        # Obtain the base64 encoded password by running 'echo -n "secret-pass" | base64'
         password='c2VjcmV0LXBhc3M='
         password_encoding=base64
         verify_certificates=no # Still searching a free certificate provider