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

INWX support added #2659

Merged
merged 7 commits into from Jan 25, 2023
Merged

INWX support added #2659

merged 7 commits into from Jan 25, 2023

Conversation

XantXant
Copy link
Contributor

Added INWX for dns challenge.

@homeassistant
Copy link

Hi @XantXant,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

letsencrypt/rootfs/etc/services.d/lets-encrypt/run Outdated Show resolved Hide resolved
letsencrypt/rootfs/etc/services.d/lets-encrypt/run Outdated Show resolved Hide resolved
letsencrypt/config.yaml Outdated Show resolved Hide resolved
added --dns-inwx-propagation-seconds
removed inwx_url as option
@dinyar
Copy link

dinyar commented Oct 16, 2022

Hi @XantXant, @mdegat01,

I was curious if there was any progress on this MR? If you think it would be helpful I'd be happy to test it.

Cheers,
Dinyar

@XantXant
Copy link
Contributor Author

Hi @dinyar ,

I'm glad that someone needs it.
It works fine for me, but another test is always good.

I still waiting that someone approve my pull request.

Best regards
XantXant

@dinyar
Copy link

dinyar commented Oct 21, 2022

I installed it locally just now, but can't seem to find where it puts the certificate (if it successfully obtained it). Is there some way to run it in debug mode with more output?

@dvonessen
Copy link

Hello, @XantXant thanks for this contribution. I am also a proud INWX customer ;-).
Can someone explain how to install this patch into my Homeassistant instance? I am using the prebuild image from HA.
Thank you!
It would be nice if someone could merge this change.

@XantXant
Copy link
Contributor Author

XantXant commented Nov 4, 2022

Hi @dvonessen, you can install ist locally.
You need the "Samba share" Addon.
When you have configured correctly, you have a "\\homeassistant\addons" network share.
Download my repo "https://github.com/XantXant/addons/tree/inwx".
Copy the "letsencrypt" folder from my repo to the "\\homeassistant\addons" network share.
Remove the following line from "\\homeassistant\addons\letsenctrypt\config.yaml":
image: homeassistant/{arch}-addon-letsencrypt
This will build the image locally.
After that you should see a "Let's Encrypt" addon under "Local Addons" in the Add-on Store.

I'm still waiting for the finally ok for the pull request.
I hope i didn't forget anything todo for the pull request.

@dvonessen
Copy link

@XantXant thank you for your advice that works. Unfortunately the inwx_shared_secret is mandatory. Despite having 2FA enabled is good. I do use the only one which is included for my Opnsense. OPNsense doesn't allow using 2FA authentication. So I cannot share the account with HA. Is it mandatory on a technical reason or just to make it as secure as possible?

If possible I would recommend to allow using the dns-inwx provider without 2FA and have 2FA optional.

@XantXant
Copy link
Contributor Author

XantXant commented Nov 7, 2022

@dvonessen it looks like the certbot-dns-inwx plugin for certbot supports also without 2FA.

The shared secret is your INWX 2FA OTP key. It is shown to you when setting up the 2FA. It is not the 6 digit code you need to enter when siging in. If you are not using 2FA, simply keep the value the way it is.

dns_inwx_url = https://api.domrobot.com/xmlrpc/ dns_inwx_username = your_username dns_inwx_password = """your_password""" dns_inwx_shared_secret = your_shared_secret optional

Can yout test it with "your_shared_secret optional".

@dvonessen
Copy link

@XantXant thank you for your fast response.
Do you mean using your_shared_secret optional as the value of dns_inwx_shared_secret?

@XantXant
Copy link
Contributor Author

XantXant commented Nov 7, 2022

@dvonessen yes, but i`m not sure.

If you are not using 2FA, simply keep the value the way it is.

From certbot-dns-inwx plugin.
Link

If it is optional, then i should change it, so the inwx_shared_secret is not mandatory.

Maybe i can test it the next days.

@XantXant
Copy link
Contributor Author

I tested it yesterday, but i had no luck, because i have a new problem.
I rebuild the addon locally and now the addon doesn't work anymore.
Can anybody help me with this new issue?

s6-rc: info: service s6rc-oneshot-runner: starting s6-rc: info: service s6rc-oneshot-runner successfully started s6-rc: info: service fix-attrs: starting s6-rc: info: service fix-attrs successfully started s6-rc: info: service legacy-cont-init: starting cont-init: info: running /etc/cont-init.d/file-structure.sh cont-init: info: /etc/cont-init.d/file-structure.sh exited 0 s6-rc: info: service legacy-cont-init successfully started s6-rc: info: service legacy-services: starting services-up: info: copying legacy longrun lets-encrypt (no readiness notification) s6-rc: info: service legacy-services successfully started [21:37:49] INFO: Selected DNS Provider: dns-inwx [21:37:49] INFO: Use propagation seconds: 60 Traceback (most recent call last): File "/usr/local/bin/certbot", line 5, in <module> from certbot.main import main File "/usr/local/lib/python3.9/site-packages/certbot/main.py", line 6, in <module> from certbot._internal import main as internal_main File "/usr/local/lib/python3.9/site-packages/certbot/_internal/main.py", line 28, in <module> from certbot import crypto_util File "/usr/local/lib/python3.9/site-packages/certbot/crypto_util.py", line 42, in <module> from certbot import interfaces File "/usr/local/lib/python3.9/site-packages/certbot/interfaces.py", line 21, in <module> from acme.client import ClientBase ImportError: cannot import name 'ClientBase' from 'acme.client' (/usr/local/lib/python3.9/site-packages/acme/client.py) s6-rc: info: service legacy-services: stopping s6-rc: info: service legacy-services successfully stopped s6-rc: info: service legacy-cont-init: stopping s6-rc: info: service legacy-cont-init successfully stopped s6-rc: info: service fix-attrs: stopping s6-rc: info: service fix-attrs successfully stopped s6-rc: info: service s6rc-oneshot-runner: stopping s6-rc: info: service s6rc-oneshot-runner successfully stopped

Updated certbot-dns-inwx to 2.2.0
Fixed acme version to 1.32.0
Added doc for inwx without 2FA
@XantXant
Copy link
Contributor Author

Fixed acme version to 1.32.0, now it is running again.
certbot to 1.32.0
certbot_dns_inwx to 2.2.0

@dvonessen :
I tested it, it works.
You can set the shared secret to the example key from the doc.
I added it to the documentation.

@stfnx
Copy link

stfnx commented Dec 22, 2022

I'd love to see this get merged soon.
Is there anything I could do to speedup the process?

Seems like all requested changes from the first review were already made 3 months ago.

@XantXant
Copy link
Contributor Author

@stfnx :
I don't know how to get this merged.
It looks like the letsencrypt addon is a little bit unattended.
The actual source code from master would not build the addon image correctly, because of the new acme version.
(tested it with 2.0.0, perhaps it is fixed with the latest acme version 2.1.0)
You can install it locally with the samba share addon, see in this thread my message from Nov. 4.

@stfnx
Copy link

stfnx commented Dec 28, 2022

I cloned your repo and followed the steps from your message from Nov. 4.
The addon shows up as local one, but when I try to install I get the following error:

The command '/bin/ash -o pipefail -c set -x && apk add --no-cache --update libffi musl openssl && apk add --no-cache --virtual .build-dependencies build-base libffi-dev musl-dev openssl-dev cargo && pip3 install --no-cache-dir --find-links "https://wheels.home-assistant.io/alpine-$(cut -d '.' -f 1-2 < /etc/alpine-release)/${BUILD_ARCH}/" cryptography==${CRYPTOGRAPHY_VERSION} certbot==${CERTBOT_VERSION} certbot-dns-azure==${CERTBOT_DNS_AZURE_VERSION} certbot-dns-cloudflare==${CERTBOT_VERSION} certbot-dns-cloudxns==${CERTBOT_VERSION} certbot-dns-digitalocean==${CERTBOT_VERSION} certbot-dns-directadmin==${CERTBOT_DNS_DIRECTADMIN_VERSION} certbot-dns-dnsimple==${CERTBOT_VERSION} certbot-dns-dnsmadeeasy==${CERTBOT_VERSION} certbot-dns-gehirn==${CERTBOT_VERSION} certbot-dns-google==${CERTBOT_VERSION} certbot-dns-hetzner==${CERTBOT_DNS_HETZNER_VERSION} certbot-dns-linode==${CERTBOT_VERSION} certbot-dns-luadns==${CERTBOT_VERSION} certbot-dns-njalla==${CERTBOT_NJALLA_VERSION} certbot-dns-nsone==${CERTBOT_VERSION} certbot-dns-ovh==${CERTBOT_VERSION} certbot-dns-rfc2136==${CERTBOT_VERSION} certbot-dns-route53==${CERTBOT_VERSION} certbot-dns-sakuracloud==${CERTBOT_VERSION} certbot-dns-netcup==${CERTBOT_NETCUP_VERSION} certbot-plugin-gandi==${CERTBOT_GANDI_VERSION} certbot-dns-transip==${CERTBOT_DNS_TRANSIP_VERSION} certbot-dns-inwx==${CERTBOT_DNS_INWX_VERSION} acme==${ACME_VERSION} && apk del .build-dependencies' returned a non-zero code: 1

@XantXant
Copy link
Contributor Author

@stfnx sorry for my late response.
ACME_VERSION was missing in the dockerfile.
Now it builds the image correct.

@XantXant
Copy link
Contributor Author

For easier testing:
I modified the master branch of my fork repository.
Now you can add the repositiory "https://github.com/XantXant/addons" in the "Add-on Store" under repositories.
Then a "Copy of Home Assistant add-on repository" will show up with all add-on's, but only the "Let's Encrypt" add-on will work.

I still hope to get the pull request apporved.

@mdegat01 mdegat01 merged commit b734b3c into home-assistant:master Jan 25, 2023
@mdegat01
Copy link
Contributor

Thanks @XantXant 👍

@stfnx
Copy link

stfnx commented Jan 25, 2023

Thanks @XantXant for implementing it and @mdegat01 for getting this merged.
Really appreciate it!

But I found an issue. In DOCS.md it says: Use the user for the dyndns service, not the normal user. (Line 472)

However this doesn't work. The normal username and password is required.

@XantXant
Copy link
Contributor Author

Thanks @XantXant for implementing it and @mdegat01 for getting this merged. Really appreciate it!

But I found an issue. In DOCS.md it says: Use the user for the dyndns service, not the normal user. (Line 472)

However this doesn't work. The normal username and password is required.

@stfnx i will fix this with a new pull request in the coming days.
It looks like there is also another problem if a '#' (perhaps also ',') is in the password.
A testuser reported it to me.
I found the following issue certbot plugin (certbot-dns-inwx)
INWX login failed: Parameter value syntax error (Error code 2005)
The solution was to add triple quotes to passwords field in credentials file.
dns_inwx_password = """your_password"""
The credential file /data/dnsapikey is generated from the file-structure.sh script.
Adding triple quotes to the add-on config was not succesfully.
Perhaps the triple quotes must be added in the add-on config and in the file-structure.sh script, if the triple quotes get removed between the config and the script.
I will test it.

@XantXant XantXant deleted the inwx branch January 25, 2023 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants