1127449 - For developer mode, do not store the credentials in plain text if the developer does not want to

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Armen [:armenzg]]

In general, storing credentials in plain text is something I have seen used across different tools.

I had filed a bug to investigate how to encrypt it on disk and then unlock it with a pass-phrase.

gbrown: what do you suggest?

I thought of adding an optional parameter like --always-prompt or the passphrase approach.

Comment 1

[Geoff Brown [:gbrown] (pto Apr 11-18)]

I would be happy with --always-prompt.

Comment 2

[Steve Fink [:sfink] [:s:]]

Heh. I was just filing a duplicate bug when I found this one. I'll just paste my comment in here instead:

One thing I noticed that disturbed me when looking into tooltool authentication was that it ends up storing an LDAP password in the clear in a file ~/.mozilla/credentials.cfg. It's in a file with mode 0600, but it still worries me.

In my naive mental model of security, a password should never be stored on disk in the clear, anywhere. In fact, it shouldn't be stored in an encrypted form if no secret is needed to decrypt it ("secret" == "something you can't get from reading the source code"), since that's almost as bad.

I know that if someone can get access to an 0600 file on my hard drive, then they can probably install a keylogger or something to get my password for real. But I'd still like to know if whatever that credentials.cfg is being used for, it could use a revocable API key or something that only gives access to what is needed.

The main relevant file here is mozharness/mozharness/lib/python/authentication.py

Comment 3

[Armen [:armenzg]]

We should not store the password in plain text.

I added some improvements to mozci by using keyring and prompted the developer to ask to store their password encrypted.

I would have to tell the developer to install keyring for developer mode OR add it to mozharness as a dependency.

https://hg.mozilla.org/build/mozharness/file/default/mozharness/lib/python/authentication.py
https://github.com/armenzg/mozilla_ci_tools/blob/master/mozci/utils/authentication.py

Comment 4

[Armen [:armenzg]]

Attachment: MozReview Request: Bug 1127449 - For Mozharness' developer's mode do not store the LDAP password unencrypted

Bug 1127449 - For Mozharness' developer's mode do not store the LDAP password unencrypted

In Mozharness we support a developer mode which is capable of downloading
artifacts from the Release Engineering LDAP protected artifacts.

The credentials are stored for developers convenience unencrypted in a plain
text. This is not wanted by most developers.

In this patch we make sure that the password is prompted of the user once but
we do not store on disk.

Comment 5

[Armen [:armenzg]]

Attachment: MozReview Request: Remove old format file

Remove old format file

Comment 6

[Armen [:armenzg]]

Attachment: MozReview Request: Bug 1127449 - For Mozharness' developer's mode do not store the LDAP password unencrypted

Bug 1127449 - For Mozharness' developer's mode do not store the LDAP password unencrypted

In Mozharness we support a developer mode which is capable of downloading
artifacts from the Release Engineering LDAP protected artifacts.

The credentials are stored for developers convenience unencrypted in a plain
text. This is not wanted by most developers.

In this patch we make sure that the password is prompted of the user once but
we do not store on disk.

Comment 7

[Steve Fink [:sfink] [:s:]]

https://reviewboard.mozilla.org/r/20433/#review18351

That certainly resolves the problem, at a potential cost in nuisance. How often will people end up needing to type in their passwords? I guess it's only in developer mode. This won't prompt for a password when it isn't needed, right?

::: testing/mozharness/mozharness/lib/python/authentication.py:16
(Diff revision 1)
>      """ Returns credentials for http access either from
>      disk or directly from the user (which we store)
>      """

This comment makes sense with the old version, where username+password were treated as a unit, but now it's hard to tell whether it's saying your password will be stored or not. Update the comment.

Comment 8

[Armen [:armenzg]]

Comment on attachment 8666089 [details]
MozReview Request: Bug 1127449 - For Mozharness' developer's mode do not store the LDAP password unencrypted

Bug 1127449 - For Mozharness' developer's mode do not store the LDAP password unencrypted

In Mozharness we support a developer mode which is capable of downloading
artifacts from the Release Engineering LDAP protected artifacts.

The credentials are stored for developers convenience unencrypted in a plain
text. This is not wanted by most developers.

In this patch we make sure that the password is prompted of the user once but
we do not store on disk.

Comment 9

[Armen [:armenzg]]

Attachment: MozReview Request: Updated docstring

Updated docstring

Comment 10

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8666687 [details]
MozReview Request: Updated docstring

https://reviewboard.mozilla.org/r/20595/#review18515

Comment 11

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8666089 [details]
MozReview Request: Bug 1127449 - For Mozharness' developer's mode do not store the LDAP password unencrypted

https://reviewboard.mozilla.org/r/20435/#review18517

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d7853450e72

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2d7853450e72