data-api-builder icon indicating copy to clipboard operation
data-api-builder copied to clipboard

`x-ms-api-role` header length restriction enforcement

Open seantleonard opened this issue 2 years ago • 9 comments

3.) Evaluate whether we need additional restrictions on the custom client role header (x-ms-api-role) such as max length allowed for the header value, if exceeded, we don’t even process/reject request.

seantleonard avatar Aug 18 '22 16:08 seantleonard

Check OWASP site for any security related guidance, if any, regarding header max length

seantleonard avatar Sep 28 '22 15:09 seantleonard

As per HTTP RFC here: "HTTP does not place a predefined limit on the length of each header field or on the length of the header section as a whole, as described in Section 2.5. Various ad hoc limitations on individual header field length are found in practice, often depending on the specific field semantics. A server that receives a request header field, or set of fields, larger than it wishes to process MUST respond with an appropriate 4xx (Client Error) status code. Ignoring such header fields would increase the server's vulnerability to request smuggling attacks A client MAY discard or truncate received header fields that are larger than the client wishes to process if the field semantics are such that the dropped value(s) can be safely ignored without changing the message framing or response semantics."

As per these articles: What is the maximum size of HTTP header values? and Maximum on HTTP header values?

Servers have their custom limit on header size like:

Apache : 8K
IIS : 16K

exceeding which they will return 413 Entity Too Large exception.

  1. So one option is that if the length of the X-MS-API-ROLE header is greater than the allowed limit that we impose, we just ignore the header as if it wasn't present at all in the request.

  2. We throw an Entity Too Large exception (http status code: 413)

ayush3797 avatar Nov 03 '22 05:11 ayush3797

Looks like Kestrel (Asp.net) has limit default to 32KB https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.server.kestrel.core.kestrelserverlimits.maxrequestheaderstotalsize?view=aspnetcore-6.0

seantleonard avatar Nov 03 '22 05:11 seantleonard

Look through some of these to see if we might need to consider additional security measures. And whether Asp.net has built in features/mitigations

https://cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html#input-validation

seantleonard avatar Nov 03 '22 05:11 seantleonard

Look through some of these to see if we might need to consider additional security measures. And whether Asp.net has built in features/mitigations

https://cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html#input-validation

I went through this, the only things I see we might want to include are:

  1. Verify the Content-Type we are getting in the request. At this point, even if we send text/html or any other data type but still send a valid json request body, the request goes through fine. We can impose a check Content-Type is application/json (in case of REST), where we can throw appropriate exception on the check failure.

  2. Similarly, we will be returning application/json in response body. So, if a request specifies any other content-type in the Accept header, we fail the request throwing appropriate exception.

ayush3797 avatar Nov 03 '22 12:11 ayush3797

Discussed about the issue with [email protected] with regards to Phoenix runtime: He informed me that currently they are planning on passing on the below headers:

  1. X-MS-API-ROLE
  2. X-MS-CLIENT-PRINCIPAL
  3. Content-Type

He told me that it should be fine to forward the Accept header as well.


Discussed with [email protected] with regards to SWA: In his own words "we're discussing in general how to handle proxying headers so it will be a bit for us to finalize and deploy the change, but content-type is something we were planning to forward anyways. I think for graphql it's needed since there's supposed to be support for a special content-type application/graphql". Sounds like they are already planning to pass on the Content-Type and can also pass Accept header as well but it'll take some time to implement it on their side.

ayush3797 avatar Nov 07 '22 10:11 ayush3797

Thanks for following up with those individuals to provide more context here, Ayush!

seantleonard avatar Nov 07 '22 16:11 seantleonard

Document these restrictions in dab

Aniruddh25 avatar Nov 08 '22 18:11 Aniruddh25

@Aniruddh25 we might have to move this as well to Jan2023 as we still need SWA->Phoenix to proxy those headers first all the way to DAB.

ayush3797 avatar Nov 14 '22 18:11 ayush3797