Skip to content

Conversation

@jjijack
Copy link
Contributor

@jjijack jjijack commented Feb 8, 2024

I began to use this script recently and installed the latest acme with default settings. However, it seems that acme has updated (like this) and the current version of deploy_freenas.py doesn't work on my device. So I add suffix "_ecc" in line 52&53 to stay in line with the latest acme format.

Seems acme has updated and the previous deploy_freenas.py doesn't work on my TrueNAS Core, which is the latest version. So I add segment"_ecc" in line 52&53 to stay in line with the latest acme format.
@jjijack jjijack force-pushed the patch-1 branch 2 times, most recently from 3373002 to 3aa4025 Compare February 9, 2024 06:37
@danb35
Copy link
Owner

danb35 commented Feb 18, 2024

the current version of deploy_freenas.py doesn't work on my device.

Of course it will--it's just that the default paths used by this script for the cert and key no longer match the defaults used by acme.sh, so you'd need to specify them in your config file. Not that updating the script to correctly handle the new defaults is a bad thing, if it can be done without breaking existing installations (which simply changing the script's defaults would have).

But I'm wondering about the logic of your patch. It looks like:

  • Set PRIVATEKEY_PATH to the contents of privkey_path from the config file, or if that isn't set there, to /root/.acme.sh/$DOMAIN/$DOMAIN.key
  • Try to open the resulting path
  • If that fails, set PRIVATEKEY_PATH to the contents of privkey_path from the config file, or if that isn't set there, to /root/.acme.sh/$DOMAIN_ecc/$DOMAIN.key

...and then do the same for the full chain. Am I following it correctly? Because if so, it seems like the second attempt use the setting from the config file is kind of redundant.

@danb35 danb35 linked an issue Feb 18, 2024 that may be closed by this pull request
@jjijack
Copy link
Contributor Author

jjijack commented Feb 19, 2024

Not that updating the script to correctly handle the new defaults is a bad thing, if it can be done without breaking existing installations (which simply changing the script's defaults would have).

I understand, so I added try/except structure to handle both old and new format later. I believe the update won't break existing installations.

Am I following it correctly? Because if so, it seems like the second attempt use the setting from the config file is kind of redundant.

Yes you are right. I copied and pasted part of the code, and it's indeed a little redundant. I can optimize the code to make it more elegant.

Use os.path.isfile, rather than try open to determine whether a file exists.
Now the config would only be read once.
Update deploy_config.example to contain both new and old acme route
@danb35 danb35 merged commit d60b0f3 into danb35:master Feb 20, 2024
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.

acme.sh defaults to ECC certs, default path changed

2 participants