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

Fix user permissions issues for Postgresql #50

Open
aeijdenberg opened this issue Jun 22, 2017 · 6 comments
Open

Fix user permissions issues for Postgresql #50

aeijdenberg opened this issue Jun 22, 2017 · 6 comments

Comments

@aeijdenberg
Copy link
Contributor

aeijdenberg commented Jun 22, 2017

When an application is bound to a service, the broker currently creates a username/password specific to that binding so that each application bound to a service has separate credentials.

It is expected that each of these users has the same access to the same underlying resource.

This is achieved easily enough with MySQL, however when database objects (such as tables) are created in Postgresql, by default that object is owned by the user that created it and other users need to be explicitly granted permission to access it (simply having permissions on the database that contains the object isn't enough to give direct permission to query over the objects).

For blue/green deployments, it is common practice to need to have 2 different applications to be able to access the same database.

Currently when using tools such as cf bgd (link) to push apps, a second application is automatically created, and services are automatically bound to it based on the services listed in the manifest.yml.

Since the second application has a different username to the first, as soon as code in the application tries to access tables in the database, it fails with permission denied errors.

Option 1 (currently implemented): Provide hook at service bind time to choose database username

To workaround this issue, the service broker currently allows specification (at service bind time) of a specific username to use when connecting to the database. See here for details.

This has the following issues:

  • does not work when service binding are created automatically (such as when specified in manifest.yml, which does not support extra parameters), which some blue/green tools use
  • is additional work for delivery teams
  • is a different user experience than when using MySQL with same broker
  • by accepting a username from a client, this means that broker needs to perform additional authorization logic (which it probably can't actually do correctly, though have not verified) to ensure it does not become a confused deputy

Option 2: Modify the Postgres binding code to ensure the new user has sufficient permissions

We could explore creating making additional calls at bind time to enumerate the schemas in the database, then use GRANT ALL PRIVILEGES TO ALL TABLES IN SCHEMA ... etc to give the new user appropriate permissions.

This has the following issues:

  • not all data object types in Postgresql have an ALL xxx option
  • tables / objects created after bind time would not be accessible by the new user

Option 3: Modify the applications to use an additional role that they grant access to

We could advise delivery teams to modify their applications to ensure that created objects are always followed by an additional GRANT ... that gives appropriate permissions to a common role for that database.

This has the following issues:

  • extra complexity / source of bugs for delivery teams
  • service broker would need to manage creation of additional role

Option 4: Modify the service broker to always use the same username for the same database with Postgresql

Modify the service broker such that for Postgresql it will only ever create a single user (calculated on the same name as the database that it creates) and share this across each application that binds to that service.

This is basically the same as option 1 - but always done (removing complexity for clients), and without allowing a user specified name (mitigating confused deputy risks).

This has the following issues:

  • harder to manage rotation of credentials (deleting all bound instances and recreating should do it)

Option 5: ?

@sarneaud
Copy link

Here's some history:
https://github.com/AusDTO/ops/issues/177

I think your option 4 name is really just "to use one username per database". I don't think we can do anything better if the CF broker API is still like it was when I last researched the problem.

Option 4 is probably the most practical one overall for now. Our ideal solution doesn't seem to be possible without a lot more work patching CF (or, heck, Postgres).

@aeijdenberg
Copy link
Contributor Author

Thanks Simon, that's a better description (I'll see if I can edit the original to use it).

It feels like client certificate authentication would work reasonably here too (multiple certs for the same username) but given that CF doesn't seem to have a good way of rotating creds, and that RDS (as far as I can tell) doesn't actually support client certs, that's a non-starter too.

@santrancisco
Copy link

I also like option4.

Regarding harder to manage rotation of credentials (deleting all bound instances and recreating should do it), If it is just user rotating credentials for security reason i think it is fair to restart all bounded instances. Even in our current environment with 1 app bind to 1 postgresql, this would mean changing the setting in CI/CD tool and trigger a new build. I don't recall any automate process that would rotate the credentials at the moment.

@drnic
Copy link

drnic commented Jun 22, 2017

Hey guys, there are a few threads here, and sorry if I miss some explicit/implicit questions. Couple of ideas:

  • a broker should support many bindings per service instance and all should work; if this broker isn't doing this, then the broker has a bug
  • two bindings can return the same credentials; its optional if they are unique - the benefit of unique credentials per binding is the ability for cf delete-binding to yank binding credentials independently from each other
  • the upstream repo https://github.com/cloudfoundry-community/pe-rds-broker/ is seamingly abandoned; since it is in https://github.com/cloudfoundry-community then any of the 200+ co-owners can take over projects if they are abandoned. My suggestion is to ping one or two of the more recent committers, such as @dlapiduz and ask if its ok to take over maintenance. Then you could merge in your work, merge in the other community PR, and nurture it along. I can add anyone to the https://github.com/cloudfoundry-community as an Owner; once youre an Owner you can add other Owners.
  • CF doesn't have a nice way to rotate service binding credentials; this is disappointing. Solving it is outside the scope of any given broker. You can generate new credentials with new bindings, so as @santrancisco suggested - apply new binding and restart the app/instances. This can be coordinated by CI job.

@aeijdenberg
Copy link
Contributor Author

Thanks @drnic. Someone has also pointed me to the AlphaGov fork which seems much better supported. They've also addressed this particular issue with this PR, which appears to be similar to our option 4.

I think we might go with our own option 4 for our immediate needs, and then look at whether it make more sense to switch to the AlphaGov fork.

@dlapiduz
Copy link
Contributor

@drnic I am not doing gov work anymore so I think it's best for someone else to take over if help is needed. As you say there are lots of admins and they can merge the PRs.

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

No branches or pull requests

5 participants