add more SSL_ meta vars from the mod_ssl family

[HoneyryderChuck]

adding a few more variables when webrick is run in SSL, and mutual tls is in place; in such a case, it's important to send information to the backend whether the certificate has been verified, among others.

[ioquatix]

This looks like a good start to me.

Can we, in no specific order:

1. Add tests.
2. Add documentation about the fields.
3. Refer to standards if they exist for the field names and values.
4. Consider as an alternative, whether exposing the SSL context and client certificate would make more sense.
5. Identify any performance concerns related to extracting all the fields vs just directly exposing the context/certificate.

I don't have a strong opinion about which direction we go in, but I think we should make sure we have considered the options carefully just because once this is set, I imagine it will be hard to change later.

[olleolleolle]

Judging by https://httpd.apache.org/docs/2.4/mod/mod_ssl.html, something like this YARD docblock, for this fragment of values. But, i guess they are not passed in anywhere by calling code, are they?

# @option meta [String] "SSL_CLIENT_M_VERSION" The version of the client certificate
# @option meta [String] "SSL_CLIENT_M_SERIAL" The serial of the client certificate
# @option meta [String] "SSL_CLIENT_S_DN" Subject DN in client's certificate
# @option meta [String] "SSL_CLIENT_I_DN" Issuer DN of client's certificate
# @option meta [String] "SSL_CLIENT_V_START" Validity of client's certificate (start time)
# @option meta [String] "SSL_CLIENT_V_END" Validity of client's certificate (end time)
# @option meta [String] "SSL_CLIENT_V_REMAIN" Number of days until client's certificate expires
# @option meta [String] "SSL_CLIENT_A_SIG" Algorithm used for the signature of client's certificate
# @option meta [String] "SSL_CLIENT_A_KEY" Algorithm used for the public key of client's certificate
# @option meta [String] "SSL_CLIENT_CERT" PEM-encoded client certificate
# @option meta [String] "SSL_CLIENT_VERIFY" Either "NONE", "SUCCESS", "GENEROUS" or "FAILED:reason".

[HoneyryderChuck]

Thx. Will follow the suggestions. FTR the already existing support for SSL vars wasn't being tested either, hence why I didn't start there.

[ioquatix]

WEBrick is horribly under-tested, so I'm not surprised we are missing a ton of tests.

[HoneyryderChuck]

@ioquatix added tests and ref to docs, just like it's done for the CGI meta vars.

Regarding the proposed alternative, I think the SSL context and client certificate should not be exposed. Protocol-wise, we should default to the lowest-common denominator established by CGI, which is the superset of rack, and for all its purposes, that seems to be mod_ssl. Most ruby apps are deployed behind a gateway/proxy (apache supports it, nginx ssl module provides the variables, which you then have to set manually via proxy_set_header, AWS API Gateway supports a variation of it as well for lambda functions), so we won't get a ruby ssl socket, and rebuilding the ssl context is complex, when you may not even need it.

I also don't think that there are relevant performance concerns to be worried about. The new SSL_ vars are short strings which require little memory. The client cert in PEM was already being sent.

[ioquatix]

Just to be clear, Rack and WEBrick are two different things, so if we are unpacking these things at the rack layer, that will still be an adaption of WEBrick, and if WEBrick chooses to expose the underlying SSL context, and Rackup expands it to match the spec, that's also okay.

I'm not particularly concerned about WEBrick performance, but more about the following:

• Puma / Falcon performance.
• Memory usage.
• Opt in vs always present (maybe a way to mitigate the performance overhead if it's not used).

By punting on the extraction of the meta fields and just supplying the context, it means we can switch this feature off more easily at the rackup/server level.

I think we should test the performance of this change. Does every SSL request include these variables, or only those that supply a client certificate?

In rack, I think we should use a single key for all the fields, e.g. tls.client_certificate => {...}` or something more specific, but I don't have a strong opinion on it.

One more question - how does this work when a load balancer terminates TLS?

[olleolleolle]

Can this key also used the non-hashrocket style? Or is it somehow special?

[HoneyryderChuck]

nothing special about it, I just copy-pasted as is from the test above. I assumed that consistency wasn't a big issue in this codebase. Perhaps you should consider using rubocop at some point?

[HoneyryderChuck]

I agree. webrick is not rack. However, this meta_vars function seems to follow the CGI spec (the non-https implementation even mentions it in the docs). And the previous iteration of https meta_vars (which is in webrick source since 2013) was already following that approach of following mod_ssl, even if didn't expose all of the vars.

Moreover, rack is a superset of CGI, and is an "offspring" of python wsgi, which as I documented in the MR opened for the rack spec, declares support for the mod_ssl family of vars .

Between the arguments above, I believe there's enough reason not to support a rack-custom "tls.ssl_context" kind of variable. IMO rack is already so constrained and beyond its evolution, that we should limit the "surprising" effects of the bits we try to optimize for.

Going even further, not a lot of ruby app server implementations even support TLS based on ruby SSL Sockets. puma doesn't. unicorn(?) doesn't. thin doesn't. From the most popular ones, I believe only webrick, and perhaps falcon. There may be implementation overhead in making things "quack" like an OpenSSL::SSL::SSLContext, whereas the env var and its strings are "the devil we know".

About performance, as mentioned, I don't think that should be a concern. The added headers have very little performance impact (small strings). It's a decision of the servers to choose which SSL_ vars they support. In the case of webrick, I've only added support for SSL_CLIENT_ family, which is the one where I see more value at the moment, particularly for the mutual TLS use-cases I'd like to have support for. And mutual TLS is enough of a "fringe" feature for these particular headers not to even be exposed to the majority of ruby deployments around (this answers the question "Does every SSL request include these variables, or only those that supply a client certificate?").

One more question - how does this work when a load balancer terminates TLS?

Apache supports mod_ssl. nginx, as mentioned in the previous comment, only provides the config variables, and it's up to the server admin to set them as proxy-headers proxy_set_header in the config (example). API-gateway exposes them as context vars:

identity"=>{"cognitoIdentityPoolId"=>nil, "clientCert"=>{"clientCertPem"=>"<CLIENT-PEM>", "serialNumber"=>"4498723124", "issuerDN"=>"CN=server", "validity"=>{"notAfter"=>"Jan 24 10:18:56 2024 GMT"....

They're all "sorta kinda" the same, because they have to adapt more than one interface, as CGI (and therefore, wsgi or rack) is not universal. If you want a non-ruby example, here's what the node-oidc-provider lib expects to come in the HTTP request.

Feel free to run this by other rack maintainers, and see what they think.