servicebroker icon indicating copy to clipboard operation
servicebroker copied to clipboard

Should the X-Api-Info-Location header be a part of the spec?

Open vorbidan opened this issue 5 years ago • 19 comments

What is the problem? When the service broker is used across multiple platform instances, it would be nice to understand which instance is making a call.

It seems that Spring Cloud Open Service Broker implementation has such a header defined, however, I could not find traces of it in the spec..

Who does this affect? Does this affect service broker authors, platform authors or developers?

  • developers,
  • platform authors
  • broker authors

Do you have any proposed solutions? Include an X-Api-Info-Location header as a part of the specification and allow for platforms to set this header during all requests (catalog, provision, bind, unbind, update, deprovision)

The format of the header MUST be:

X-Api-Info-Location: Platform value

Example:

X-Api-Info-Location: cloudfoundry ewogICJpbmZvX2xvY2F0aW9uX3VybCI6ICJodHRwczovL2FwaS5wbGF0Zm9ybS5leGFtcGxlLmNvbS92Mi9pbmZvIgp9

Where the value, when decoded, is:

{
  "info_location_url": "https://api.platform.example.com/v2/info"
}

Additional context Here is how Pivotal describes the usage:

X-Api-Info-Location Header

All calls to the broker from Cloud Foundry include an X-Api-Info-Location header containing >the /v2/info url for that instance. The /v2/info endpoint will return further information, >including the location of that Cloud Foundry instance’s UAA.

Support for this header was introduced in cf-release v212. Pivotal Docs

vorbidan avatar Jun 14 '19 21:06 vorbidan

Hey @vorbidan

Thanks for opening this issue.

There are two headers sent on those requests today which may help you understand where the request originated from:

  1. The Originating Identify hedaer (X-Broker-API-Originating-Identity)
  2. The Request Identity

Did you look at either of those fields and see if it is possible to understand which platform the call originated from?

mattmcneeney avatar Jun 17 '19 12:06 mattmcneeney

@mattmcneeney - thank you for following up with the response. Somehow I missed that the X-Broker-API-Request-Identity was mentioned int the spec... Still, I believe, it does not address the question of identifying the platform for the purposes of issuing a ping-back. Having just the platform ID in the header, may require keeping the state of the registered platform in some sort of the data store, if we want to further de-reference and issue a request back to platform... I was also thinking about the context - it is platform specific, i.e. cloudfoundry, it also now holds values like org_id and space_id, would it be inappropriate to add a field representing a platform API url, and/or the platform info location?

vorbidan avatar Jun 17 '19 14:06 vorbidan

That makes sense. What about having a platform_guid in the context object alongside org_guid etc?

mattmcneeney avatar Jun 17 '19 15:06 mattmcneeney

What would the platform_guid be? a URI, or UUID?

vorbidan avatar Jun 17 '19 15:06 vorbidan

Good question, maybe a GUID doesn't make sense. Maybe a URL is better, but do we have to worry about passing CC API URLs to service providers for security reasons?

@waterlink @williammartin any thoughts on this?

mattmcneeney avatar Jun 17 '19 15:06 mattmcneeney

What does Pivotal use in X-Api-Info-Location header in requests from the platform?

vorbidan avatar Jun 17 '19 16:06 vorbidan

@vorbidan @mattmcneeney

Right now, CF sends X-Api-Info-Location header to service brokers with the URL value pointing to external domain + /v2/info. For example:

X-Api-Info-Location: api.example.org/v2/info

waterlink avatar Jun 17 '19 16:06 waterlink

Great! So it looks like we may want to discuss 2 options:

  1. Make the X-Api-Info-Location header a part of the spec (this header is already being sent by CF, and is "understood" by Spring Cloud Services Broker implementation, and if my PR is accepted, will be recognized by Go implementation)
  2. Make this info to be a part of the context

In either case @mattmcneeney 's security concern needs to be addressed

vorbidan avatar Jun 17 '19 17:06 vorbidan

@mattmcneeney

About the security concern.

I’m not sure what you’re hinting at? What does the CC URL give to the potential attacker?


And even if it does have some concern, I can see how an attacker can get this URL by a variety of other means cheaply and quickly (including social engineering, or just signing up for the account if the platform is hosted; it’s also not hard to guess given enough information about the target).

waterlink avatar Jun 17 '19 21:06 waterlink

Well if we are already sending this information today, then I think my concern is void. If this is a header we send today then it would make sense for it to be an optional part of the spec. Any concerns about this from the svc-cat side?

mattmcneeney avatar Jun 18 '19 10:06 mattmcneeney

It looks like it makes a lot of sense to just add the header as an optional part of the spec, to bring it on par with implementation. The only caveat I see is that, at the moment, this header is implemented in CF world only. What about Kubernetes? Or because it is optional, it's not a big deal?

vorbidan avatar Jun 18 '19 14:06 vorbidan

@jberkhahn would you mind this header being part of the core spec? Would you use X-Api-Info-Location for specifying the location of the k8s cluster for example?

mattmcneeney avatar Jun 20 '19 07:06 mattmcneeney

That would be difficult as service catalog doesn't inherently know where the kube cluster is running, and also the part issuing the requests (the service catalog controller) is an entirely different piece from the part you normally talk to (the main kube apiserver). On top of that, the actual api of the apiserver is obfuscated behind a go client lib, so I don't even know what endpoint on the apiserver I would put in there even if I did know the apiserver's address.

So yeah, I don't really know what we'd put in there. I'll try to bring this up at a SIG meeting and get back to you

jberkhahn avatar Jun 20 '19 17:06 jberkhahn

Great, thanks @jberkhahn

So depending on the outcome from that discussion, we have two options:

  1. Add the header to the spec, as an optional header.
  2. Add the header to the profile, probably as a new section, though I do worry about adding a new first-level thing in profile as it's less visible than the main spec file (and all of the other info in profile to date is linked from the main spec - we could add a link though).

Let me know which of these seems most suitable for you @jberkhahn and we can look to open a PR.

mattmcneeney avatar Jun 24 '19 12:06 mattmcneeney

If a broker hands out different credentials to the platform instances, it can use these to identify the calling platform. There is no need for an additional header.

fmui avatar Jun 25 '19 13:06 fmui

@fmui That's true, but I think there are many cases where that doesn't happen today

mattmcneeney avatar Jun 25 '19 13:06 mattmcneeney

Hey @vorbidan - I opened up a PR for this. Let me know what you think! #686

mattmcneeney avatar Jun 27 '19 08:06 mattmcneeney

Dug a bit more into this. The k8s equivalent is a kubectl command called cluster-info, but the way it actually works is it queries several endpoints, all of which are authenticated. There isn't really a k8s equivalent for this.

jberkhahn avatar Jul 01 '19 21:07 jberkhahn

Cool - will update the PR to move this to profile only since it sounds like only CF will use this.

mattmcneeney avatar Jul 02 '19 09:07 mattmcneeney