On Wed, Jan 14, 2009 at 10:58:58AM -0800, Kees Cook wrote:
> On Wed, Jan 14, 2009 at 02:54:11PM +0000, Jonathan Davies wrote:
> > I've improved the error message so that it asks people to see the
> > manage-credentials manpage.
>
> Please make sure that the tool that creates the credentials stores them in
> a mode 0600 file. The API examples[1] do not mention this, and I think
> it's an important bit of protection.
>
> While playing with lplib for security team work, I took this a step
> further and even make the directory unreadable. e.g.:

er, I missed a rather important last line. Re-paste:

cachedir = os.path.expanduser('~/.launchpadlib/cache')
if not os.path.exists(cachedir):
os.makedirs(cachedir,0700)

credfile = os.path.expanduser('~/.launchpadlib/credentials')
try:
credentials = Credentials()
credentials.load(open(credfile))
launchpad = Launchpad(credentials, EDGE_SERVICE_ROOT, cachedir)
except:
launchpad = Launchpad.get_token_and_login(sys.argv[0], EDGE_SERVICE_ROOT, cachedir)
launchpad.credentials.save(open(credfile,"w",0600) )

--
Kees Cook
Ubuntu Security Team

--
Ubuntu-motu mailing list
Ubuntu-motu@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-motu

On Wed, Jan 14, 2009 at 10:58:58AM -0800, Kees Cook wrote:
> On Wed, Jan 14, 2009 at 02:54:11PM +0000, Jonathan Davies wrote:
> > I've improved the error message so that it asks people to see the
> > manage-credentials manpage.
>
> Please make sure that the tool that creates the credentials stores them in
> a mode 0600 file. The API examples[1] do not mention this, and I think
> it's an important bit of protection.
>
> While playing with lplib for security team work, I took this a step
> further and even make the directory unreadable. e.g.:

er, I missed a rather important last line. Re-paste:

cachedir = os.path.expanduser('~/.launchpadlib/cache')
if not os.path.exists(cachedir):
os.makedirs(cachedir,0700)

credfile = os.path.expanduser('~/.launchpadlib/credentials')
try:
credentials = Credentials()
credentials.load(open(credfile))
launchpad = Launchpad(credentials, EDGE_SERVICE_ROOT, cachedir)
except:
launchpad = Launchpad.get_token_and_login(sys.argv[0], EDGE_SERVICE_ROOT, cachedir)
launchpad.credentials.save(open(credfile,"w",0600) )

--
Kees Cook
Ubuntu Security Team

--
ubuntu-devel mailing list
ubuntu-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel

On Wed, Jan 14, 2009 at 11:18:35AM -0800, Kees Cook wrote:
> On Wed, Jan 14, 2009 at 10:58:58AM -0800, Kees Cook wrote:
> > Please make sure that the tool that creates the credentials stores them in
> > a mode 0600 file. The API examples[1] do not mention this, and I think
> > it's an important bit of protection.
> >
> > While playing with lplib for security team work, I took this a step
> > further and even make the directory unreadable. e.g.:
>
> er, I missed a rather important last line. Re-paste:
>
> cachedir = os.path.expanduser('~/.launchpadlib/cache')
> if not os.path.exists(cachedir):
> os.makedirs(cachedir,0700)
>
> credfile = os.path.expanduser('~/.launchpadlib/credentials')
> try:
> credentials = Credentials()
> credentials.load(open(credfile))
> launchpad = Launchpad(credentials, EDGE_SERVICE_ROOT, cachedir)
> except:
> launchpad = Launchpad.get_token_and_login(sys.argv[0], EDGE_SERVICE_ROOT, cachedir)
> launchpad.credentials.save(open(credfile,"w",0600) )

Isn't the third argument to Python's open() the buffer size, not the
file mode? That's what the documentation says, anyway.

$ python -c 'open("pyopentest", "w", 0600).close()'; ls -l pyopentest
-rw-r--r-- 1 cjwatson cjwatson 0 2009-01-15 00:19 pyopentest

I think you need:

launchpad = Launchpad.get_token_and_login(sys.argv[0], EDGE_SERVICE_ROOT, cachedir)
credfd = open(credfile, "w")
os.chmod(credfile, 0600)
launchpad.credentials.save(credfd)
credfd.close()

Python didn't have fchmod until 2.6 so this is a little awkward, but it
does the job.

(Thanks for the note about making the file non-world-readable, though;
I'd forgotten it entirely in a launchpadlib application I maintain.
Fixed.)

--
Colin Watson [cjwatson@ubuntu.com]

--
ubuntu-devel mailing list
ubuntu-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel

On Thu, Jan 15, 2009 at 12:23:13AM +0000, Colin Watson wrote:
> On Wed, Jan 14, 2009 at 11:18:35AM -0800, Kees Cook wrote:
> > launchpad.credentials.save(open(credfile,"w",0600) )
>
> Isn't the third argument to Python's open() the buffer size, not the
> file mode? That's what the documentation says, anyway.

UUuurg, this is what I get for switching languages so much. Thanks for
catching that. (Luckily my mkdir worked as expected, which makes the
latter goof less of an issue.)

> I think you need:
>
> launchpad = Launchpad.get_token_and_login(sys.argv[0], EDGE_SERVICE_ROOT, cachedir)
> credfd = open(credfile, "w")
> os.chmod(credfile, 0600)
> launchpad.credentials.save(credfd)
> credfd.close()
>
> Python didn't have fchmod until 2.6 so this is a little awkward, but it
> does the job.

Now I have to redeem my embarrassment by suggesting this as a way to avoid
the create/chmod race when fchmod is unavailable:

credfd = tempfile.NamedTemporaryFile(dir=os.path.dirname(cr edfile))
launchpad.credentials.save(credfd)
os.link(credfd.name, credfile)
credfd.close()

The link will fail if the target exists, and the close will clean up the
original file. Wheee

--
Kees Cook
Ubuntu Security Team

--
ubuntu-devel mailing list
ubuntu-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel

Hi,

On Thursday 15 January 2009 01:48:44 Kees Cook wrote:
[..]
>
> Now I have to redeem my embarrassment by suggesting this as a way to avoid
> the create/chmod race when fchmod is unavailable:
>
> credfd = tempfile.NamedTemporaryFile(dir=os.path.dirname(cr edfile))
> launchpad.credentials.save(credfd)
> os.link(credfd.name, credfile)
> credfd.close()
>
> The link will fail if the target exists, and the close will clean up the
> original file. Wheee

how about simply using os.open() to avoid the race in the first place?
Or mangle the umask to the right thing before the (fd)open?

Cheers,
Stefan.

--
ubuntu-devel mailing list
ubuntu-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel

2009/1/14 Kees Cook <kees@ubuntu.com>:
> Please make sure that the tool that creates the credentials stores them in
> a mode 0600 file. The API examples[1] do not mention this, and I think
> it's an important bit of protection.
>
> While playing with lplib for security team work, I took this a step
> further and even make the directory unreadable. e.g.:

Absolutely, this change has been pushed as of revision 276.

Jonathan

--
Ubuntu-motu mailing list
Ubuntu-motu@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-motu

2009/1/14 Kees Cook <kees@ubuntu.com>:
> Please make sure that the tool that creates the credentials stores them in
> a mode 0600 file. The API examples[1] do not mention this, and I think
> it's an important bit of protection.
>
> While playing with lplib for security team work, I took this a step
> further and even make the directory unreadable. e.g.:

Absolutely, this change has been pushed as of revision 276.

Jonathan

--
ubuntu-devel mailing list
ubuntu-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel