Closed Bug 1127449 Opened 8 years ago Closed 7 years ago

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

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
Tracking Status
firefox44 --- fixed

People

(Reporter: armenzg, Assigned: armenzg)

Details

(Whiteboard: [easier-mozharness])

Attachments

(2 files, 2 obsolete files)

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.
Whiteboard: [easier-mozharness]
I would be happy with --always-prompt.
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
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
Assignee: nobody → armenzg
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.
Remove old format file
Attachment #8664989 - Attachment is obsolete: true
Attachment #8664990 - Attachment is obsolete: true
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.
Attachment #8666089 - Flags: review?(sphink)
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 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.
Updated docstring
Attachment #8666687 - Flags: review?(sphink)
Comment on attachment 8666687 [details]
MozReview Request: Updated docstring

https://reviewboard.mozilla.org/r/20595/#review18515
Attachment #8666687 - Flags: review?(sphink) → review+
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
Attachment #8666089 - Flags: review?(sphink) → review+
You need to log in before you can comment on or make changes to this bug.