Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report bugs to RouterOS deploy API #2344

Open
cngarrison opened this issue Jun 18, 2019 · 35 comments
Open

Report bugs to RouterOS deploy API #2344

cngarrison opened this issue Jun 18, 2019 · 35 comments
Assignees
Labels
3rd party api report bugs to dns api, deploy hooks and notification hooks

Comments

@cngarrison
Copy link
Contributor

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

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

Thanks!

@Neilpang Neilpang added the 3rd party api report bugs to dns api, deploy hooks and notification hooks label Jun 19, 2019
@herbetom
Copy link

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
Copy link
Contributor Author

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
Copy link

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
Copy link

I created a pull request (#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
Copy link

herbetom commented Oct 17, 2019

@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
Copy link

@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
Copy link

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link

@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
Copy link
Contributor

abiessmann commented Feb 19, 2022

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
Copy link
Contributor

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
Copy link
Contributor

sjtuross commented Feb 19, 2022

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

@abiessmann
Copy link
Contributor

@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
Copy link
Contributor

sjtuross commented Mar 17, 2022

@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.

DEPLOY_SCRIPT_CMD="/system script add name=\"LE Cert Deploy - $_cdomain\" owner=$ROUTER_OS_USER \

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

@abiessmann
Copy link
Contributor

@sjtuross Fixed in #3986 ...

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

@sjtuross
Copy link
Contributor

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
Copy link
Contributor

abiessmann commented Mar 17, 2022

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 ;)

@sjtuross
Copy link
Contributor

sjtuross commented Mar 17, 2022

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
Copy link

bjmgeek commented Mar 17, 2022

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link

bjmgeek commented Mar 18, 2022

@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
Copy link

bjmgeek commented May 22, 2023

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
Copy link

bjmgeek commented May 22, 2023

#4637 is a pull request for this issue.

@dario-pilori
Copy link
Contributor

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.

@shpokas
Copy link

shpokas commented Jan 29, 2024

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
Copy link
Contributor

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
Copy link
Contributor Author

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

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

@schumi4
Copy link

schumi4 commented Feb 24, 2024

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
Copy link

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
Copy link
Contributor

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.

#5245

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party api report bugs to dns api, deploy hooks and notification hooks
Projects
None yet
Development

No branches or pull requests