kubernetes-retired / service-catalog

Consume services in Kubernetes using the Open Service Broker API
https://svc-cat.io
Apache License 2.0
1.05k stars 385 forks source link

Credentials in binary form end up being doubly-base64-encoded in the Secret #1945

Closed luksa closed 4 years ago

luksa commented 6 years ago

Imagine that a broker returns a binary PFX certificate as one of the credentials. In order to do that, it needs to base64 encode it in the binding response JSON. The service catalog controller doesn't know the received string is base64-encoded.

If the controller stored the base64-encoded string in the Secret, everything would be fine, since the service consumer expects the credential to be in that form. But the problem is that the controller base64-encodes it again before storing it in the credentials Secret, so we end up with a doubly-encoded string. The service consumer must decode twice to get the original value.

I'm not sure we can do anything about it at this point, as there's no way for the controller to know whether a credential is already base64-encoded or not.

Related OSB API issue: https://github.com/openservicebrokerapi/servicebroker/issues/497

nilebox commented 6 years ago

Currently any credential is serialized into []byte first, and then we set Secret values via Data map[string][]byte, so internally Service Catalog already sets data as binary. It's Kubernetes Secret that encodes this binary data with base64: https://github.com/kubernetes/api/blob/db88d6fb5927cf390f4d33133339a4fb0546399a/core/v1/types.go#L4634-L4639

So the real cause of double base64 encoding is that JSON doesn't natively support binary fields, and that there is no extra field in the JSON payload (or in the /catalog response) that would tell Service Catalog to decode the base64-encoded credential before serializing it into []byte. IMO it's a perfectly reasonable limitation for such generic spec as OSB. Otherwise the credential response structure would have been much more complex than just map[string]interface{}.

fejta-bot commented 5 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 5 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

ashokkkannan commented 5 years ago

I understand that handling binary data needs to be done on the API spec side. But do we want to consider providing some hooks via service catalog for transforming binding data? There may be other scenarios where users might require some fine-grained transformations.

fejta-bot commented 5 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

fejta-bot commented 4 years ago

Stale issues rot after 30d of inactivity. Mark the issue as fresh with /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle rotten

fejta-bot commented 4 years ago

Rotten issues close after 30d of inactivity. Reopen the issue with /reopen. Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /close

k8s-ci-robot commented 4 years ago

@fejta-bot: Closing this issue.

In response to [this](https://github.com/kubernetes-sigs/service-catalog/issues/1945#issuecomment-559577329): >Rotten issues close after 30d of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh with `/remove-lifecycle rotten`. > >Send feedback to sig-testing, kubernetes/test-infra and/or [fejta](https://github.com/fejta). >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.