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

add SSL_ variables description to the rack spec #2033

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HoneyryderChuck
Copy link

@HoneyryderChuck HoneyryderChuck commented Feb 1, 2023

While they should already "just work", the spec still lacks definition about what their special meaning. This is in contrast to PEP-0333, where it is clearly stated that these belong to the mod_ssl family of SSL vars.

This follow this MR in webrick.

Discussed in #2028

While they should already "just work", the spec still lacks definition
about what their special meaning. This is in contrast to
[PEP-0333](https://peps.python.org/pep-0333/), where it is clearly
stated that these belong to the mod_ssl family of SSL vars.
@jeremyevans
Copy link
Contributor

I would prefer to leave this unspecified. In this case, I don't think the advantages of specifying it (consistency between implementations) outweigh the costs (requiring the server to convert an internal SSL certificate representation into a bunch of env vars that may not be used). If the use of SSL client certificates became popular in Ruby, and there were multiple Ruby web servers supporting it where specification would be beneficial, I would reconsider.

@HoneyryderChuck
Copy link
Author

I think that there is value in specifying it.

First, this is an optional part of the spec, i.e. a server doing TLS termination may choose not to expose these variables. It may also choose to expose only a subset of them. This is the use case I'm targeting, as only 3 of the SSL_CLIENT_* interests me for now, and I don't see a use-case (yet) for the rest.

As a user / lib implementer, I'd appreciate if there'd be some agreement on the shape of the SSL keys, which is where I believe the rack spec can help. Otherwise (and I believe this may be what's happening in practice) proxies are configured with "x-smth-smth-cert-*" headers to the service, and there will be different tools which cannot compose because they specify different requirements for headers.

This change is motivated by implementing mtls client auth for rodauth-oauth. Without this change, I'll have to document different setups for app/proxy servers, as people who come from a different toolchain may just have different expectations. This is the type of confusion that the rack spec eliminated for years.

@ioquatix
Copy link
Member

ioquatix commented Feb 3, 2023

@MSP-Greg @nateberkopec wdyt?

@nateberkopec
Copy link

I'm not really interesting in implementing this in Puma. I see the use-case as too limited and the API surface pretty broad. I agree that I'm not seeing the advantages of consistency between implementations, either.

@HoneyryderChuck
Copy link
Author

HoneyryderChuck commented Feb 10, 2023

I'm not really interesting in implementing this in Puma.

And that's fine. As I mentioned earlier, "First, this is an optional part of the spec". It's not "all or nothing". The spec states above "The environment is required to include these variables
(adopted from {PEP 333}[https://peps.python.org/pep-0333/]), except when they'd be empty". This also refers PEP 333, which refers the same variables.

I see the use-case as too limited and the API surface pretty broad.

I don't argue about it being limited, but mutual TLS should still be a use-case to consider. Bear in mind that it is a requirement in certain compliance frameworks and enterprise scenarios. webrick has supported it for almost a decade btw.

I didn't understand what you meant by API surface being broad though, can you clarify?

I agree that I'm not seeing the advantages of consistency between implementations, either.

I did give a use-case of something I'm planning to ship next week above. And while it's fair to say "one use-case does not make a spring", I'd also counter-argue that there is already enough deviation in the wild due to this indefinition of the spec, but enough evidence of what's being proposed here, that at least speccing could be useful in building common middleware based on the use case. Again, this is where I see "rack the spec" being a useful tool.

Anyway, I think I argued enough about this, and if we didn't get to a simple agreement here about these 3 lines of spec yet, I don't think I can provide any more useful insight to change anyone's minds. So feel free to do what you think it's best with the MR.

@ioquatix
Copy link
Member

I will revisit this when I implement HTTP/3, as it doesn't use OpenSSL in the same way. There may be more general value in that case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants