acmesh-official / acme.sh

A pure Unix shell script implementing ACME client protocol
https://acme.sh
GNU General Public License v3.0
39.38k stars 4.97k forks source link

Report bugs to RouterOS deploy API #2344

Open cngarrison opened 5 years ago

cngarrison commented 5 years ago

This is the place to report bugs in the RouterOS deploy API.

If you experience a bug, please report it in this issue.

Thanks!

herbetom commented 5 years ago

Not really a bug, more a feature request, but only having a single RouterOS installation is probably quite uncommon, any posssibility to deploy to multiple instances of RouterOS?

cngarrison commented 5 years ago

I like the idea, but let's flesh it out a bit more. You want a wildcard cert that is deployed to multiple routers? Or one cert per router?

The first should be easy to add by passing a list for ROUTER_OS_HOST (would assume same value across all routers for ROUTER_OS_USERNAME and ROUTER_OS_ADDITIONAL_SERVICES) and looping over the list. I'll consider adding that (PR's are also welcomed).

For the second case it should be doable now; just do another acme.sh run with different values for ROUTER_OS_* vars.

herbetom commented 5 years ago

Thanks for the quick reply. Well, usually i don't use Wildcard Certificates, but the idea to specify a list of hosts would be definitely interesting.

I have to admit, that while i wrote the first post, i only had tried the fritzbox deployhook. That hook saves the host, username and password within the account.conf. That's in my opinion not so nice, especially if you have multiple targets to deploy to and the reason for my comment. I will try to get that changed.

But now, after i loocked into the diffrent deploy script's, i noticed, that the handling of how and if at all such configurations are saved is diffrent from script to script.

Since i now know that ROUTER_OS_HOST, ROUTER_OS_USERNAME and ROUTER_OS_ADDITIONAL_SERVICES aren't saved at all, i see that theire is not really a problem deploying to multiple routeros installations (like you said), since the vars need anyway be set before running the deploy script. I would really like it, if the vars would be saved in the domain configuration (like in the ssh-deploy-hook). That seems to me to be the best solution to me.

After i now tried to use the script for the first time i now got the first bug. Yeah ;)

I used the command acme.sh --deploy -d router.example.com --deploy-hook routeros --debug on a raspbian installation and got the following output:

[Do 25. Jul 04:57:41 CEST 2019] Lets find script dir.
[Do 25. Jul 04:57:41 CEST 2019] _SCRIPT_='/root/.acme.sh/acme.sh'
[Do 25. Jul 04:57:41 CEST 2019] _script='/root/.acme.sh/acme.sh'
[Do 25. Jul 04:57:41 CEST 2019] _script_home='/root/.acme.sh'
[Do 25. Jul 04:57:41 CEST 2019] Using config home:/root/.acme.sh
https://github.com/Neilpang/acme.sh
v2.8.2
[Do 25. Jul 04:57:41 CEST 2019] Using config home:/root/.acme.sh
[Do 25. Jul 04:57:41 CEST 2019] ACME_DIRECTORY='https://acme-v02.api.letsencrypt.org/directory'
[Do 25. Jul 04:57:41 CEST 2019] DOMAIN_PATH='/root/.acme.sh/router.example.com'
[Do 25. Jul 04:57:41 CEST 2019] _deployApi='/root/.acme.sh/deploy/routeros.sh'
[Do 25. Jul 04:57:41 CEST 2019] _cdomain='router.example.com'
[Do 25. Jul 04:57:41 CEST 2019] _ckey='/root/.acme.sh/router.example.com/router.example.com.key'
[Do 25. Jul 04:57:42 CEST 2019] _ccert='/root/.acme.sh/router.example.com/router.example.com.cer'
[Do 25. Jul 04:57:42 CEST 2019] _cca='/root/.acme.sh/router.example.com/ca.cer'
[Do 25. Jul 04:57:42 CEST 2019] _cfullchain='/root/.acme.sh/router.example.com/fullchain.cer'
[Do 25. Jul 04:57:42 CEST 2019] Not enabling additional services
[Do 25. Jul 04:57:42 CEST 2019] Trying to push key '/root/.acme.sh/router.example.com/router.example.com.key' to router
router.example.com.key                                                                 100% 1675   239.1KB/s   00:00
[Do 25. Jul 04:57:43 CEST 2019] Trying to push cert '/root/.acme.sh/router.example.com/fullchain.cer' to router
fullchain.cer                                                                         100% 3596   172.2KB/s   00:00
syntax error (line 1 column 7)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
expected command name (line 1 column 1)
[Do 25. Jul 04:57:47 CEST 2019] Success

First of all, clearly i wasn't a success. But since their is no validation of the output of the ssh commands it's not surprising and more a beauty thing and also not that easy to validate.

I got it to working by removing the quotation marks arround $DEPLOY_SCRIPT_CMD in the deploy script line 104 that's all it needed, so it seems to be some sort of escaping or var handling problem.

I hope this can be fixed somehow. If i have a quiet moment in the next time i will try to examine this closer and make a PR, but i can't promise anything.

herbetom commented 5 years ago

I created a pull request (https://github.com/Neilpang/acme.sh/pull/2413) for the ability to store the env vars inside the domain conf, fixed some errors like the line breaks in the routeros script which weren't working, and made it suitable for my use by making it possible to prevent setting the certificate for the www-ssl service. I need that because i use multiple certificates with multiple subdomains on my Router. The first for the management part, the second for the sstp-server, and the third for hotspot server.

herbetom commented 5 years ago

@cngarrison For me the deploy hook for routeros still isn't working. I'm trying to deploy a cert from a Raspbian 9 (stretch) to a Mikrotik RouerOS v6.45.6

Is the deployhook working for you with v6.45.6? I'm wondering if only i have that problem or if i'm the only one trying/reporting?

Nickmman commented 5 years ago

@herbetom I was able to get this working, I edited the source= part of the routeros script to add \'s after each line:

source=\"## generated by routeros deploy script in acme.sh; \
\n/certificate remove [ find name=$_cdomain.cer_0 ]; \
\n/certificate remove [ find name=$_cdomain.cer_1 ]; \
\ndelay 1; \
\n/certificate import file-name=$_cdomain.cer passphrase=\\\"\\\"; \
\n/certificate import file-name=$_cdomain.key passphrase=\\\"\\\"; \
\ndelay 1; \
\n/file remove $_cdomain.cer; \
\n/file remove $_cdomain.key; \
\ndelay 2; \
\n/ip service set www-ssl certificate=$_cdomain.cer_0; \
\n$ROUTER_OS_ADDITIONAL_SERVICES; \
\n\"
"

There's probably a cleaner way to do it, but that's how it's working for me right now. I also added ;'s after each "command" since their Wiki says that it marks the end of the command line, it may or may not be needed.

herbetom commented 5 years ago

Thanks for your response @Nickmman! I will try it. Back in July I already tried to create a pull request (#2413) with something that worked back then for me (no idea if if still works, probably, but I haven't tested it). The main problem with my solution was, that the automatic travis ci check failed. But with your "variety" maybe it will pass the check.

cngarrison commented 4 years ago

@Nickmman Please create a PR with your changes; hopefully it will pass the CI tests (specifically the shfmt test) and if so then I may be able to work out how to merge the PR from @herbetom so that is passes tests too.

cngarrison commented 4 years ago

Is the deployhook working for you with v6.45.6? I'm wondering if only i have that problem or if i'm the only one trying/reporting?

@herbetom I upgraded to 6.45 and can confirm the same problem - fixed using the trailing slash for each line in the deploy script.

sjtuross commented 2 years ago

Firstly I can confirm routeros deployment hook still works on 6.49.2 and 7.1.1, but I don't see the required environment variables are saved in conf. When renew by cron happens, the deploy hook won't work, right?

I see this related PR #2413, but it's not merged.

herbetom commented 2 years ago

@sjtuross Yeah, I haven't checked if something has changed, but that's how I remember it.

Feel free to take whatever you need from my #2413 so that you can implement a solution. But I don't know it that's any help 😅

As for the new syntax: since the old syntax still works on both (new and old) Router OS Versions it should probably stay in the old format for now since Routers with Version 6 are probably still the vast majority.

abiessmann commented 2 years ago

I encountered some issues when trying to use the routeros deploy hook with my fresh RB5009 (still running 7.0.5).

My setup is dockerized acme.sh and therefore I have an unusual path for ssh identity, so I had to hack it to be able to pass -i <identity> -o UserKnownHostsFile=.... My approach would be to have the ssh and scp commands configurable like the Le_Deploy_ssh_cmd of the ssh deploy hook. Mainly to be able to also switch port for both, cause ssh uses -p while scp uses -P.

Second thing is the RouterOS script. I'm not familiar with routeros scripting but had problems in the first place. I now have working adoption for my setup including user derived from $ROUTER_OS_USER, removed policy and moved the first line of script to comment.

Third is that fullchain.crt installs three certificates while the script is only deleting two of them, dunno if this is a problem.

Please see and comment #3947

abiessmann commented 2 years ago

Just saw another change in dev by @mac-zhou. He introduced ROUTER_OS_PORT which was not in my base ... and came without mention in this thread ...

We should discuss, whether to have different parameters for identity, ssh options (think of jump hosts ...) and the existing ROUTER_OS_PORT or just to have the command configurable (as the ssh deploy hook does).

sjtuross commented 2 years ago

@abiessmann you can map a folder or volume with id_rsa and known_hosts to "/root/.ssh" in the container.

abiessmann commented 2 years ago

@sjtuross that would be another solution. However, I have managed the identities and the ssh configuration for the ssh deploy hook in the acme.sh configuration so far. So I thought #3947 would be a good idea ...

sjtuross commented 2 years ago

@abiessmann there is a bug introduced in this PR #3947 that breaks deployment. I traced down to the below line today. I think the variable name should be $ROUTER_OS_USERNAME instead of $ROUTER_OS_USER. Currently the generated DEPLOY_SCRIPT_CMD is broken due to missing owner value.

https://github.com/acmesh-official/acme.sh/blob/9ebb2ac2e4e0e5489700721c1ec9729ac27de5cd/deploy/routeros.sh#L133

P.S. It would be good to add _debug "$DEPLOY_SCRIPT_CMD" to make debug easier.

abiessmann commented 2 years ago

@sjtuross Fixed in #3986 ...

unfortunately the ssh command seems to not report an error on failing script execution. Any thoughts on this?

sjtuross commented 2 years ago

Thanks. Not sure what you mean about ssh error. Below is what I got. After certs were pushed to ros, the next deploy command failed and then script run/delete command returned "no such item".

[Thu Mar 17 00:59:25 UTC 2022] Trying to push cert '/acme.sh/ros.example.com/fullchain.cer' to router
expected end of command (line 1 column 80)
no such item
no such item
[Thu Mar 17 00:59:26 UTC 2022] Success
abiessmann commented 2 years ago

Thanks. Not sure what you mean about ssh error. Below is what I got. After certs were pushed to ros, the next deploy command failed and then script run/delete command returned "no such item".

[Thu Mar 17 00:59:25 UTC 2022] Trying to push cert '/acme.sh/ros.example.com/fullchain.cer' to router
expected end of command (line 1 column 80)
no such item
no such item
[Thu Mar 17 00:59:26 UTC 2022] Success

@sjtuross as you can see in your response the overall result is 'Success' ...

Please see https://github.com/abiessmann/acme.sh/commit/0fdb5ef66bc144c14b02875b6e3680ddeb1f526b for a possible solution ... feedback welcome ;)

sjtuross commented 2 years ago

Thanks. Not sure what you mean about ssh error. Below is what I got. After certs were pushed to ros, the next deploy command failed and then script run/delete command returned "no such item".

[Thu Mar 17 00:59:25 UTC 2022] Trying to push cert '/acme.sh/ros.example.com/fullchain.cer' to router
expected end of command (line 1 column 80)
no such item
no such item
[Thu Mar 17 00:59:26 UTC 2022] Success

@sjtuross as you can see in your response the overall result is 'Success' ...

Please see abiessmann@0fdb5ef for a possible solution ... feedback welcome ;)

It's because it simply returns 0 as the return code. Ideally it should check the command result and return non-zero return code.

bjmgeek commented 2 years ago

I'm having the same issue. I tried a brand new install of acme.sh and got the above error (unexpected end of command) but on line 1 column 83. I'm using RouterOS 6.49.2

abiessmann commented 2 years ago

I'm having the same issue. I tried a brand new install of acme.sh and got the above error (unexpected end of command) but on line 1 column 83. I'm using RouterOS 6.49.2

@bjmgeek could you please verify latest fix in #3986 (merged to https://github.com/acmesh-official/acme.sh/tree/dev) fixes your issue?

abiessmann commented 2 years ago

Please see abiessmann@0fdb5ef for a possible solution ... feedback welcome ;)

It's because it simply returns 0 as the return code. Ideally it should check the command result and return non-zero return code.

@sjtuross you are completely right! Could you please check my branch https://github.com/abiessmann/acme.sh/tree/deploy_routeros_handle_remote_errors and check if this works for you and will detect those erroneous situations?

sjtuross commented 2 years ago

@abiessmann I tried your branch. The error handling works as expected with the original bug. I think you could update the wording like Submitting sequence of commands to routeros Error code 1 returned from routeros because it's not really related to ssh.

[Fri Mar 18 07:36:16 UTC 2022] Push key '/acme.sh/ros.example.com/ros.example.com.key' to router
[Fri Mar 18 07:36:16 UTC 2022] Push key '/acme.sh/ros.example.com/fullchain.cer' to router
[Fri Mar 18 07:36:17 UTC 2022] Submitting sequence of commands to remote server by ssh
expected end of command (line 1 column 84)
[Fri Mar 18 07:36:17 UTC 2022] Error code 1 returned from ssh
[Fri Mar 18 07:36:17 UTC 2022] Error deploy for domain:ros.example.com
[Fri Mar 18 07:36:17 UTC 2022] Deploy error.
abiessmann commented 2 years ago

@abiessmann I tried your branch. The error handling works as expected with the original bug. I think you could update the wording like Submitting sequence of commands to routeros Error code 1 returned from routeros because it's not really related to ssh.

@sjtuross thank you for review and comment --> #3989

bjmgeek commented 2 years ago

@abiessmann That branch worked.

dev laptop:~/.acme.sh$ acme.sh --ecc --deploy -d router.example.com --deploy-hook routeros
[Fri Mar 18 08:34:38 AM EDT 2022] Trying to push key '/home/bminton/.acme.sh/router.example.com_ecc/router.example.com.key' to router
router.example.com.key                                                                                                                         100%  227    96.4KB/s   00:00    
[Fri Mar 18 08:34:39 AM EDT 2022] Trying to push cert '/home/bminton/.acme.sh/router.example.com_ecc/fullchain.cer' to router
fullchain.cer                                                                                                                                        100% 5345     1.4MB/s   00:00    
     certificates-imported: 3
     private-keys-imported: 0
            files-imported: 1
       decryption-failures: 0
  keys-with-no-certificate: 0

     certificates-imported: 0
     private-keys-imported: 1
            files-imported: 1
       decryption-failures: 0
  keys-with-no-certificate: 0
bjmgeek commented 1 year ago

I'm having a new routeros deploy issue now:

pops-mintonw10:~$ acme.sh --deploy -d '*.example.com' --deploy-hook routeros [Mon May 22 09:32:10 AM EDT 2023] Push key '/home/bminton/.acme.sh/*.example.com/*.example.com.key' to routeros *.example.com.key 100% 1675 469.9KB/s 00:00 [Mon May 22 09:32:11 AM EDT 2023] Push key '/home/bminton/.acme.sh/*.example.com/fullchain.cer' to routeros fullchain.cer 100% 5601 1.1MB/s 00:00 [Mon May 22 09:32:11 AM EDT 2023] Submitting sequence of commands to routeros failure: item with such name already exists [Mon May 22 09:32:12 AM EDT 2023] Error code 1 returned from routeros [Mon May 22 09:32:12 AM EDT 2023] Error deploy for domain:*.example.com [Mon May 22 09:32:12 AM EDT 2023] Deploy error.

I'm guessing it's because of the wildcard certificate. Indeed, I tested the same configuration, with the only difference being -d 'router.example.com' instead of -d '*.example.com' and it worked.

I've set the ROUTER_OS_HOST, ROUTER_OS_PORT, and ROUTER_OS_USERNAME variables, and I have an ssh key set up for the router.

bjmgeek commented 1 year ago

https://github.com/acmesh-official/acme.sh/pull/4637 is a pull request for this issue.

dario-pilori commented 10 months ago

The deploy hook doesn't work with the latest version of RouterOS (v7.13). Apparently, the issue is that RouterOS doesn't allow whitespaces in script names.

This pull request https://github.com/acmesh-official/acme.sh/pull/4940 should fix the issue.

shpokas commented 9 months ago

The deploy hook doesn't work with the latest version of RouterOS (v7.13). Apparently, the issue is that RouterOS doesn't allow whitespaces in script names.

This pull request #4940 should fix the issue.

RouterOS v7.13 and later accepts whitespaces in script names just fine, but the deploy script now creates a script on MIkrotik device with underscores in the script name, but then tries to execute script with whitespaces in the name...

This looks wrong.

dario-pilori commented 9 months ago

RouterOS v7.13 and later accepts whitespaces in script names just fine, but the deploy script now creates a script on MIkrotik device with underscores in the script name, but then tries to execute script with whitespaces in the name...

This looks wrong.

I agree, this is not a consistent behavior for RouterOS, and it might be reported to MikroTik as a bug. If you manually create a script, the console complains with the message:

/system/script/add name="my script" source=":put 2" Warning: name was corrected to my_script

This can be interpreted as "RouterOS does not support whitespaces", and it "helps" you by adding automatically underscores. But this "autocorrect" feature is not present in the script execution...

IMHO, I still think that the best option is removing all whitespaces.

cngarrison commented 9 months ago

...but then tries to execute script with whitespaces in the name...

What are you seeing that indicates it's executing script with whitespaces?

schumi4 commented 8 months ago

So I understand correctly that the RouterOS deployhook is broken with acme.sh 3.0.8 and ROS 7.13.5?

[Sat Feb 24 22:15:18 CET 2024] Submitting sequence of commands to routeros
failure: item with such name already exists
[Sat Feb 24 22:15:18 CET 2024] Error code 1 returned from routeros
[Sat Feb 24 22:15:18 CET 2024] Error deploy for domain:*.REDACTED.TLD
[Sat Feb 24 22:15:18 CET 2024] Deploy error.
kolegacik commented 4 months ago

There is an issue where certificate import removes certificate file, so there is no need for manual removal within ROS script. When deploy script fails, it leaves ROS script in place, so any subsequent deploy fails.

Running acme.sh 3.0.8 and ROS 7.15.

My edit of ROS script:

  DEPLOY_SCRIPT_CMD="/system script add name=\"LECertDeploy-$_cdomain\" owner=$ROUTER_OS_USERNAME \
comment=\"generated by routeros deploy script in acme.sh\" \
source=\"/certificate remove [ find name=$_cdomain.cer_0 ];\r\
\n/certificate remove [ find name=$_cdomain.cer_1 ];\r\
\n/certificate remove [ find name=$_cdomain.cer_2 ];\r\
\ndelay 1;\r\
\n/certificate import file-name=$_cdomain.cer passphrase=\\\"\\\";\r\
\n/certificate import file-name=$_cdomain.key passphrase=\\\"\\\";\r\
\ndelay 2;\r\
\n/ip/service set www-ssl certificate=\\\"$_cdomain.cer_0\\\";\r\
\n$ROUTER_OS_ADDITIONAL_SERVICES\r\
\n\"
"
nathanejohnson commented 3 months ago

I just added a PR that should address some of the issues people are seeing with newer routerOS versions, and should be backwards compatible. This should address the issue @kolegacik mentions, as well as the one @schumi4 mentions.

https://github.com/acmesh-official/acme.sh/pull/5245