Remove external OTP dependency from deploy hook Synology_DSM.sh

[Eagle3386]

Also adapt to DSM 7's API improvements.

See #2727 for further details.

[wyxls]

@Neilpang Hello, Thank you so much for this project.

Could this PR be merged into master branch and be released in recent days because it's a problem that syno's dsm OS doesn't have oathtool and related dependencies in it.

Syno user really need this to auto deploy the certs for the accounts that have enabled two-step authtication.

[Neilpang]

the SYNO_TOTP_SECRET is totally removed in this pr, how does the exising users renew the cert?

if you make change to the code, keep compatible with the existing users.

Imagine that the existing users are just using a cronjob to renew the cert, we must make sure the existing certs can be renewed automatically without human involved.

There is only one exception that: the api changed, and the exising code can not work anymore, so we have to change the code.

[Eagle3386]

the SYNO_TOTP_SECRET is totally removed in this pr, how does the exising users renew the cert?

This is intentionally, because it's one of two reasons for the whole change: getting rid of storing the OTP secret for good - just like you can via the web UI by ticking "Remember this device", but via Synology's official API instead of the UI.

if you make change to the code, keep compatible with the existing users.

Due to change in login procedure for enhanced comfort - as well as security due to removal of the previous requirement to store the secret in plain text on disk - that's simply impossible. We fundamentally switch from one login method that can be considered "legacy" to a modern one that supports OTP as well as non-OTP for API scenarios like ours.

Imagine that the existing users are just using a cronjob to renew the cert, we must make sure the existing certs can be renewed automatically without human involved.

Again, this is impossible in this case. Even more so, the breaking change is deliberately & intentional as it simply adapts to Synology's modernized API & its improved security.

There is only one exception that: the api changed, and the exising code can not work anymore, so we have to change the code.

Yes, Synology changed the API upon release of DSM 7. They only included the legacy login method for backward compatibility. However, security is more important than supporting insecure, legacy login methods - which is why I explicitly refrained from supporting the old behavior.

Nonetheless, I happily update the FAQ / Wiki as soon as this PR got merged - just as I updated the deploy API plug-in's file header with improved docs regarding its usage:

# Usage:
# 1. export SYNO_Username="adminUser"
# 2. export SYNO_Password="adminPassword"
# Optional exports (shown values are the defaults):
# - export SYNO_Certificate="" to replace a specific certificate via description
# - export SYNO_Scheme="http"
# - export SYNO_Hostname="localhost"
# - export SYNO_Port="5000"
# - export SYNO_Device_Name="CertRenewal" - required for skipping 2FA-OTP
# - export SYNO_Device_ID=""              - required for skipping 2FA-OTP
# 3. acme.sh --deploy --deploy-hook synology_dsm -d example.com

[evilhamsterman]

I agree with @Eagle3386, this is a breaking change, but it's for good reason. The legacy method is problematic to setup and maintain, frequently breaking. There's no guarantee that it will continue to be support, in fact it will likely be removed in a future major update of DSM. The new API is the way forward and will require breaking changes to support.

[wyxls]

I have merged the new 'deploy/synology_dsm.sh' manually and tried using it. I think it's acceptable for those existing synology DSM users. The old method to deploy certs via SYNO_DID or SYNO_TOTP_SECRET with two-step authentication enabled account is too far from easy.

It's much more simple than before:

1. export SYNO_Username and SYNO_Password
2. fill in the OTP once when it prompts

Just like login via DSM WebUI and only run it once.

[Eagle3386]

(…) It's much more simple than before:

1. export SYNO_Username and SYNO_Password
2. fill in the OTP once when it prompts

Just like login via DSM WebUI and only run it once.

I'd like to clarify that the mentioned OTP code prompt needs to be answered only once - any future execution of the deploy script then utilizes the received device ID for the actual deployment of the certificates.

This fulfills 3 requirements at once:

1. Synology's requirement regarding secure logins of any 3rd party API consumer
2. Provide the maximum comfort to any user of this script - especially regarding automation, i. e. via cron
3. Remove dependencies on external libraries which required even more interference with the system, up to the point of "restoring" them upon Synology DSM updates/-grades, or installation of otherwise unused packages, e. g. Docker package which surely isn't used by every ACME.sh & Synology user.

[wyxls]

I see. The trusted device ID is stored and it's convenient for automatic deploy after the certs is renewed.

[Eagle3386]

Exactly! 😎

@Neilpang Anything further to address from my side or can you, due to explanations/reasoning given above, accept this PR? Because I'd then update the Wiki right away in order to explain the changed usage, why it's better (especially regarding security) & how any legacy user can easily adopt to the new workflow within less than 2 minutes. 🥳

[Neilpang]

if the cert was first issued by the old credentials, it MUST be renewed with the same credentials too. This is the compatiblity. Unless the old credentials/api was shutdown, it can not be renewed anymore.

I understand you want to get rid of the old api, but compatiblity is the FIRST rule.

Imagine you configured a server with acme.sh, and it has been run well for years. but one day the ssl cert is not renewed in time, what do you think?

please make your code:

1. check if the old credentials exist, use the old methods to renew. You can print a warning message to notfify the user to upgrade.
2. otherwise, check and use the new api.

[Eagle3386]

While I disagree with your compatibility point (if Synology changes the API tomorrow, user complaints then would be pretty much the same as those caused by this PR), I consider your suggestion of issuing a warning a valid alternative.

I'll edit my PR accordingly within the next couple of days.

However, let me be clear regarding support of your mentioned "old methods": I will not support any additional Docker or goathtool approach - only the initial one, i.e. the one using oathtool will be supported. This is 100% in line with the old deploy script & therefore the only officially supported TOTP method thus far.

[evilhamsterman]

@Neilpang We never know when Synology might pull the legacy API as far as I can tell they've never made a commitment to when they'll EOL that. Like @Eagle3386 said if Synology breaks it users would be in the same mess, so it would be better to do it now. I don't think there'd be very many production Synology systems using the current method as it is so weird and fragile, I know I wouldn't. Most likely it's homelab stuff, If someone can figure out the existing script they shouldn't have much difficulty figuring out the new one.

[Neilpang]

If the legacy API will ever be closed, then it's not our fault. Our goal is to keep our code as compatible as long as we can.

You can modify the usage wiki to only describe your new credentials and usages, so that anyone from now on would use the new methods.

But we must support to renew the cert which was ever issued by the old methods.

So, I think I'm clear:

1. edit the wiki to only introduce the new usage.
2. when issuing/renewing a cert, if new credentials exists, try use the new methods, otherwise, if the old credentials exist, try to use the old methods as before and show a warning message to tell the user to upgrade to use the new method. Otherwise, no credentials found, just return error code.
3. if one day the old api is closed, we can clean the code to remove all the old methods.

[Eagle3386]

@Neilpang Finally had some free time to complete this, but found that even the old method using OTP was calling the Synology API wrong: https://github.com/acmesh-official/acme.sh/blob/41b6aebe7cf2d3bd12b062d78519818f6aaf099f/deploy/synology_dsm.sh#L105-L111

It's wrong, because

1. it puts &device_id=$SYNO_DID at the POST request's URL end - which the API only allows for API calls omitting the TOTP code after using a different login API call with an TOTP code, see Example 3 & Example 4 over at the API docs for further details.
2. It actually defines enable_syno_token=yes twice: in the middle of the URL query string & right after the ? as part of the "base" URL - while it should've define enable_device_token=yes for that second time instead.

However, since you requested that old setups should continue "as is", I reintroduced these misuses of the API - complaints due to errors will be completely redirected to you! 🤪

Also, there's neither a _warn nor __yellow function built-in, hence I can't show a warning. I can either use _err or the pretty easily ignored _info, but since the former would fail any deployment attempt for those old setups, I opted for the latter instead - much against my own creed regarding good coding!

[alidaf]

Any idea when this is likely to see the light of day? The latest version I see is 9th June. How are the instructions likely to change - will it still be shell based? I found the instructions for determining SYNO_DID and SYNO-TOTP_SECRET completely confusing, the latter being possibly browser/OS dependent.

[Eagle3386]

Regarding your "instructions request", be reassured that I'll update the wiki page as soon as Neil published the new version.

Besides that:

• yes, the new script is still shell-based
• if you don't touch anything, your script will continue to work as before
• if you want to switch, you'll have to delete 2 lines from your config (updated wiki page will include them, too)
• using the newer variant, you'll have to run the script once from the shell & only provide the TOTP code for the account you desire to be used for the cert renewal process (with sufficient privileges, that is) once
• each subsequent run (even that one already described in the wiki page for running a custom Task Scheduler script) will then use the token(s) provided by Synology's modern API after that very first "enter TOTP code once"-run
• hence no more storing the secret anywhere within your account config & ultimately as weel as massively improved security for you 🥳

[alidaf]

Thank you. This is actually something I've only recently started with partial success by following this guide, mostly due to the 2-factor authentication. Somehow, quite by chance, I managed to get a deployment of 2 certificates but renewals don't work. I think the process was muddied a lot by there being multiple guides with different flavours (dockers, native etc) and procedures but trying to go through the process again with what seems like this, more official (and more accessible) guide, fails at the deployment, so I'm keen for the new version so I can try again.

[jaydee73]

@Eagle3386 : New version was released last night (at least the docker repo got an update). I would love to see an updated wiki to got it integrated in my environment.

I would be very happy if the wiki will also reflect existing installations. Just like "I have a running environment based on the old method, what is necessary now to migrate over to the new method". You are writing "using the newer variant, you'll have to run the script once from the shell". I think you mean the script with parameter "--deployhook" by this? Because in an existing installation there is only the parameter --renewal to be used. This is not completely clear for me atm, so it would be nice if the wiki can cover this part. Probably @alidaf would agree. ;-)

And alidaf, maybe this guide helps you to get it going or at least to understand the background a little bit better: https://www.christosgeo.com/2022/02/03/renew-lets-encrypt-certificates-on-synology-using-acme-sh/ That's what I used to build it on my Syno. And afaik many others too...it is often referenced as a guide...so maybe you already knew...

[alidaf]

I would be very happy if the wiki will also reflect existing installations. Probably @alidaf would agree. ;-)

I would indeed!

[Eagle3386]

@alidaf Please see Deploy the certificate to Synology DSM for updated instructions on how to automate deployment with 2FA enabled.

@jaydee73 Same as for @alidaf, however, regarding already running setups: yes, they're covered as well, but instructions for those are intentionally dropped completely. Migration is pretty simple: delete the lines containing SAVED_SYNO_DID & SAVED_SYNO_TOTP_SECRET from your account's configuration file (located at acme.sh/example.net[_ecc]/example.net.conf) & then follow the update wiki's entry as described there.

Regarding the command line parameters: --renewal only renews the certificate, it does not deploy the renewed certificate's files. You have to use --renewal --deploy --deploy-hook synology_dsm to automatically deploy any renewed certificate.

Regarding your linked guide: it uses a Docker container while the wiki page - the old just as the new one - talk about ACME.sh running on your actual system, i. e. inside /usr/local/share/acme.sh

[jaydee73]

@Eagle3386 Thanks for the input. I'll try to work through it the coming days. One thing already popped up: You wrote, that after deleting the two lines (DID und TOTP), we should follow the wiki instructions. You mean: From top to bottom? Point 3 says "acme.sh --deploy --deploy-hook synology_dsm -d example.com". Do we really have to go through this deploy-stuff again, even if we already have an existing certificate? If so, how can me make sure that the script does not create a new (additional!) certificate to the existing one?

And on top of that, we have a new issue (https://github.com/acmesh-official/acme.sh/issues/4721) saying that 3.0.7 synology hook is broken. Are you aware of this? Is this because of your updated stuff?

[Eagle3386]

@jaydee73

1. Yes, from top to bottom, but one-time only.
2. Stop complaining about setting up deployment for subsequent automated deployment - I just fixed insecure, outdated stuff from the original deploy hook author & made it like it was supposed to be done from the beginning!
3. If 3-4 little steps are too much to ask for, then go, look for help elsewhere. I'm done with users of such kind.
4. You don't seem to understand even the very basics of ACME.sh - point 3 is deployment only, hence it doesn't even care, if issuance or renewal happened before.
5. The issue you mentioned clearly has an ECC certificate problem: the user runs on DSM 6 (reported API version is 6 instead of 7) & 6 can't do ECC certificates.
6. The hook is not broken, it just needs to be used correctly - I'm using the exact same script code for an ECC wildcard certificate & neither issuance nor renewal had any problem succeeding.

[jaydee73]

Holy moly, this was not what I wanted to achieve. I'm sorry. That probably happens when one (=me!) is not using his native language. Daher einmal ganz kurz: Ich wollte dir definitiv nicht auf den Schlips treten. Ganz im Gegenteil, ich bin dankbar für deine Hilfe.

It doesn't matter for me if there are 2 steps, 20 steps or 200 steps to be done. It's simply that I want to make sure (in advance!) to do the right steps. And maybe also help others with the same questions. This wasn't meant to be a complain at all.

You are totally right, step 3 ist only about deploying. My bad, I should have paid better attention.

Also thanks for clarifying the other issue. Wasn't clear for me that he still sits on DSM6.

Sorry again! I hope pulse is slowly getting back to normal... :)

[Eagle3386]

Yeah, that helped, clarify a lot & settle things back to normal. - "Schwamm drüber!" 😉

Also, I'm here to help with further questions, if any new arise.

[jaydee73]

Strange...I made a new comment some hours ago, but for whatever reason it isn't visible....ok, I'll try again...:

Yesterday I tried to implement the new method on my already running system. Unfortunately I failed. I deleted the two old conf-file lines (SYNO DID and SYNO TOTP) and added the two new lines (Device name & device ID) to my conf file. Small info: I am using the official acme.he docker container. Every time (I have tried several times...) I executed the script with the deploy hook (directly on the shell), it still used the old method and stated, that is is deprecated. No asking for an otp code. Restart of the container also didn't change anything.

Then I started from scratch and installed a new container. Issuing (again directly on the shell) went without problems. Deploy also acted as expected. I was asked for an otp code, entered it and the certificate was successfully deployed to the Syno. So far so good.

Nevertheless two questions arised: a) I defined 'export SYNO_Device_Name="CertRenewal"' in my conf file as proposed. Regardless of that, I was asked by the script to enter a device name. I assume this is expected behaviour? b) I also added the line 'export SYNO_Device_ID=""' to my conf file. The wiki says, the script requests the Device ID from the API and saves it to the conf file. But unfortunately nothing was added to my conf file. I would expect a line like 'export SYNO_Device_ID="value"'. I assume, this missing line would cause trouble when renewal time comes?

In general, my conf file indeed does "work". Yesterday I also added Gotify and executed some "export" commands directly in the shell. They were correctly added to my conf file.

So, yes, some new help would be appreciated. Big thanks!

[wyxls]

@jaydee73 The new script will first check the conf file for SYNO_* envs needed, then check&load current running shell's SYNO_* envs, throw errors if there's no such envs. SYNO_Username and SYNO_Password are required and others are optional. So.....

a. I think it's an expected behaviour. Now the script will only check if there's defined "SYNO_Device_ID" even there's already a "SYNO_Device_Name" exist. It doesn't matter anything unless you manually change "SYNO_Device_Name" which doesn't match "SYNO_Device_ID". Related code shoule be below: [ -z "${SYNO_Device_ID:-}" ]; then printf "Enter OTP code for user '%s': " "$SYNO_Username" read -r otp_code

b. Yes, it might be trouble when redeployment happens because the script will ask for "SYNO_Device_ID".

Run "export" command will add env variable into current running shell to make sure the new script could load it for deployment. Basically, Device ID is generated by Syno's API while the script successfully pass 2FA authentication unless you already have one. Related code should be below: response=$(_get "$_base_url/webapi/entry.cgi?api=SYNO.API.Auth&version=$api_version&method=login&format=sid&account=$encoded_username&passwd=$encoded_password&otp_code=$otp_code&enable_syno_token=yes&enable_device_token=yes&device_name=$SYNO_Device_Name") _debug3 response "$response" SYNO_Device_ID=$(echo "$response" | grep "device_id" | sed -n 's/.*"device_id" *: *"\([^"]*\).*/\1/p') _secure_debug2 SYNO_Device_ID "$SYNO_Device_ID"

If "SYNO_Device_ID" is not defined in advance, the script will add SAVED_SYNO_Device_ID='blahblahblah' into conf file for future redeployment once the deployment succeed. After that, you will no longer be required to enter anything when redeployment happens.

[Eagle3386]

Thanks, @wyxls for explaining most of the stuff already. 👍🏻

@jaydee73, to make it absolutely clear:

1. Don't add anything manually into your configuration file, but do delete those 2 lines mentioned.
2. The deploy script will add those 2 lines for you, because it must login with your admin's credentials & TOTP code in order to do any subsequent login without TOTP code.
3. That first deployment call must be done manually, i.e. run acme.sh --deploy --deploy-hook synology_dsm (…), input the TOTP code (& optionally a device name, if you don't like the default CertRenewal) & wait for the deployment to succeed.
4. Any subsequent deployment, i.e. when running via Synology DSM's Task Scheduler as the NAS guide explains, will then use a token (retrieved via previously mentioned manual run) & hence neither require another TOTP code nor the TOTP secret to generate a new TOTP code via some 3rd party tool.
5. You don't define export SYNO_Device_Name="CertRenewal" inside the configuration file, but execute that very statement as a command (export is a shell command, not a shell language keyword 😉).
6. Again, the device ID is written to the configuration file only after a successful certificate deployment.

[jaydee73]

Thanks for all your support & patience, guys. But in the end, it didn't work out for me...again...

I really followed all your steps....nothing added to conf file, deleted the two lines, manually run acme.sh for deploy, entered TOTP code and device name.

The script successfully deployed the certificate [Wed Aug 9 20:37:52 UTC 2023] Success The certificate is visible in Syno GUI, so I do assume that everything went the right way.

But again, no device ID was written to my conf file after deployment.

This seems to be rocket science, or at least my very basic knowledge isn't nearly enough to get it going... :-(

I'll probably wait for some other guys who are also using this script in docker. Maybe they'll have the same problem or maybe it's working for them and they have an additional hint for me. Until then I'll stick with the "old" and less secure "DID method".

Thanks again, especially to Eagle3386.

[Eagle3386]

(…) The script successfully deployed the certificate [Wed Aug 9 20:37:52 UTC 2023] Success The certificate is visible in Syno GUI, so I do assume that everything went the right way.

But again, no device ID was written to my conf file after deployment.

Hold on here for a second, please. If the deployment succeeds, yet no update to the configuration file occurs, then it might be a simple permission problem.

This seems to be rocket science, or at least my very basic knowledge isn't nearly enough to get it going... :-(

They put humans onto the moon with far less code, let alone computational power - if certificate deployment / its configuration file update is more complex, then we're officially outta space here! 🥳🤣

I'll probably wait for some other guys who are also using this script in docker. Maybe they'll have the same problem or maybe it's working for them and they have an additional hint for me. Until then I'll stick with the "old" and less secure "DID method".

Please, provide us with a link to the container info page as well as the debug log snippet after deployment into DSM succeeded, but prior to the deploy script finishing its work (use --debug 3 and remove every line that's outside said scope).

I'm thinking of either a permission problem (as stated above) or maybe a container configuration problem & want to analyze this further.

Thanks again, especially to Eagle3386.

You're welcome & thanks for your acknowledgement. 😇

[jaydee73]

I will provide the informations you need this evening.

One info in advance. Several other "hooks" (is this the right expression?) can write (an do so) into the conf file:

--set-default-ca --issue --notify-hook => They all write into my conf file. All the commands are executed in the same shell. And of course --deploy --deploy-hook... is also executed in this shell. So my basic knowledge wouldn't think of a general permission problem of the container. But of course I do not know how the syno deploy script works...

Hopefully we can figure this out together.

[Eagle3386]

That's strange, because as you can see here: https://github.com/acmesh-official/acme.sh/blob/56cf93dff25c09540ebe1984040af6938c3f1046/deploy/synology_dsm.sh#L160-L163 device name & also ID are saved just like username & password - which all use ACME.sh's _savedeployconf function.

However, the script not only works on my DSM 7.2, but on many others as well: there are/were 3 users with issues thus far - 2 were solved within a day or so (1 was due to outdated DSM, the other calling the wrong API path on DSM 6), hence your issue is the only remaining one.

Can you - just for testing, of course - remove the lines for username & password as well, re-run the deployment script (you might need to re-run export SYNO_Username="YourUserName" & export SYNO_Password="YourPassword" in advance, too) & report back if username & password do get written into the file?

Because if they reappear within the file, it might be a value issue, but if not, then there might be a more serious issue here…