-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Client certificate support #2956
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
base: develop
Are you sure you want to change the base?
Client certificate support #2956
Conversation
The default nginx 444 response drops the inbound connection without sending any response to the client.
OpenSSL data parsing could be confused when parsing certificates which have Country/Org and other parameters in the subject line. This is fixed by writing a more robust parser of the output lines, and using that to do parsing which now correctly handles this case.
Add initial support for managing Client Certificate Authority public certificates as certificate objects in the database. The new provider type 'clientca' is defined to implement this.
The frontend is modified to filter certificates from selector lists so only non-clientca certificate types can be set as server certificates.
d53f73e
to
a18f0c2
Compare
Client certificate support is added as a new separate type of option for access-lists. This commit is the support code to enable access-lists to contain Client Certificate references.
457dd90
to
0cc6480
Compare
FYI the CI build is failing because the API returns new fields that are not defined in the Swagger/OpenAPI document:
|
0cc6480
to
15b629e
Compare
|
0aa73d6
to
1558f99
Compare
This commit adds the basic support necessary to produce the combined client CA files when certificates are updated.
When an access list contains client CAs, the combined CA auth file is added to all location blocks via an `if` statement. This allows LetsEncrypt and other support paths to work, while correctly denying access to the protected resources.
1558f99
to
a3ed464
Compare
drop_unauthorized returns 444 when a client is not authorized as opposed to 403. It can be used with Client Certificate authorization.
a3ed464
to
f3c7409
Compare
This commit changes access-list IP directives to be implemented using the nginx "geo" directive. This allows IP-based blocks to return 444 (drop connection) on authorization failure when the "Drop Unauthorized" is enabled. It also allows the implementation of "Satisfy Any" with the new client CA certificate support - i.e. Satisfy Any can allow clients from the local network to skip client certificate challenge, or drop down to requesting basic authentication. It should be noted that including basic authentication requirements in Satisfy Any mode does prevent a 444 response from being sent, as the basic auth challenge requires the server to respond.
LibreSSL uses a different output separated and semantics, which broke the X509 parser. With some slight modifications both can be supported.
d04c6e5
to
0969cd7
Compare
I keep getting 400 Bad Request: The SSL certificate error. Is there anyway I can turn on debugging or access Nginx config settings? I read that changing the |
Any further progress on this? |
Okay did some more tests - the only access list issue is that if you have "Satisfy All" set, and no IPs in the Access List, then it's treated as a default failure which is not the original behavior. Should be easy enough to fix. |
IP access list control was implemented as default success for an empty access control list - but this had the effect of an empty list default allowing if "Satisfy Any" was set. Fortunately this was bugged, so empty lists default failed - but this broke empty lists for "Satisfy All". This patch is the correct fix: lists now always default fail, but an empty list removes the check from access control considerations. This restores the original implementations behavior and fixes the bug.
And fixed! It was one potentially serious bug ("Satisfy Any" and an empty IP list led to a default allow) which I never actually found because the template I wrote was bugged. This has been fixed so IP lists now always default to fail, and an empty IP control list simple removes the check from being parsed in the host (which is the correct solution). This removes the regression in behavior so I'd be content saying this is now finished. |
Docker Image for build 16 is available on DockerHub as Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes. |
Is this feature planned to get merged? I would absolutely love this without having to use a hacky workaround! |
@wrouesnel Will, you legend !! now this is what I was looking for to lock down the nginx to a device with a cert :) but now that I have this up and running, I created ca,server and client x509 certs and installed them in the nginx and in the phone but all I get is 403 error any special requirements for the x509 certs?? do you have a write up on creating the certs ?? I guess I am creating them wrong or using the wrong ones in the nginx prox manager client access cert |
@oziee I'd need more information to know what's not working. I do know that you can wind up having this pop up when you don't do the Android certificate load just right (I think in some cases I had to convert the certificate, and then add a pin in order to get Android to pick it up). |
Oh android only.. I'm iOS |
It should also work on iOS. Maybe it's a little bit more difficult to deploy the client certificate on iOS. I used Apple Configurator and created a profile with the cert to deploy in the past. To do so you'll have to create a new profile and then add the cert to it. To create the certs I use easyrsa which is IMHO the easiest way to create the ca and certs. |
I think Cloudflare through the tunnel is playing some part maybe.. In the network I can curl with the cert and works but then I might be doing something very wrong with the cert that I install on the phone |
@jc21 any chance this might find it's way in? |
@wrouesnel very nice work with this! Thanks alot. I just have a question, i have "satisfy any" with certificate and IP ranges, and it kinda works. If I'm on WAN it will require me to preset a valid client certificate, and my LAN is on the Allow access list. I'm not required to present a client certificate when I'm on the LAN but the certificate picker comes up every time. Is this by design? I feel like it should parse the allow list first, and if it finds a match for the current IP it won't "require" a client certificate when satisfy any is active. I mean, it's not a very big deal, but i think it would be cleaner if the certificate picker didn't show when I'm on my LAN. BR |
It's a limitation of the "optional" certificate challenge that implements it. Because nginx can ask for the cert it does. So your browser asks you if you want to present a certificate. Your browser should remember you clicking "no" and then nginx will allow you through anyway if another challenge matches. |
I see, and yes it will allow me in with even if i click no. But you're right, it should remember it, I've just spent the night in incognito mode while testing this out :) So in in a real use case it shouldn't matter that much. As a bonus test-case, it seems that applications that don't support client certificates are able to get access (if IP are in allow list) without any interaction about certificates. Thanks again 👍 |
@jc21 can this feature please be merged. |
I would really like to see this feature merged. As far as I can see it would be ready to. @jc21 Is there something standing in the way? |
Would also like to see this merged. |
I noticed the current PR doesn't build, and the test image created by JC21 no longer works as the python library versions have moved on and are incompatible. I've re-based the PR changes onto the latest develop in this branch here: https://github.com/metahertz/nginx-proxy-manager/tree/client_certificate_support and have a working test image on dockerhub here: metahertz/nginx-proxy-manager@sha256:7ec29fb342080b7810753fcc562fe0017d7750678f414291e8a684f676b61e41 Asking @wrouesnel @jc21 if they could take a look over the rebase and let me know what you'd like me to do with the branch :) Thanks! |
this very useful feature and awesome, @jc21 there is any plan to merge it? |
Also want a merge on it! @wrouesnel @jc21 |
any updates on this? |
Just bumping a request to get this pulled in! I've been running the pull request docker on this for sometime now! I don't feel great working on an old code base. @jc21 can we get it pulled in? |
I switched to Caddy in the meantime. Generally I love graphical UI's and will gladly choose the tool that provides one over one with only file based config. I just love to click with the mouse ;) .
Maybe someday I will return to NPM (maybe for v3?). But for now, Caddy also seems a very manageable candidate |
If you're willing to tinker with config files then you don't need Caddy at all. NPM is just nginx with a fancy code of paint. |
Yes I knew that. I already saw someone achieve mTLS. But if I have to deal with config files anyway, I prefer to do it on a system that is designed with that in mind. Otherwise I would have to remember what I changed in the background. Also the Advandced-Tab and custom locations I find to be very inconsistent and confusing at times. I had to constantly deal with errors and review the actual config files to see what NPM was generating. I wish somewhere was doumented more clearly, where the custom content will be placed and what other consequences this has. Or even just the functionality to review the config of a host in the UI without having to explore the backend. And on top of this, I was quite disappointed by the lack of reaction by the maintainers on a pull request. As far as I can see, @wrouesnel presented this feature on a silver tablet with quite some effort and declared it finished in Aug 2023. And since then @jc21 never reacted even once to the countless requests for merging this. |
I'm using Caddy instead of NPM because this feature is mandatory imo. Please get it merged. |
I've done the same thing, move to Caddy. I deploy caddy though using --watch on the config file, and present the config file using code-server (https://github.com/coder/code-server), which is basically VSCode on the browser. This means that I can edit the config file using a UI and have it automatically reload. |
This PR adds client-certificate support to nginx-proxy-manager. Closes #768. Relates to #622.
A new SSL certificate is defined - "client certificate authority" - which allows uploading client CA certificates. These can then be assigned to Access Lists via the UI or API, and finally the Access List assigned to a host, which will thus enable Client Certificate Authorization for mutual TLS connections to the host.
This includes a slight revamp of the access-list system to implement client IP checks as
geo
directives. This allows the "Drop Unauthorized" function to simply not respond to clients from the wrong IP address, as well as allowing "Satisfy All" and "Satisfy Any" to include Client CA functionality - namely, usingSatisfy Any
is it possible to selectively require client certificates from some networks but not others (in my household the primary use-case of this is for Home Assistant to require certificates from the internet but not the local network).Known Issues