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

Admin REST lacks security #339

Closed
mryellow opened this issue Jun 16, 2014 · 19 comments
Closed

Admin REST lacks security #339

mryellow opened this issue Jun 16, 2014 · 19 comments

Comments

@mryellow
Copy link

The lack of any form of authentication on the Admin REST API worries me.

Relying on restricting this port to local access isn't good enough. Gaining local access would mean gaining the ability to inject dodgy data, channels, ACL and users. Yes at this point your machine is compromised, but why make their lives easy.

@jessliu jessliu added this to the Future milestone Jun 20, 2014
@jchris
Copy link
Contributor

jchris commented Aug 13, 2014

The point is to make it easy to attach background processes that want to interact with the data without being constrained by the validation helper functions. Also to be able to Sync from Gateway -> Gateway.

If you want a secure interface, do your document update operations on the public port.

I can see some enhancements along these lines like configuration options to require name and password or an https cert on the private port if you want to do end to end ssl.

@snej
Copy link
Contributor

snej commented Mar 2, 2015

Been thinking about this. There are definitely configurations where a developer needs to run a privileged app server on another host, but can't ensure that both hosts are on a private secure network. (I think this would be common in hosted situations like AWS or Heroku.) This has come up a couple of times in conversations on the forum/mailing list lately.

For this reason it would be really good to be able to give the admin API a password/secret. It would be pretty easy to implement this, I think, by having the rest.handler class conditionally do a basic auth check on admin-user requests.

@tleyden
Copy link
Contributor

tleyden commented Mar 2, 2015

@snej +1 on this, since it would allow avoiding setting up a VPN in many scenarios.

On the downside, it makes it very easy for users to do very insecure operations. (doing basic auth with admin account over non-https)

@snej
Copy link
Contributor

snej commented Mar 3, 2015

The Gateway could easily enforce that non-local admin access has to use HTTPS.

@zgramana zgramana modified the milestone: Future Oct 2, 2015
@mikef-dk
Copy link

mikef-dk commented Feb 5, 2016

+1 on this as well. I think this would be absolutely useful.

I've tried to run my app server which handles the authentication / sessions on Google App Engine and struggled with this as well. Ultimately I had to drop the idea and move the app server to the same provider due to a missing authentication of the REST Admin API.

@toddfreese
Copy link

+1 on this.

@binhrobles
Copy link

+1 on this as well. Rolling out our own authentication strategy and currently whitelisting the Auth Service's server IP range for the reverse proxy in front of the Sync Gateway admin port. Would like to be able to manage security for the port at the Sync Gateway level, though.

@househippo
Copy link

Could SG leverage rBAC in CB 5.0.
Have it pass along credentials to CB for yes/no.

@djpongh djpongh added this to the 2.0.0 milestone May 26, 2017
@ghost ghost modified the milestones: 2.1.0, 2.0.0 Jul 17, 2017
@djpongh djpongh added the ffc label Jul 18, 2017
@mrjumpy
Copy link

mrjumpy commented Oct 13, 2017

+1

@djpongh djpongh modified the milestones: 2.1.0, 2.0.0 Nov 15, 2017
@djpongh djpongh modified the milestones: 2.0.0, 2.1.0 Dec 19, 2017
@ruaanviljoen
Copy link

+1

@djpongh djpongh added pmin and removed ffp labels Feb 7, 2018
@djpongh djpongh added backlog and removed icebox labels Mar 8, 2018
@adamcfraser adamcfraser added ready and removed backlog labels Apr 2, 2018
@adamcfraser adamcfraser self-assigned this Apr 2, 2018
@adamcfraser adamcfraser added backlog and removed ready labels Apr 9, 2018
@djpongh djpongh removed the P1: high label Apr 10, 2018
@djpongh djpongh modified the milestones: 2.1.0, 2.2.0 Apr 10, 2018
@djpongh djpongh added icebox and removed backlog labels Apr 10, 2018
@ghost
Copy link

ghost commented May 22, 2018

This is required specially when SG to SG replication runs and don't have VPN connectivity, and want security. Secondly for every data entry endpoint I think authentication must be available.
And the admin port case is even strong when one can add remove or change users.

@mryellow
Copy link
Author

mryellow commented May 22, 2018

@erpankaj22081982 Unfortunately this issue is a prime example of how security must never be an after-thought but simply must be considered from the very start. I have little doubt the technical debt is now so great that this can or will never be changed.

@rajagp
Copy link

rajagp commented May 23, 2018

@snej
Copy link
Contributor

snej commented Jul 25, 2018

@mryellow: Security was considered from the very start. We made a deliberate decision to simplify the setup/admin process by not requiring a password on localhost, which is secure enough for most needs.

AFAIK it would still be pretty easy to enable a password on the admin API; it's just never become a critical issue or something requested by a customer. If you feel strongly about it you could submit a PR; IIRC the source file to edit is rest/router.go.

@mryellow
Copy link
Author

mryellow commented Jul 25, 2018

Security was considered from the very start.

You're replying to this issue from half a decade ago to tell me that security was considered?

The whole thing with this issue was voicing this concern early in an attempt to stop a legacy being formed which would be covered in so much technical debt that there would be no way to ever fix this issue.

The place we currently stand.

We made a deliberate decision to simplify the setup/admin process by not requiring a password on localhost, which is secure enough for most needs.

You made that decision at the same time Google was making the decision to ensure that all internal links were encrypted. This was a post-Snowden world in which you were deciding to leave this completely open.

This was a mistake. No amount of revision of history will make this decision a good one. It remains to be fixed at some point in the future, or will continue to always be an issue until this software is history.

@snej
Copy link
Contributor

snej commented Jul 26, 2018

You seem very upset about this, so much so that I suspect you may be misunderstanding the situation — your mention of encrypting internal links is irrelevant, and if you think there’s too much technical debt to fix this, you haven’t looked at the code.

The admin API is by default not exposed to the network at all, only to the link-local interface. That provides complete protection against external attacks, which is important for a public-facing component. Yes, code running in the same host as SG can access the admin port, but if an attacker can run code locally you’re already in big trouble. (And yes, someone could override the configuration to bind the admin API to a network interface, in which case they would be responsible for the security implications of what they’d done.)

We could make the admin API secure against even local attack by adding authentication. That’s a good enhancement, and it would be pretty easy to implement (despite your repeated mention of “technical debt”, I don’t see any sign you’re familiar with our source code). It hasn’t been done simply because neither we nor our customers have considered it to be enough of a priority yet. Again, I invite you to spent a few hours adding this feature and sending a PR, since you’re the most excited about it of anyone.

@mryellow
Copy link
Author

encrypting internal links is irrelevant,

It's part of the issue. Encryption and authentication.

if you think there’s too much technical debt to fix this, you haven’t looked at the code.

It's on the all the deployments out there side. You can't change the default now, you'll break everything.

only to the link-local interface

As stated in the original comment. "Yes at this point your machine is compromised, but why make their lives easy.".

We could make the admin API secure against even local attack by adding authentication.

Would have been easier at the start. This is my point about having to think about it and implement it fro the beginning rather than think you'll do it later. It will never be done later.

don’t see any sign you’re familiar with our source code

The ease at which the source code can be changed is not the issue. It's the deployments which have to adjust to any changes there.

It hasn’t been done simply because neither we nor our customers have considered it to be enough of a priority yet.

Which has been a mistake. I see several posts above from people who seem to think it should have been a priority. Perhaps like myself they're not your customers because they found other solutions instead.

Again, I invite you to spent a few hours adding this feature and sending a PR, since you’re the most excited about it of anyone.

Too late.

@ghost
Copy link

ghost commented Jul 27, 2018

Thought this was an issue until we realized the Public API and Admin API can be separated. Made a comment and then deleted it. Very happy with the security configuration of CouchDB so far, although it did throw off our security auditing team for a moment since they didn't have intimate knowledge of the product.

Makes perfect sense. If localhost is compromised, so is everything else. We just stuffed it in a Docker container and said good bye.

@adamcfraser adamcfraser modified the milestones: Iridium, Mercury Aug 13, 2018
@djpongh djpongh removed the P1: high label Aug 30, 2018
@adamcfraser
Copy link
Collaborator

Moved to https://issues.couchbase.com/browse/CBG-641

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests