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

Harden updater authentication #22276

Merged
merged 1 commit into from
Feb 10, 2016
Merged

Harden updater authentication #22276

merged 1 commit into from
Feb 10, 2016

Conversation

LukasReschke
Copy link
Member

  • Reset tokens after 2 days
  • Used BCrypt for storing the password in the config.php. This makes it substantially harder in case of a leakage of the token to bruteforce it. In the future we can evaluate also an HMAC including the IP. That's a bit tricker though at the moment considering that we support reverse proxies. Didn't feel brave enough to touch that dragon now as well ;)

Reason behind this that if somebody is able to read the config file somehow this won't immediately result in a RCE.


Requires owncloud/updater#239

@LukasReschke
Copy link
Member Author

@karlitschek @VicDeo @DeepDiver1975 Please review.

@MorrisJobke
Copy link
Contributor

👍

@karlitschek
Copy link
Contributor

👍 Can we do a bit longer then 2 hours please? I'm worried that we are running into timeout situation here. I don't think there is harm done when we do 2 days instead of 2 hours.
Reliability of the updater is the absolutely top priority

@LukasReschke
Copy link
Member Author

Fair enough. Let's make it two days, considering this change it's also not that critical anymore as before.

- Reset tokens after 2 hours as discussed at owncloud/updater#220 (comment)
- Used BCrypt for storing the password in the config.php. This makes it substantially harder in case of a leakage of the token to bruteforce it. In the future we can evaluate also an HMAC including the IP. That's a bit tricker though at the moment considering that we support reverse proxies. Didn't feel brave enough to touch that dragon now as well ;)
@LukasReschke
Copy link
Member Author

2 days it is 😉

@VicDeo
Copy link
Member

VicDeo commented Feb 10, 2016

👍 Tested

DeepDiver1975 added a commit that referenced this pull request Feb 10, 2016
@DeepDiver1975 DeepDiver1975 merged commit 6b83632 into master Feb 10, 2016
@DeepDiver1975 DeepDiver1975 deleted the harden-updater-auth branch February 10, 2016 16:31
@lock
Copy link

lock bot commented Aug 7, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants