openservicebrokerapi / servicebroker

Open Service Broker API Specification
https://openservicebrokerapi.org/
Apache License 2.0
1.19k stars 434 forks source link

Is the spec contradictory around TLS requirements? #571

Closed williammartin closed 6 years ago

williammartin commented 6 years ago

In the platform to service broker auth section, there seems to be some contradictory text:

While the communication between a Platform and Service Broker MAY be unsecure, it is RECOMMENDED that all communications between a Platform and a Service Broker are secured via TLS and authenticated.

vs.

Additionally, the Service Broker MUST secure communications with TLS. The Platform and Service Broker SHOULD agree whether the Service Broker will use a root-signed certificate or a self-signed certificate.

So, is it recommended or required?

williammartin commented 6 years ago

This commit seems to have introduced the MUST requirement on TLS at all times. This seems like a breaking change that we should revert.

That said, the previous text,

If authentication is used, the Service Broker MUST authenticate the request using the predetermined authentication mechanism, securing communications via TLS, and MUST return a 401 Unauthorized response if the authentication fails.

suggests that if any authentication scheme (basic or otherwise) is used, TLS MUST also be used. It seems weird to conflate the concepts of platform to broker authentication and transport level security; though I can understand why one may want to ensure the latter in the case of the former, it shouldn't be required by the specification.

This commit first introduced text about the optionality of TLS, and later was followed by two commits introducing bearer tokens, and rewording, that required TLS during the authentication step. Later however, this TLS requirement was extended to all authentication mechanisms in this commit

My suggestion is that we remove the requirement of TLS on basic authentication, but keep the requirement for bearer tokens, as they were introduced together (and are required by https://tools.ietf.org/html/rfc6750) and therefore not breaking.

In fact in the following PR, this was called out as a breaking change.

williammartin commented 6 years ago

My suggestion is that we remove the requirement of TLS on basic authentication, but keep the requirement for bearer tokens, as they were introduced together (and are required by https://tools.ietf.org/html/rfc6750) and therefore not breaking.

Actually, in the same we don't specify that we don't specify character sets (https://github.com/openservicebrokerapi/servicebroker/issues/560), I'm wondering whether we need to call our TLS in the specification here, since it is defined in the Bearer Token RFC.