firecracker icon indicating copy to clipboard operation
firecracker copied to clipboard

[Bug] Firecracker MMDS API headers are parsed case-sensitive

Open TilBlechschmidt opened this issue 3 years ago • 3 comments

Describe the bug

When sending MMDS requests to Firecracker, the X-metadata-token and X-metadata-token-ttl-seconds headers are parsed case-sensitive.

To Reproduce

  1. Start Firecracker VM with MMDS enabled (according to doc examples)
  2. Send an HTTP request with a lowercased header field name
    • e.g. curl -X PUT -H "x-metadata-token-ttl-seconds: 21000" http://169.254.169.254/latest/api/token
  3. Error response is returned

Expected behaviour

Header field names are parsed case-insensitive in compliance with RFC2616 Section 4.2 "Message Headers" which states that:

Each header field consists of a name followed by a colon (":") and the field value. Field names are case-insensitive.

Additional context

While most HTTP clients will use field names as-is, some proxies are rewriting them and popular HTTP libraries like hyper will consequently lower-case them without any option to change this behaviour. That being said, since the RFC states that field names are case-insensitive I see the responsibility on the side of Firecracker and not the HTTP clients.

Digging through the code, it seems that the culprit lies in the HashMap::get call which fetches fields by hash instead of case-insensitive lookup.

TilBlechschmidt avatar Sep 09 '22 12:09 TilBlechschmidt

Hi @TilBlechschmidt, thank you for opening this issue! We already have an open PR tackling this: #3006, but it hasn't been updated in a while and it got out of sync with the main branch. We will push forward to get this merged.

luminitavoicu avatar Sep 13 '22 11:09 luminitavoicu

Ah right, I went through the issues looking for a similar one but totally forgot to check the PRs. Thanks a lot!

(For people using hyper: Temporarily worked around it by using curl instead of hyper as I did not find a suitable fix apart from the open issue linked previously which is not implemented yet)

TilBlechschmidt avatar Sep 13 '22 11:09 TilBlechschmidt

@Compulves if you are not interested in pushing the PR forward anymore let me know and I'll take a stab at bringing it back in sync 🙂

TilBlechschmidt avatar Sep 13 '22 11:09 TilBlechschmidt