drop --user on pip install dns plugin #2971

wolviex · 2023-06-01T18:06:16Z

Do not install dns_plugin (via pip) into the user site because it will lack sys.path precedence to urllib3 in /opt/certbot/lib/python3.7/site-packages.
resolves #2921 and #2844
solution recommended by certbot engineer: https://community.letsencrypt.org/t/nginx-proxy-manager-and-cert/198147
Tested with docker image 'jc21/nginx-proxy-manager:latest', Godaddy DNS API credentials and wildcard certificate.

Do not install dns_plugin into the user site because it will lack sys.path precedence to urllib3 in /opt/certbot/lib/python3.7/site-packages

nginxproxymanagerci · 2023-06-01T18:24:07Z

Docker Image for build 1 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-2971

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

wolviex · 2023-06-06T16:17:10Z

Docker Image for build 1 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-2971

Note: ensure you backup your NPM instance before testing this PR image! Especially if this PR contains database changes.

PR image is functioning as expected and stable.

biggen1684 · 2023-06-08T02:39:10Z

Can confirm this patch fixes issues with DNS validation of SSLs through Godaddy. Should definitely be merged with the main branch. Good job @wolviex!

martindur · 2023-06-11T12:43:34Z

Worked smoothly with Route53 as well. Thanks for the effort @wolviex 🙏

jeffshead · 2023-06-12T12:37:51Z

Doesn't fix issue with ClouDNS: #2844

wolviex · 2023-06-12T23:52:55Z

Doesn't fix issue with ClouDNS: #2844

Sounds like you're facing a different issue. Note that the error changed when you tried this PR, it did indeed resolve the issue #2921. I'll make some comments on that issue.

justSem · 2023-06-14T15:15:13Z

Your proposed fix does seem to throw some issues with the TransIP API
This error also occurs on a clean install.

Error: Command failed: certbot certonly --config "/etc/letsencrypt.ini" --work-dir "/tmp/letsencrypt-lib" --logs-dir "/tmp/letsencrypt-log" --cert-name "npm-5" --agree-tos --email "redacted@domain.tls" --domains "*.domain.tls" --authenticator dns-transip --dns-transip-credentials "/etc/letsencrypt/credentials/credentials-5"
Traceback (most recent call last):
  File "/usr/bin/certbot", line 5, in 
    from certbot.main import main
  File "/opt/certbot/lib/python3.7/site-packages/certbot/main.py", line 6, in 
    from certbot._internal import main as internal_main
  File "/opt/certbot/lib/python3.7/site-packages/certbot/_internal/main.py", line 21, in 
    import josepy as jose
  File "/opt/certbot/lib/python3.7/site-packages/josepy/__init__.py", line 40, in 
    from josepy.json_util import (
  File "/opt/certbot/lib/python3.7/site-packages/josepy/json_util.py", line 14, in 
    from OpenSSL import crypto
  File "/opt/certbot/lib/python3.7/site-packages/OpenSSL/__init__.py", line 8, in 
    from OpenSSL import crypto, SSL
  File "/opt/certbot/lib/python3.7/site-packages/OpenSSL/crypto.py", line 1517, in 
    class X509StoreFlags(object):
  File "/opt/certbot/lib/python3.7/site-packages/OpenSSL/crypto.py", line 1537, in X509StoreFlags
    CB_ISSUER_CHECK = _lib.X509_V_FLAG_CB_ISSUER_CHECK
AttributeError: module 'lib' has no attribute 'X509_V_FLAG_CB_ISSUER_CHECK'

    at ChildProcess.exithandler (node:child_process:402:12)
    at ChildProcess.emit (node:events:513:28)
    at maybeClose (node:internal/child_process:1100:16)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:304:5)

This error can be fixed by exec-ing to the container and executing:
. /opt/certbot/bin/activate && pip install --upgrade pyopenssl

wolviex · 2023-06-14T15:22:19Z

Your proposed fix does seem to throw some issues with the TransIP API This error also occurs on a clean install.

Are you saying you get this same error on an fresh container pulled with the 'jc21/nginx-proxy-manager:latest' image tag?
OR are you saying this PR introduces the error not found on the latest?

justSem · 2023-06-14T15:26:38Z

Your proposed fix does seem to throw some issues with the TransIP API This error also occurs on a clean install.

Are you saying you get this same error on an fresh container pulled with the 'jc21/nginx-proxy-manager:latest' image tag? OR are you saying this PR introduces the error not found on the latest?

Good point. I may have spoken a bit to soon.
But no, this error is not introduced by your PR, it just came apparent when testing your proposed solution.

When manualy applying the PR to the :latest container this error is present as well.

unixorn · 2023-06-15T05:41:13Z

I was having the sys.path precedence to urllib3 problem with Route 53 and this fixes it.

wolviex · 2023-06-16T16:29:59Z

Appears to resolve #2844 as well

justSem

Since I can't push to your PR branch; I'd propose the line

const prepareCmd = '. /opt/certbot/bin/activate && pip install --no-cache-dir ' + dns_plugin.package_name + (dns_plugin.version_requirement || '') + ' ' + dns_plugin.dependencies + ' && deactivate';

to be changed to

const prepareCmd = '. /opt/certbot/bin/activate && pip install --upgrade pyopenssl  && pip install --no-cache-dir ' + dns_plugin.package_name + (dns_plugin.version_requirement || '') + ' ' + dns_plugin.dependencies + ' && deactivate';

wolviex

This PR directly addresses the error Will not install to user site and nothing else.
This is as recommended by a certbot engineer.

Please explain why the pyopenssl upgrade is to be considered a part of this PR.

justSem · 2023-06-23T10:45:59Z

This PR directly addresses the error Will not install to user site and nothing else. This is as recommended by a certbot engineer.

Please explain why the pyopenssl upgrade is to be considered a part of this PR.

I figured, since it fixes a subsequent issue, it might as well be integrated into the same PR. But if you wish, I'll create a separate PR for it.

wolviex · 2023-06-23T20:42:50Z

I figured, since it fixes a subsequent issue, it might as well be integrated into the same PR. But if you wish, I'll create a separate PR for it.

I thought you clarified that your issue is also present on the :latest ?
It seems like a dependancy issue, due to deprecated functionality in pyopenssl
pyca/pyopenssl#1142

It may be something that should be addressed in another module

biggen1684 · 2023-06-26T11:49:09Z

Crazy that this PR hasn't been merged yet with the official since it clearly fixes an issue. @jc21 still around?

wolviex · 2023-06-27T23:27:36Z

Crazy that this PR hasn't been merged yet with the official since it clearly fixes an issue. @jc21 still around?

My Guess is this is backburner since it looks like a v3 based in golang is in the works.
BTW, this is the same fix process as already merged in commit 05307aa

a1ad · 2023-07-11T15:39:04Z

@wolviex I still need your version, so I guess it is still not fixed in latest with 05307aa

wolviex · 2023-07-11T16:00:58Z

@wolviex I still need your version, so I guess it is still not fixed in latest with 05307aa

Sorry for the confusion - no, the issue is not fixed in 05307aa. I only meant it is the same fix method used in that commit, just for reference. 05307aa is also not latest (2a06384 is and my PR is on top of that).

a1ad · 2023-07-11T16:02:20Z

Ok, no problem. I hope the PR is merging soon!

jc21 · 2023-07-20T02:20:30Z

I'm almost happy to merge the PR, however I'd ask that someone (or more than one) test this image with PUID/PGID set and also test without it being set.

The certbot install stuff may behave differently when the process is run as a non-root user vs a root user.

Nightreaver · 2023-07-25T10:47:07Z

I can confirm its working on AWS R53.
Have it running with

version: '3.8'
services:
  app:
    image: 'jc21/nginx-proxy-manager:github-pr-2971'
    restart: unless-stopped
    environment:
      PUID: 1000
      PGID: 1000
    ports:
      - '80:80'
      - '81:81'
      - '443:443'

and got my cert without issues

mashb1t · 2023-07-25T15:23:46Z

tested and works for me too using Route53

jc21 · 2023-07-26T00:28:58Z

I also only have Route53 in use for my hosts, but yeah I'd like to see other certbot plugins tested too.

wolviex · 2023-07-26T03:13:46Z

I use GoDaddy, and have PUID/PGID set to 1000. This is what worked for me in the first place.
created new container without PUID/PGID and was still functional.
Note that I did have to workaround certbot-dns-godaddy issue with certbot/acme version mismatching by manually downgrading acme. I beleive this is unrelated.
miigotu/certbot-dns-godaddy#27

jc21 · 2023-07-30T21:21:14Z

Ok sounds good I'll merge it.

drop --user on pip install dns plugin godaddy

f39e527

Do not install dns_plugin into the user site because it will lack sys.path precedence to urllib3 in /opt/certbot/lib/python3.7/site-packages

wolviex changed the title ~~drop --user on pip install dns plugin godaddy~~ drop --user on pip install dns plugin Jun 1, 2023

jeffshead mentioned this pull request Jun 12, 2023

Add ClouDNS LetsEncrypt certificate #2844

Open

justSem suggested changes Jun 20, 2023

View reviewed changes

wolviex mentioned this pull request Jun 20, 2023

SSL DNS Challenge Issue #2921

Closed

wolviex commented Jun 23, 2023

View reviewed changes

jc21 added the requires-verification Waiting for one or more people to confirm the fix label Jul 20, 2023

jc21 merged commit 3333a32 into NginxProxyManager:develop Jul 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drop --user on pip install dns plugin #2971

drop --user on pip install dns plugin #2971

wolviex commented Jun 1, 2023 •

edited

Loading

nginxproxymanagerci bot commented Jun 1, 2023

wolviex commented Jun 6, 2023

biggen1684 commented Jun 8, 2023 •

edited

Loading

martindur commented Jun 11, 2023

jeffshead commented Jun 12, 2023 •

edited

Loading

wolviex commented Jun 12, 2023 •

edited

Loading

justSem commented Jun 14, 2023 •

edited

Loading

wolviex commented Jun 14, 2023

justSem commented Jun 14, 2023

unixorn commented Jun 15, 2023

wolviex commented Jun 16, 2023

justSem left a comment

wolviex left a comment •

edited

Loading

justSem commented Jun 23, 2023

wolviex commented Jun 23, 2023

biggen1684 commented Jun 26, 2023

wolviex commented Jun 27, 2023

a1ad commented Jul 11, 2023

wolviex commented Jul 11, 2023

a1ad commented Jul 11, 2023

jc21 commented Jul 20, 2023

Nightreaver commented Jul 25, 2023

mashb1t commented Jul 25, 2023

jc21 commented Jul 26, 2023

wolviex commented Jul 26, 2023

jc21 commented Jul 30, 2023

drop --user on pip install dns plugin #2971

drop --user on pip install dns plugin #2971

Conversation

wolviex commented Jun 1, 2023 • edited Loading

nginxproxymanagerci bot commented Jun 1, 2023

wolviex commented Jun 6, 2023

biggen1684 commented Jun 8, 2023 • edited Loading

martindur commented Jun 11, 2023

jeffshead commented Jun 12, 2023 • edited Loading

wolviex commented Jun 12, 2023 • edited Loading

justSem commented Jun 14, 2023 • edited Loading

wolviex commented Jun 14, 2023

justSem commented Jun 14, 2023

unixorn commented Jun 15, 2023

wolviex commented Jun 16, 2023

justSem left a comment

Choose a reason for hiding this comment

wolviex left a comment • edited Loading

Choose a reason for hiding this comment

justSem commented Jun 23, 2023

wolviex commented Jun 23, 2023

biggen1684 commented Jun 26, 2023

wolviex commented Jun 27, 2023

a1ad commented Jul 11, 2023

wolviex commented Jul 11, 2023

a1ad commented Jul 11, 2023

jc21 commented Jul 20, 2023

Nightreaver commented Jul 25, 2023

mashb1t commented Jul 25, 2023

jc21 commented Jul 26, 2023

wolviex commented Jul 26, 2023

jc21 commented Jul 30, 2023

wolviex commented Jun 1, 2023 •

edited

Loading

biggen1684 commented Jun 8, 2023 •

edited

Loading

jeffshead commented Jun 12, 2023 •

edited

Loading

wolviex commented Jun 12, 2023 •

edited

Loading

justSem commented Jun 14, 2023 •

edited

Loading

wolviex left a comment •

edited

Loading