Skip to content

Commit

Permalink
acme: merge cli into init script
Browse files Browse the repository at this point in the history
Signed-off-by: Glen Huang <i@glenhuang.com>
  • Loading branch information
Glen Huang authored and tohojo committed Mar 1, 2023
1 parent e93a9d0 commit c6960a2
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 171 deletions.
10 changes: 4 additions & 6 deletions net/acme-common/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ define Package/acme-common/install
$(INSTALL_DIR) $(1)/etc/ssl/acme
$(INSTALL_DIR) $(1)/etc/config
$(INSTALL_CONF) ./files/acme.config $(1)/etc/config/acme
$(INSTALL_DIR) $(1)/usr/bin
$(INSTALL_BIN) ./files/acme.sh $(1)/usr/bin/acme
$(INSTALL_DIR) $(1)/usr/lib/acme
$(INSTALL_DATA) ./files/functions.sh $(1)/usr/lib/acme
$(INSTALL_BIN) ./files/acme-notify.sh $(1)/usr/lib/acme/notify
Expand All @@ -50,15 +48,15 @@ define Package/acme-common/install
$(INSTALL_DIR) $(1)/etc/hotplug.d/acme
endef

define Package/acme/postinst
define Package/acme-common/postinst
#!/bin/sh
grep -q '/usr/bin/acme' /etc/crontabs/root 2>/dev/null && exit 0
echo "0 0 * * * /usr/bin/acme get" >> /etc/crontabs/root
grep -q '/etc/init.d/acme' /etc/crontabs/root 2>/dev/null && exit 0
echo "0 0 * * * /etc/init.d/acme start" >> /etc/crontabs/root
endef

define Package/acme-common/prerm
#!/bin/sh
sed -i '\|/usr/bin/acme|d' /etc/crontabs/root
sed -i '\|/etc/init.d/acme|d' /etc/crontabs/root
endef

define Build/Configure
Expand Down
132 changes: 130 additions & 2 deletions net/acme-common/files/acme.init
Original file line number Diff line number Diff line change
@@ -1,9 +1,137 @@
#!/bin/sh /etc/rc.common

START=80
USE_PROCD=1
run_dir=/var/run/acme
export CHALLENGE_DIR=$run_dir/challenge
export CERT_DIR=/etc/ssl/acme
NFT_HANDLE=
HOOK=/usr/lib/acme/hook
LOG_TAG=acme

# shellcheck source=net/acme/files/functions.sh
. /usr/lib/acme/functions.sh

cleanup() {
log debug "cleaning up"
if [ -e $run_dir/lock ]; then
rm $run_dir/lock
fi
if [ "$NFT_HANDLE" ]; then
# $NFT_HANDLE contains the string 'handle XX' so pass it unquoted to nft
nft delete rule inet fw4 input $NFT_HANDLE
fi
}

load_options() {
section=$1

# compatibility for old option name
config_get_bool staging "$section" use_staging
if [ -z "$staging" ]; then
config_get_bool staging "$section" staging 0
fi
export staging
config_get calias "$section" calias
export calias
config_get dalias "$section" dalias
export dalias
config_get domains "$section" domains
export domains
export main_domain
main_domain="$(first_arg $domains)"
config_get keylength "$section" keylength ec-256
export keylength
config_get dns "$section" dns
export dns
config_get acme_server "$section" acme_server
export acme_server
config_get days "$section" days
export days
config_get standalone "$section" standalone 0
export standalone
config_get dns_wait "$section" dns_wait
export dns_wait

config_get webroot "$section" webroot
export webroot
if [ "$webroot" ]; then
log warn "Option \"webroot\" is deprecated, please remove it and change your web server's config so it serves ACME challenge requests from $CHALLENGE_DIR."
fi
}

first_arg() {
echo "$1"
}

get_cert() {
section=$1

config_get_bool enabled "$section" enabled 1
[ "$enabled" = 1 ] || return

load_options "$section"
if [ -z "$dns" ] && [ "$standalone" = 0 ]; then
mkdir -p "$CHALLENGE_DIR"
fi

if [ "$standalone" = 1 ] && [ -z "$NFT_HANDLE" ]; then
if ! NFT_HANDLE=$(nft -a -e insert rule inet fw4 input tcp dport 80 counter accept comment ACME | grep -o 'handle [0-9]\+'); then
return 1
fi
log debug "added nft rule: $NFT_HANDLE"
fi

load_credentials() {
eval export "$1"
}
config_list_foreach "$section" credentials load_credentials

"$HOOK" get
}

load_globals() {
section=$1

config_get account_email "$section" account_email
if [ -z "$account_email" ]; then
log err "account_email option is required"
exit 1
fi
export account_email

config_get state_dir "$section" state_dir
if [ "$state_dir" ]; then
log warn "Option \"state_dir\" is deprecated, please remove it. Certificates now exist in $CERT_DIR."
mkdir -p "$state_dir"
else
state_dir=/etc/acme
fi
export state_dir

config_get debug "$section" debug 0
export debug

# only look for the first acme section
return 1
}

start_service() {
mkdir -p $run_dir
exec 200>$run_dir/lock
if ! flock -n 200; then
log err "Another ACME instance is already running."
exit 1
fi

trap cleanup EXIT

config_load acme
config_foreach load_globals acme

config_foreach get_cert cert
}

service_triggers() {
procd_add_config_trigger config.change acme \
/usr/bin/acme get
/etc/init.d/acme start
}
160 changes: 0 additions & 160 deletions net/acme-common/files/acme.sh

This file was deleted.

4 changes: 2 additions & 2 deletions net/acme-common/files/acme.uci-defaults
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/sh

grep -q '/usr/bin/acme' /etc/crontabs/root 2>/dev/null && exit 0
echo "0 0 * * * /usr/bin/acme get" >> /etc/crontabs/root
grep -q '/etc/init.d/acme' /etc/crontabs/root 2>/dev/null && exit 0
echo "0 0 * * * /etc/init.d/acme start" >>/etc/crontabs/root
2 changes: 1 addition & 1 deletion net/acme-common/files/functions.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
log() {
prio="$1"
shift
if [ "$prio" != debug ] || [ "$debug" = 0 ]; then
if [ "$prio" != debug ] || [ "$debug" = 1 ]; then
logger -t "$LOG_TAG" -s -p "daemon.$prio" -- "$@"
fi
}

14 comments on commit c6960a2

@LGA1150
Copy link
Contributor

@LGA1150 LGA1150 commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/home/ubuntu/openwrt/build_dir/target-aarch64_cortex-a53_musl/root-mediatek/usr/lib/opkg/info/acme-common.postinst-pkg: line 3: /etc/crontabs/root: No such file or directory
/home/ubuntu/openwrt/build_dir/target-aarch64_cortex-a53_musl/root-mediatek/etc/init.d/acme: line 12: /usr/lib/acme/functions.sh: No such file or directory
postinst script ./usr/lib/opkg/info/acme-common.postinst has failed with exit code 1
make[2]: *** [package/Makefile:73: package/install] Error 1

@tohojo
Copy link
Contributor

@tohojo tohojo commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hgl PTAL

@hgl
Copy link
Contributor

@hgl hgl commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this from building a firmware image?

If so, the build system seems to access /etc/crontabs/root and /usr/lib/acme/functions.sh directly on the host environment?

@tohojo Any idea how that could happen?

@LGA1150
Copy link
Contributor

@LGA1150 LGA1150 commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hgl Yes

@hgl
Copy link
Contributor

@hgl hgl commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the image builder, and it's unlikely this PR has caused the shown error, since how /etc/crontabs/root is written was not changed, and /usr/lib/acme/functions.sh was not deleted (multiple people reported the issue since this PR got merged).

It'd be helpful if someone could help answer the following questions:

  1. Does the image builder just directly run a package's postinst when it builds the firmware?
  2. Does the image builder also run a package's init script when it builds the firmware?
  3. If so, are the scripts just run from the root of the filesystem without any chroot?

These are my guesses on where the error stemmed from, but I'd be very surprised if the answer to all of them are yes.

@zcy85611
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've met the same error, no change made on any files, just compile as usual, please have look, thanks

@hgl
Copy link
Contributor

@hgl hgl commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tohojo Do you think it has something to do with "start" being available?

@zxlhhyccc
Copy link

@zxlhhyccc zxlhhyccc commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/home/ubuntu/openwrt/build_dir/target-aarch64_cortex-a53_musl/root-mediatek/usr/lib/opkg/info/acme-common.postinst-pkg: line 3: /etc/crontabs/root: No such file or directory
/home/ubuntu/openwrt/build_dir/target-aarch64_cortex-a53_musl/root-mediatek/etc/init.d/acme: line 12: /usr/lib/acme/functions.sh: No such file or directory
postinst script ./usr/lib/opkg/info/acme-common.postinst has failed with exit code 1
make[2]: *** [package/Makefile:73: package/install] Error 1

https://github.com/openwrt/packages/blob/master/net/acme-common/Makefile#L42

@tohojo
Copy link
Contributor

@tohojo tohojo commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at some other postinst examples, they all seem to have checks like: if [ -z "$${IPKG_INSTROOT}" ]; then around all their logic. So yeah, looks like the script itself does need to deal with this.

@hgl
Copy link
Contributor

@hgl hgl commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, that means the answer to my three questions are yes. Will try to send the PR tomorrow. I guess that also means we can get rid of the uci-defaults.

@tohojo
Copy link
Contributor

@tohojo tohojo commented on c6960a2 Mar 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there are two ways to go about it:

  1. Have the postinst script put the crontab entry into "$IPKG_INSTROOT/etc/crontabs/root" so it'll be shipped as part of the image

  2. Have the postinst script bail out if $PKG_INSTROOT is set, and have the uci_defaults take care of setting the crontab entry on first boot

  3. would probably require also creating the /etc/crontabs directory if it doesn't exist; but otherwise I don't really have any strong opinion on which method is better...

@Fail-Safe
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hgl Have you been able to make any headway on this?

@hgl
Copy link
Contributor

@hgl hgl commented on c6960a2 Mar 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Fail-Safe fix was merged into master, can you test if it’s addressed?

@Fail-Safe
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hgl Just re-built my image and it is back to working again. Many thanks!

Please sign in to comment.