authentication.conf should not be created "chmod go+r"
Bug #475501 reported by
Joke de Buhr
This bug affects 2 people
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Bazaar |
In Progress
|
Medium
|
Joke de Buhr | ||
| Breezy |
Fix Released
|
Medium
|
Jelmer Vernooij | ||
Bug Description
lauchpad-login and other plugins may use set_credential() to create the authentication.conf file. Since it's very likely someone might at passwords later on the file should never be created world readable initially.
It's safer to create the file with a umask(0177) rather the default umask. This way it's impossible to gain access to user-stored password if the user forget to run "chmod u=rw,go= ~/.bazaar/
Even set_credential() has the ability to store passwords so umasking the file would be safer if plugins start to set passwords with set_credential().
Related branches
lp://staging/~joke/bzr/file_permissions_authentication.conf
- Vincent Ladeuil: Needs Information
-
Diff: 52 lines (+19/-0) (has conflicts)1 file modifiedbzrlib/config.py (+19/-0)
lp://staging/~jelmer/brz/authconfperm
- Martin Packman: Approve
-
Diff: 144 lines (+64/-3)4 files modifiedbreezy/config.py (+21/-1)
breezy/help_topics/en/configuration.txt (+4/-0)
breezy/tests/test_config.py (+34/-2)
doc/en/release-notes/brz-3.0.txt (+5/-0)
| visibility: | private → public |
Revision history for this message
| Vincent Ladeuil (vila) wrote | #1 |
| Changed in bzr: | |
| assignee: | nobody → Joke de Buhr (joke) |
| importance: | Undecided → Medium |
| status: | New → In Progress |
Revision history for this message
| Joke de Buhr (joke) wrote | #2 |
| tags: | added: authentication |
Revision history for this message
| John A Meinel (jameinel) wrote | #3 |
| Changed in brz: | |
| milestone: | none → 3.0.0 |
| Changed in brz: | |
| status: | New → Triaged |
| importance: | Undecided → High |
| importance: | High → Medium |
| Changed in brz: | |
| status: | Triaged → In Progress |
| assignee: | nobody → Jelmer Vernooij (jelmer) |
| Changed in brz: | |
| status: | In Progress → Fix Released |
To post a comment you must log in.
Thanks for working on this !
Would you mind doing a merge proposal so we can better track your patch ?
A couple of comments in the mean time:
The _save() docstring is out of date: obviously this is used outside of tests now,
Can you add tests to:
- check that the file is created with the right umask,
- check that we issue the warning if not (and mention the mode bits in the warning)
I'm not sure about how to address that concern on windows, but at least to start we may not want to
do a check there if there is no way to make it succeeds...
Don't hesitate to bring the topic on the mailing list where we'll get more feedback.