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

Update Synology DSM deploy hook #2935

Merged
merged 6 commits into from
May 16, 2020
Merged

Conversation

tresni
Copy link
Contributor

@tresni tresni commented May 16, 2020

A couple of reported bugs in #2727 that are addressed in this PR.

  1. URL encoding of all URL params (52b8160)
  2. wget support is fixed (d15c14a)

Also fixed an unreported issue where the error message isn't what it should be if certificate is not found and $SYNO_Create was not set (668967a).

Tested from an OSX machine to virtual DSM running DSM 6.1 with --use-wget/ACME_USE_WGET=1 and with curl. Both were able to create a certificate if not present and update an existing certificate.

I'm actually not entirely sure why/how this worked with curl but not wget, but it did.  The short answer is that using a GET does not result in the HTTP_HEADER file being written, instead you must pass in the http_headers param ($2) which will return the HTTP headers as a string.  Luckily, the Token is in both the body and the header.  We need it and the id (and smid if 2fa) cookie to proceed.  So now we parrse the response for that instead of the HTTP_HEADER file.

Interesting side note: wget is fine if the URL contains a \r or \n, but curl will barf on it.  So we need to make sure those are stripped from the token as it will be passed in the URL later.
@auto-comment
Copy link

auto-comment bot commented May 16, 2020

First, NEVER send a PR to master branch, it will NEVER be accepted. Please send to the dev branch instead.
If this is a PR to support new DNS API or new notification API, please read this guide first: https://github.com/acmesh-official/acme.sh/wiki/DNS-API-Dev-Guide
Please check the guide items one by one.
Then add your usage here: https://github.com/acmesh-official/acme.sh/wiki/dnsapi

Or some other wiki pages:
https://github.com/acmesh-official/acme.sh/wiki/deployhooks
https://github.com/acmesh-official/acme.sh/wiki/notify

This means, by default, we will rotate the default certificate that comes with the DSM
SYNO_Certificate gets set by _getdeployconf, so this may be an empty string but that's fine
@Neilpang Neilpang merged commit 8780ba3 into acmesh-official:dev May 16, 2020
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.

None yet

2 participants