caddy: "invalid header field value" when including http.request.tls.client.certificate_pem placeholder in a header

Caddy version: v2.2.0 h1:sMUFqTbVIRlmA8NkFnNt9l7s0e+0gw+7GPIrhty905A=

I am trying to pass pem-encoded client certificate to proxied service via a X-SSL-Cert header, like so:

sub.example.com {
    reverse_proxy 127.0.0.1:8000 {
        header_up X-SSL-Cert {http.request.tls.client.certificate_pem}
    }

    tls {
        client_auth {
            mode require
        }
    }
}

I am trying to use recently added client.certificate_pem placeholder (https://github.com/caddyserver/caddy/pull/3662/commits/497c3f785772b0017c525ef6de587eeba4aec4dc#diff-18e521e381018c19bda45164dcad2b9cR347 ) which, if I understand correctly, is synonymous to apache’s SSL_CLIENT_CERT (i.e. in an apache config one would write RequestHeader set X-SSL-Cert "%{SSL_CLIENT_CERT}s").

But for the given Caddyfile I get a 502 Bad Gateway with the following error in the logs:

Sep 30 19:11:55 example.guest caddy[9838]:
{
	"level": "error",
	"ts": 1601493115.886311,
	"logger": "http.log.error",
	"msg": "net/http: invalid header field value \"-----BEGIN CERTIFICATE-----\\nMIIG... certificate content...-----END CERTIFICATE-----\\n\" for key X-Ssl-Cert",
	"duration": 0.000691328,
	"status": 502,
	"err_id": "gys3swkea",
	"err_trace": "reverseproxy.(*Handler).ServeHTTP (reverseproxy.go:440)"
}

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 36 (25 by maintainers)

Most upvoted comments

As it stands now, certificate_pem will always cause an error in Caddy

Only when used in a header, which it may not necessarily be. Placeholders can be used almost anywhere in the config.

My understanding is that it fails because \n is a control character and httpguts.ValidHeaderFieldValue(h) rejects control characters.

The problem is there’s no standard for how header values should be encoded if they have invalid characters. The typical ways are either URL encode or base64 encode, but it depends on the applications which one makes the most sense. If there’s evidence that some apps specifically support URL encoded PEM, then I don’t have a problem adding a placeholder for that.

francislavoie on Jun 1, 2023

Did you also see the solution that Martin posted here https://github.com/mcebular/caddy/commit/df63c2d78fc3c2d6daf5ccf8c42a4dc4cc1f4572?

IMO this is something that should be improved in Keycloak, and I don’t think changing the Caddy source / featureset to accommodate a URL-encoding of a base64-encoding of data that isn’t even going in a URL is going to really be doing anyone a favor upside_down_face

Keycloak supports Nginx, Apache and HAProxy, which can all pass PEM certificates in HTTP headers without failing, unlike Caddy. My ask of Keycloak will essentially boil down to having them patch a solution to support DER. It should be doable but there is no reason why the Keycloak team will need to support this, as a PEM file is well understood.

For now I’ll keep using Martin’s solution and will read through Keycloak’s contribution guidelines tomorrow and make the ask.

henryreed on Jun 2, 2023

@mholt the ask is that the PEM is URL-encoded, i.e. URL-encoding the \n characters, not URL-encoded raw bytes.

francislavoie on Jun 2, 2023

You could try this maybe, if it requires the ASCII armor to exist

header_up X-Client-Cert "-----BEGIN CERTIFICATE-----{tls_client_certificate_der_base64}-----END CERTIFICATE-----"

This is the same text as a PEM, but without newlines. That might be fine depending on how Keycloak parses it.

Anyway, I suggest you reach out to Keycloak as well, they should probably be able to support der-base64 as well.

francislavoie on Jun 1, 2023

@anestos

It is not super easy for me to test but if i do and it works will it get merged?

Maybe. Depends on the results of the test and the general utility/usefulness of the feature.\

I don’t want to spend time trying this in my environemnt otherwise.

Yeah… we maintainers feel this.

mholt on Sep 30, 2021

@Ramzec @mc0239 please take a look at PR, we can merge it, but you need to check it first and make sure that it solves the problem

musinit on Aug 17, 2021

I don’t see the harm in URL encoding it personally. It’ll be more compatible with existing applications written to support the approach Apache and Nginx take with passing through certificates via headers.

If we do the DER approach then it should be called certificate_der, but I don’t see the harm in adding certificate_pem_escaped for those that want it that way.

francislavoie on Oct 1, 2020

Looks like it hits this check: https://godoc.org/golang.org/x/net/http/httpguts#ValidHeaderFieldValue

in https://golang.org/src/net/http/transport.go

I’m not certain how to decipher that 🤔

Edit: Yeah isCTL() rejects \n as it’s ASCII value is 10 (it rejects 0-31 and 127)

Edit2: Looks like HTTP headers use \r\n (CRLF) for newlines.

francislavoie on Oct 1, 2020