Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

An equivalent of Apache's +ExportCertData in Caddy #6713

Open
Marian-Kechlibar opened this issue Nov 27, 2024 · 15 comments
Open

An equivalent of Apache's +ExportCertData in Caddy #6713

Marian-Kechlibar opened this issue Nov 27, 2024 · 15 comments
Labels
feature ⚙️ New feature or request good first issue 🐤 Good for newcomers

Comments

@Marian-Kechlibar
Copy link

Apache Webserver's mod_ssl has an optional configuration flag +ExportCertData, which is turned off by default (for performance reasons), but when turned on, it populates the superglobals with variables containing the current server and client certificate. To cite from the docs:

When this option is enabled, additional CGI/SSI environment variables are created: SSL_SERVER_CERT, SSL_CLIENT_CERT and SSL_CLIENT_CERT_CHAIN_n (with n = 0,1,2,..). These contain the PEM-encoded X.509 Certificates of server and client for the current HTTPS connection and can be used by CGI scripts for deeper Certificate checking. Additionally all other certificates of the client certificate chain are provided, too. This bloats up the environment a little bit which is why you have to use this option to enable it on demand.

This would be useful in Caddy too, as it would make migration of Apache-optimized apps to Caddy more straightforward. We had a debate with @francislavoie about the functionality here and I made case that it makes sense to have access to the server-side cert as well.

https://caddy.community/t/populating-server-server-cert-and-server-client-cert/26521

He recommended me to start an Issue on GitHub, so here I am. I know next to nothing about Caddy code, so I dare not submit a Pull request for that functionality.

@francislavoie francislavoie added feature ⚙️ New feature or request good first issue 🐤 Good for newcomers labels Nov 27, 2024
@mholt
Copy link
Member

mholt commented Dec 4, 2024

Thanks -- to clarify, it looks like, based on the forum topic, the request is actually for placeholders containing server cert info.

@Marian-Kechlibar
Copy link
Author

Marian-Kechlibar commented Dec 4, 2024

Yes. it is.

Meanwhile, I was able to use the placeholder for the client certificate, but with some unexpected behavior, which may warrant its own issue...

Namely, when I enter the following into my Caddyfile:

request_header +X-Tls-Client-Cert-Der-Base64 {http.request.tls.client.certificate_der_base64}

the request header gets added even if there is no client cert provided, and populated with the literal "{http.request.tls.client.certificate_der_base64}"

That is unexpected for me. I would expect the header either to be absent, or empty in that case. (If HTTP allows empty headers; IDK). Should I start another topic over that purpose?

@mholt
Copy link
Member

mholt commented Dec 4, 2024

Ah, that's because in the header directive(s) only known placeholders are replaced. If there is no client certificate, the placeholder is not set.

We can change this in one of two ways:

  • Always replace placeholder-looking substrings, but this is problematic (syntactically) for values that aren't intended to be placeholders (e.g. JSON)
  • Replacers return two values: the value, and whether the key (placeholder name) is known. We are evidently returning false to known, when there is no client cert. It is kind of debatable whether that is correct. We could return true as in, we know this placeholder, but do we really if there is no client auth?

I'm leaning towards the second though, if we do make a change.

@Marian-Kechlibar
Copy link
Author

Ah, that's because in the header directive(s) only known placeholders are replaced. If there is no client certificate, the placeholder is not set.

We can change this in one of two ways:

  • Always replace placeholder-looking substrings, but this is problematic (syntactically) for values that aren't intended to be placeholders (e.g. JSON)
  • Replacers return two values: the value, and whether the key (placeholder name) is known. We are evidently returning false to known, when there is no client cert. It is kind of debatable whether that is correct. We could return true as in, we know this placeholder, but do we really if there is no client auth?

I'm leaning towards the second though, if we do make a change.

The second variant feels more natural to me, too.

@steffenbusch
Copy link
Contributor

Can you try this:

@hasClientCert {
       not vars {http.request.tls.client.certificate_der_base64} ""
}
request_header @hasClientCert +X-Tls-Client-Cert-Der-Base64 {http.request.tls.client.certificate_der_base64}

@mholt
Copy link
Member

mholt commented Dec 10, 2024

Thanks for the workaround @steffenbusch !

I suppose we could still change the placeholders behavior in the case of client auth missing.

@Ko496-glitch
Copy link

@mholt is this issue still open? because i would like to take a try on this.

@mholt
Copy link
Member

mholt commented Jan 6, 2025

As far as I know, it is not being worked on by anyone yet.

@Ko496-glitch
Copy link

@mholt got it, so i would start working on this. Could you give any advice on from where I should start?

@mholt
Copy link
Member

mholt commented Jan 7, 2025

I would start in here:

if strings.HasPrefix(field, "client.") {
cert := getTLSPeerCert(req.TLS)
if cert == nil {
return nil, false
}
// subject alternate names (SANs)
if strings.HasPrefix(field, "client.san.") {
field = field[len("client.san."):]
var fieldName string
var fieldValue any
switch {
case strings.HasPrefix(field, "dns_names"):
fieldName = "dns_names"
fieldValue = cert.DNSNames
case strings.HasPrefix(field, "emails"):
fieldName = "emails"
fieldValue = cert.EmailAddresses
case strings.HasPrefix(field, "ips"):
fieldName = "ips"
fieldValue = cert.IPAddresses
case strings.HasPrefix(field, "uris"):
fieldName = "uris"
fieldValue = cert.URIs
default:
return nil, false
}
field = field[len(fieldName):]
// if no index was specified, return the whole list
if field == "" {
return fieldValue, true
}
if len(field) < 2 || field[0] != '.' {
return nil, false
}
field = field[1:] // trim '.' between field name and index
// get the numeric index
idx, err := strconv.Atoi(field)
if err != nil || idx < 0 {
return nil, false
}
// access the indexed element and return it
switch v := fieldValue.(type) {
case []string:
if idx >= len(v) {
return nil, true
}
return v[idx], true
case []net.IP:
if idx >= len(v) {
return nil, true
}
return v[idx], true
case []*url.URL:
if idx >= len(v) {
return nil, true
}
return v[idx], true
}
}
switch field {
case "client.fingerprint":
return fmt.Sprintf("%x", sha256.Sum256(cert.Raw)), true
case "client.public_key", "client.public_key_sha256":
if cert.PublicKey == nil {
return nil, true
}
pubKeyBytes, err := marshalPublicKey(cert.PublicKey)
if err != nil {
return nil, true
}
if strings.HasSuffix(field, "_sha256") {
return fmt.Sprintf("%x", sha256.Sum256(pubKeyBytes)), true
}
return fmt.Sprintf("%x", pubKeyBytes), true
case "client.issuer":
return cert.Issuer, true
case "client.serial":
return cert.SerialNumber, true
case "client.subject":
return cert.Subject, true
case "client.certificate_pem":
block := pem.Block{Type: "CERTIFICATE", Bytes: cert.Raw}
return pem.EncodeToMemory(&block), true
case "client.certificate_der_base64":
return base64.StdEncoding.EncodeToString(cert.Raw), true
default:
return nil, false
}
}

@Ko496-glitch
Copy link

@mholt So i did some research and found that apache mod_ssl env variables list is huge so do you want me to work on most common use env variables or each of those that mod_ssl provides in apache. I am pasting the screenshot for reference.
Thanks

Image

@Marian-Kechlibar
Copy link
Author

If I may comment on this situation as a bystander with Apache experience: you can relatively easily parse the certificate details such as validity start by invoking openssl_x509_parse() on the certificate string SSL_CLIENT_CERT or SSL_SERVER_CERT, so the "details"-related variables such as SSL_CLIENT_V_START are somewhat less important to have.

@mohammed90
Copy link
Member

I think it's best to stay conservative in what we add, and only add what's needed.

@mholt
Copy link
Member

mholt commented Jan 7, 2025

Yeah, already have some of those; I would stick to just the few that this issue is requesting, but even before that, if you want, we can make this adjustment: #6713 (comment)

@Ko496-glitch
Copy link

I agree that the second approach makes sense. By still parsing the placeholder and returning a value like "No Client cert provided" when no .ClientCert is available during runtime, we can gracefully handle scenarios where the user is not doing mTLS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request good first issue 🐤 Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants