Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The secret key that encrypts the backups should not be world readable. #150

Merged
merged 3 commits into from Aug 24, 2014

Conversation

hjjg
Copy link
Contributor

@hjjg hjjg commented Aug 22, 2014

umask at the creation of the key plus migration.py-function to fix permission in existing installations.

@JoshData
Copy link
Member

Hello & thanks!

Could you re-do this with the umask technique we use here:

https://github.com/mail-in-a-box/mailinabox/blob/master/setup/dns.sh#L43

This is more secure by ensuring the file is created initially with the correct permissions, rather than opening a small window when it has incorrect permissions.

The fix for existing installations should really go in the migrate script so that we don't clutter the setup script.

@hjjg
Copy link
Contributor Author

hjjg commented Aug 22, 2014

How about that? ;)

@JoshData
Copy link
Member

Fantastic. Did you test both parts? (Hoping to avoid testing myself before merging.)

@hjjg
Copy link
Contributor Author

hjjg commented Aug 22, 2014

I did not test the complete scripts but the individual lines of code. Both worked fine (i.e. possible namespace issues, syntax..)

@hjjg
Copy link
Contributor Author

hjjg commented Aug 22, 2014

I think you should also notify users that there secret key could be compromised.

@JoshData
Copy link
Member

The threat model for this project assumes that only trusted users have local access, so absent some other problem there's no real problem.

@JoshData
Copy link
Member

The management part doesn't work. If you run setup.sh on your own box again you'll see it sets incorrect/crazy permissions.

@hjjg
Copy link
Contributor Author

hjjg commented Aug 24, 2014

The bug was at the migrate.py script. It works now. It's a different behaviour in Python 3 that caused this. Most of my scripts are in Python 2 :)

JoshData added a commit that referenced this pull request Aug 24, 2014
The secret key that encrypts the backups should not be world readable.
@JoshData JoshData merged commit 28231ac into mail-in-a-box:master Aug 24, 2014
@JoshData
Copy link
Member

Thanks!

@hjjg hjjg deleted the secretkeyfix branch August 25, 2014 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants