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

nginx should not serve certain owncloud paths #776

Merged

Conversation

btibor91
Copy link
Contributor

ownCloud locations in nginx were not prefixed with "/cloud/" subdirectory and therefore these rules won't match.

@JoshData
Copy link
Member

Looks like I made the mistake in 9d6dc78.

@JoshData
Copy link
Member

I'm trying to determine whether this is a security vulnerability. If you think so, please say so, but we can discuss details privately.

@btibor91
Copy link
Contributor Author

@JoshData - I wrote you an e-mail with more details.

@JoshData JoshData mentioned this pull request Mar 31, 2016
@yodax
Copy link
Contributor

yodax commented Mar 31, 2016

@btibor91 Good find, great eye for detail 😄

@JoshData JoshData changed the title Fix denied ownCloud nginx locations nginx should not serve certain owncloud paths Apr 1, 2016
@JoshData
Copy link
Member

JoshData commented Apr 1, 2016

Ok so here's my analysis of what's happening here:

Prior to this PR, we had been exposing people's ownCloud config.php's to the world. This is bad practice, but I don't think any damage was done. ownCloud's config.php has two sensitive fields:

  • passwordsalt: This field does not appear to be used when ownCloud is set up to do user authentication based on IMAP, as we do (thanks @yodax for pointing that out to me). It's also used by the files_external plugin to encrypt files stored in external storage, which again we're not using. So exposure of this field doesn't really matter for us.
  • secret: This field is used by the files_external plugin, which as above we're not using.

If anyone has modified their box, they will need to investigate further if this affects them.

If we were ever to enable the files_external plugin, we'd have a problem. Since we're not using either of these fields now, we should reset their values to something new so that affected users have a clean config after this PR is applied.

Thanks everyone. I'll merge this PR into master shortly.

@JoshData JoshData merged commit c5e8a97 into mail-in-a-box:master Apr 1, 2016
@btibor91 btibor91 deleted the nginx-owncloud-locations-fix branch April 1, 2016 12:38
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.

3 participants