deis / charts

(OBSOLETE) Helm Classic v1 Charts for Deis Workflow
https://deis.com/workflow/
MIT License
45 stars 36 forks source link

[Workflow 2.1.0] builder-key may contain whitespace unsafe for HTTP headers #303

Closed felixbuenemann closed 8 years ago

felixbuenemann commented 8 years ago

The deis-controller-secret-builder-key-auth.yaml uses builder-key: {{ randAscii 64 | b64enc }} to generate the token used for API token auth between the builder and the controller.

This is not safe, since it sends the base64 decoded value as a X-Deis-Builder-Auth HTTP header.

The generated secret can start or end with whitespace, for example for me it generated this:

 5m|yIoC9z=|rR6FzujV\"mzMpzhltnW:?t,T8%'e!Ug~t!yK)`ioC/Fu2'OkK<JP

If you look closely, you'll see that the key begins with s space.

Whitespace in HTTP headers is opaque (ignored) so keys beginning or ending with spaces won’t work.

This lead to deis pushfailing with [ERROR] Failed handshake: EOF because it was sending the header:

X-Deis-Builder-Auth:  5m|yIoC9z=|rR6FzujV"mzMpzhltnW:?t,T8%'e!Ug~t!yK)`ioC/Fu2'OkK<JP

The first space was ignored, so the authentication failed (deis-builder should really show a better error message if authentication to the controller fails).

To fix this problem I propose to either double base64 encode the secret, so the header is sent base64 encoded or use a character set that is guaranteed to be safe when used as an HTTP header. Something like [A-Za-z0-9] should probably provide enough entropy for a 64 byte secret.

In my case I fixed the issue by generating a new secret manually, switching out the secret and killing the builder and controller pods using kubectl, after which deis push started working.

A similar key is generated in deis-controller-secret-django-secret-key.yaml, but I haven't checked if it's used anywhere as a header. It probably wouldn't hurt to use a safer set of character there as well.