client certificate validation

[4xoc]

This PR implements client certificate validation for TLS based web server instances of Gitea. When CA_FILE is set in the server section, the contents of that file (one or more PEM encoded CA certs) is used to validate a client certificate sent by the user when establishing the TLS channel. The use of client certificate becomes mandatory once the setting is used for all requests going to the web server.

Existing configuration is not affected by this change. It's not expected this breaks anything for users not actively using this new feature.

No information of the client certificate is used for authentication or authorization within Gitea outside the TLS channel. Although it is possible to use certificate information to log in users automatically this is not implemented.

[4xoc]

Hi @lunny, any chance this can be merged soonish?

[lunny]

Hi @lunny, any chance this can be merged soonish?

I think we need some tests.

[zeripath]

I'm genuinely not sure of the use of this without adding in an authentication layer using the subject layer in the certificate.

Is that an intended follow-up to this PR?

[4xoc]

No it is not. For my use-case the simple validation against a specific CA is enough. There's still the usual authentication happening just like now. The main benefit is that, with public reachable services that yet should be limited in access have a secure way of allowing access to such service. A possible attacker couldn't even abuse a security issue within Gitea because the tls connection isn't even established.

[denyskon]

@4xoc Are you still interested in this PR? Could you add a test for it as requested previously?

[4xoc]

What would you like to see being tested here? Generally I'm not to sure what makes sense to really test here, other than maybe validating that tlsConfig actually contains some matching data and the config is parsed correctly. Everything else should already be in the tls and x509 packages.

[denyskon]

@lunny Could you clarify what you'd like to see to be tested?

[lunny]

@lunny Could you clarify what you'd like to see to be tested?

Looks like it's not easy to have a test for the listening. Then maybe someone could have a manually test.

[denyskon]

@4xoc Could you please resolve the conflicts? I'd then test the feature manually and probably approve the PR

[lafriks]

Personally I don't see added value for this to be implemented in gitea directly if it does not add actual user authentication. This can easily be implemented already using pretty much any reverse proxy in front of gitea

[denyskon]

I think it is okay to offer it directly, not everyone wants to use a reverse proxy. And after all this isn't a lot of code to be added.

[4xoc]

@lafriks I get your point but the scope of doing user authentication is much larger than simply having the TLS engine take care of establishing the secure connection. Personally I don't have the need for authentication with certs thus this isn't part of the PR. I may look into this in the future or if there's genuine interest in that feature. Otherwise I'd not like to add code to authentication that is essentially not used by anyone and turns out to be a security risk in the future because nobody took care of it.

[denyskon]

I tested the feature manually and it works as expected

[4xoc]

When can this finally be merged? The lack of progress is honestly quite annoying for a simple ans small PR like this. It's over a year now that this PR has been opened and every time I rebase it's just getting stale yet again. If something's missing or needs to be done please say so but the sheer lack of communication is unacceptable for such a small change.

[lafriks]

@4xoc reverse proxy in front of gitea can also do client certificate authentication, so I see no added value for gitea if this does not result into actual user authentication.

Examples:
https://caddyserver.com/docs/json/apps/http/servers/tls_connection_policies/client_authentication/
http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_client_certificate
https://httpd.apache.org/docs/2.4/ssl/ssl_howto.html#allclients
etc

[wxiaoguang]

I do not see enough benefit either by introducing this patch without a complete design.

For such spacial use case, reverse proxy could do well.

[Wizmaster]

Hello @4xoc
I'm a bit late in the discussion, I think your PR would be a useful addition in Gitea even without being able to properly authenticate the end users. It would allow Gitea to run behind a reverse-proxy and enable mTLS connection between the proxy and Gitea.
As far as I saw in Gitea documentation, it is currently not possible to do that. Your PR would enable that scenario. (and I would be happy it see it in Gitea then 😄)

[4xoc]

Hey @Wizmaster, I'm sorry to say but you're probably too late for the party. I've lost any interest in this PR and will not pick it up anymore mainly because I don't see any chance this change getting through. I'm fully aware this sounds salty and arrogant, but I've got better things to do than chase a small PR for 1y+ to end up getting nowhere.

[Wizmaster]

No worries @4xoc! I quite understand.
If I find the time, I'll try to submit a new PR to add client cert support in Gitea.