openwrt / packages

Community maintained packages for OpenWrt. Documentation for submitting pull requests is in CONTRIBUTING.md
GNU General Public License v2.0
3.92k stars 3.44k forks source link

luci-app-acme: "Apply to nginx" does not work. #20698

Open Curious-r opened 1 year ago

Curious-r commented 1 year ago

Steps to reproduce:

  1. Install luci-app-acme by offical feed
  2. Modify the certificate configuration and check "Apply to nginx"
  3. Save and apply

Actual behavior:

  1. Certs was successfully issued in /etc/acme, but it was not applied to nginx.

Expected behavior:

  1. Certs apply to nginx rightly

Additional Information:

OpenWrt version information from system /etc/openwrt_release

DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='SNAPSHOT'
DISTRIB_REVISION='r0-e467856'
DISTRIB_TARGET='ipq40xx/generic'
DISTRIB_ARCH='arm_cortex-a7_neon-vfpv4'
DISTRIB_DESCRIPTION='OpenWrt SNAPSHOT r0-e467856'
DISTRIB_TAINTS='no-all'

The firmware is with luci-ssl-nginx, and all dependencies of luci-app-acme was installed.

Logs:

Sun Mar 19 23:15:17 2023 daemon.info acme-acmesh: Running ACME for example.com
Sun Mar 19 23:15:17 2023 daemon.info acme-acmesh: /usr/lib/acme/client/acme.sh --ecc -d example.com --keylength ec-384 --accountemail example@example.com --server letsencrypt --dns dns_cf --issue --home /etc/acme

Config of acme:

config cert 'example'
        option enabled '1'
        option use_staging '0'
        option keylength 'ec-384'
        list domains 'example.com'
        option update_nginx '1'
        option validation_method 'dns'
        option dns 'dns_cf'
        list credentials 'CF_Token="***************************"'
        list credentials 'CF_Account_ID="***************************"'
        list credentials 'CF_Zone_ID="***************************"'

Files in /etc/acme/gateway.example.com_eccafter certs issued:

root@Suzhou:/etc/acme/suzhou.gateway.curious.host_ecc# ls
ca.cer                                example.com.cer       example.com.cer                          example.com.conf      example.com.key
fullchain.cer                         example.com.csr

uci show nginx:

nginx.global=main
nginx.global.uci_enable='true'
nginx._lan=server
nginx._lan.listen='443 ssl default_server' '[::]:443 ssl default_server'
nginx._lan.server_name='_lan'
nginx._lan.include='restrict_locally' 'conf.d/*.locations'
nginx._lan.uci_manage_ssl='self-signed'
nginx._lan.ssl_certificate='/etc/nginx/conf.d/_lan.crt'
nginx._lan.ssl_certificate_key='/etc/nginx/conf.d/_lan.key'
nginx._lan.ssl_session_cache='shared:SSL:32k'
nginx._lan.ssl_session_timeout='64m'
nginx._lan.access_log='off; # logd openwrt'
nginx._redirect2ssl=server
nginx._redirect2ssl.listen='80' '[::]:80'
nginx._redirect2ssl.server_name='_redirect2ssl'
nginx._redirect2ssl.return='302 https://$host$request_uri'
Curious-r commented 1 year ago

Well, I seem to remember that I solved this problem once two years ago. We need to manually create the UCI configuration corresponding to the domain (or nginx native configuration) before ACME can help us update the certificate configuration.

Curious-r commented 1 year ago

No.. This didn't work out, as if "apply to nginx" wouldn't trigger at all. Strange, the packages from brach openwrt-22.03 work fine.

jow- commented 1 year ago

LuCI merely modifies the configuration for the underlying acme package, so the issue likely lies there. @tohojo ?

Curious-r commented 1 year ago

@hgl PTAL

The difference between acme packages in the two branches is mainly due to your refactoring. I read all the acme related code in the master branch but didn't find an implementation of nginx_update feature, can you help me look at this issue?

The acme nginx_update feature in openwrt-22.03:

    local nginx_updated
    nginx_updated=0
    if command -v nginx-util 2> /dev/null && [ "$update_nginx" -eq "1" ]; then
        nginx_updated=1
        for domain in $domains; do
            nginx-util add_ssl "${domain}" acme "${domain_dir}/fullchain.cer" \
                "${domain_dir}/${main_domain}.key" || nginx_updated=0

https://github.com/openwrt/packages/blob/openwrt-22.03/net/acme/files/run.sh

Curious-r commented 1 year ago

LuCI merely modifies the configuration for the underlying acme package, so the issue likely lies there. @tohojo ?

Thanks! I was about to update this issue. 😆

tohojo commented 1 year ago

After the refactoring, the support for modifying nginx configuration was removed entirely, on the expectation that it's better if users manually configure their web server to pick up the cert. Acme will still reload the web server (via the hook script in the updated nginx package), but you have to manually configure nginx to use the certificate, which will be symlinked to /etc/ssl/acme/$MAIN_DOMAIN.{crt,key,fullchain.crt}

Curious-r commented 1 year ago

After the refactoring, the support for modifying nginx configuration was removed entirely, on the expectation that it's better if users manually configure their web server to pick up the cert. Acme will still reload the web server (via the hook script in the updated nginx package), but you have to manually configure nginx to use the certificate, which will be symlinked to /etc/ssl/acme/$MAIN_DOMAIN.{crt,key,fullchain.crt}

What is the reason to remove the feature? It used to work very well.

tohojo commented 1 year ago

Separation of concerns, basically: as we added support for more and more daemons, this started to become a bit of a hairball of ad-hoc configuration editing scripts in the acme runner itself. So the refactoring moves to an event-based approach where acme will call hook scripts on renew. The idea being that the packages that want to support auto-configuring themselves can then add their own hook scripts to do this.

At the time of making the change we only added simple reload support to the existing daemons (so that configurations would keep working after an upgrade). And, well, no one has added the auto-config support to any of the packages thus far :)

You mention above that you needed to do some manual re-config of nginx anyway, before this change. What did that consist in, exactly?

Curious-r commented 1 year ago

Separation of concerns, basically: as we added support for more and more daemons, this started to become a bit of a hairball of ad-hoc configuration editing scripts in the acme runner itself. So the refactoring moves to an event-based approach where acme will call hook scripts on renew. The idea being that the packages that want to support auto-configuring themselves can then add their own hook scripts to do this.

At the time of making the change we only added simple reload support to the existing daemons (so that configurations would keep working after an upgrade). And, well, no one has added the auto-config support to any of the packages thus far :)

You mention above that you needed to do some manual re-config of nginx anyway, before this change. What did that consist in, exactly?

Before this change, if the server block corresponding to the FQDN exists in the nginx configuration, acme can add an SSL path to it. But if the server block is not exists, acme won't create a new one. So I have to create server block manually, by uci command or /etc/config/nginx. This is because nginx does not have a graphical configuration interface in LUCI, so these things must be done manually. If the ACME plugin can automatically configure nginx, it will be much easier to use for users who do not have the expertise.

tohojo commented 1 year ago

We never had any code that added a server block directly in acme, so I assume you were using nginx_util for this?

Curious-r commented 1 year ago

We never had any code that added a server block directly in acme, so I assume you were using nginx_util for this?

In the past, I configured it manually. I don't mean that acme can add service blocks directly, that's what I envisioned. Do you think this is a good feature? By calling the nginx_util, automatically configuring nginx when the certificate is deployed.

tohojo commented 1 year ago

Yup, this would be a reasonable thing to add, I think. The way to do it would be to add a hotplug script to the nginx_util package which would react to new acme events and run the util accordingly...

hgl commented 1 year ago

The LuCI acme package needs update. I’ve been meaning to send a PR, but haven’t gotten around to it. Hope I can squeeze out some time soon.

As for updating nginx, acme currently automatically makes nginx reload conf whenever any certificate is issued or renewed. So you can simply add the ssl directive after issuing and it will continue working after renewing.

popy2k14 commented 6 months ago

Hey guys.

I am also setting up nginx on openwrt. acme.sh is working and i have ticked the renew option in luci acme config:

image

Here is my nginx config for the server:

config server 'myserver'
         list listen '192.168.0.254:444 ssl'
         option server_name '*.XXX.duckdns.org'
         list include 'conf.d/XXX.locations'
         option ssl_certificate '/etc/acme/*.XXX.duckdns.org/*.XXX.duckdns.org.cer'
         option ssl_certificate_key '/etc/acme/*.XXX.duckdns.org/*.XXX.duckdns.org.key'
         option ssl_session_cache 'shared:SSL:32k'
         option ssl_session_timeout '64m'

XXX.location test file:

location / {
         proxy_pass              https://bitwarden.com/:443;
}

After issuing (acme.sh with --force --debug --nginx -> for testing) the following is displayed:

[Sun Feb 25 16:13:54 CET 2024] Your cert is in: /etc/acme/....cer
[Sun Feb 25 16:13:54 CET 2024] Your cert key is in: /etc/acme/....key
[Sun Feb 25 16:13:54 CET 2024] The intermediate CA cert is in: /etc/acme/.....cer
[Sun Feb 25 16:13:54 CET 2024] And the full chain certs is there: /etc/acme/....cer
[Sun Feb 25 16:13:54 CET 2024] _on_issue_success
[Sun Feb 25 16:13:54 CET 2024] Run renew hook:'/usr/lib/acme/notify renewed'
[Sun Feb 25 16:13:54 CET 2024] 'dns_duckdns' does not contain 'dns'
[Sun Feb 25 16:13:54 CET 2024] The NOTIFY_HOOK is empty, just return.

Is this actually working and nginx reloaded after acme.sh done it's magic? What makes me curious is the last message:

[Sun Feb 25 16:13:54 CET 2024] The NOTIFY_HOOK is empty, just return.

How can i check that nginx is reloaded after acme.sh runned? Just want to be prepared for the newxt renew.

thx

tohojo commented 6 months ago

popy2k14 @.***> writes:

Hey guys.

I am also setting up nginx on openwrt. acme.sh is working and i have ticked the renew option in luci acme:

image

After issuing (with --force --debug --nginx -> for testing) tzhe following is displayed:

[Sun Feb 25 16:13:54 CET 2024] Your cert is in: /etc/acme/....cer
[Sun Feb 25 16:13:54 CET 2024] Your cert key is in: /etc/acme/....key
[Sun Feb 25 16:13:54 CET 2024] The intermediate CA cert is in: /etc/acme/.....cer
[Sun Feb 25 16:13:54 CET 2024] And the full chain certs is there: /etc/acme/....cer
[Sun Feb 25 16:13:54 CET 2024] _on_issue_success
[Sun Feb 25 16:13:54 CET 2024] Run renew hook:'/usr/lib/acme/notify renewed'
[Sun Feb 25 16:13:54 CET 2024] 'dns_duckdns' does not contain 'dns'
[Sun Feb 25 16:13:54 CET 2024] The NOTIFY_HOOK is empty, just return.

Is this actually working and nginx reloaded after acme.sh done it's magic? What makes me curious is the last message:

[Sun Feb 25 16:13:54 CET 2024] The NOTIFY_HOOK is empty, just return.

These messages are all coming from acme.sh itself (I'm assuming you also ran acme.sh itself manually instead of relying on the luci init script?). The OpenWrt init script sets up its own notification hooks and calls nginx renew via procd trigger mechanism, but none of that works when you're running acme.sh manually...

How can i check that nginx is reloaded after acme.sh runned? Just want to be prepared for the newxt renew.

I don't actually know if the service reload is logged anywhere. You should be able to trigger the ubus message with this command:

ubus call service event '{"type":"acme.renew","data":{}}'

popy2k14 commented 6 months ago

@tohojo thx for your answer Yes, I have executed acme.sh from CLI to test it.

So when I got you right, the nginx renew is triggered because of the Cron job (/etc/init.d/acme restart), which runs every night.

That was I want to know 😊 The missing part was the Openwrt init.d script, which sets up the trigger.

Will see if it works in about 2 months, when the renew is triggered by acme.

Thx

stokito commented 3 months ago

The issue looks like can be closed.

popy2k14 commented 3 months ago

I am using HA now to get my nginxs cert's. So i can not say if the renew worked or not. But i think it would have worked :-)

So yes, for me it could be closed.