kereis / traefik-certs-dumper

Dumps Let's Encrypt certificates of a specified domain which Traefik stores in acme.json.
Apache License 2.0
129 stars 24 forks source link

Added support for PKCS12 format #41

Closed momper14 closed 3 years ago

momper14 commented 3 years ago

This pullrequest adds the ability to combine the cert and key into a PKCS12 certificate. The reason I implemented it was my Plex setup. I added an section for this in the README how it can be configured.

PS: Sorry for the smal changes in formatting. I think I hit the autoformat key.

kereis commented 3 years ago

Hey,

thank you for your PR! I'll try to test it. Something that we need to check is that the openssl is available for other architectures (other Dockerfiles).

Cheers

kereis commented 3 years ago

Hey,

sorry for the delayed response. I wasn't available until now. I tested your changes a bit and for now, it looks good to me. However, maybe we should consider adding an environment variable which you can set as plain password instead of using Docker secrets?

momper14 commented 3 years ago

Hey,

Thanks for testing the feature. I described in the readme that there are two env variables for setting the password. "PKCS12_PASSWORD" for plain text and "PKCS12_PASSWORD_FILE" for docker secrets. But maybe I need to improve the description if this is not clear enoughenough.

PS. "PKCS12_PASSWORD" has the higher priority if both are set

deejayexe commented 3 years ago

Hi!

I want to test it with docker environment, but I don't see any update in docker hub humenius/traefik-certs-dumper, there is any dev image?

fabh2o commented 3 years ago

You can build the image from the fork:

git clone git@github.com:momper14/traefik-certs-dumper.git
cd traefik-cert-dumper
sudo docker build -t tmp/traefik-cert-dumper -f docker/Dockerfile .

and then use the image tmp/traefik-cert-dumper

deejayexe commented 3 years ago

Ok, thanks I have been able to create it and I have been able to dump PKCS12 certificate with traefik and acme ca zerossl.

kereis commented 3 years ago

Hey,

Thanks for testing the feature. I described in the readme that there are two env variables for setting the password. "PKCS12_PASSWORD" for plain text and "PKCS12_PASSWORD_FILE" for docker secrets. But maybe I need to improve the description if this is not clear enoughenough.

PS. "PKCS12_PASSWORD" has the higher priority if both are set

Hey, I changed COMBINE_PKCS12 to be explicitly set to yes and changed the doc a bit. Feel free to take a look and say if it's okay. :)

momper14 commented 3 years ago

Hey. For me, your changes looks good. But maybe we can change [[ ! "${COMBINE_PKCS12}" = yes ]] to [[ "${COMBINE_PKCS12}" != yes ]] in line 134? In my opinion, it is a bit more clear.

kereis commented 3 years ago

Sorry again for the delay. I moved exclamation like in your suggestion:

Hey. For me, your changes looks good. But maybe we can change [[ ! "${COMBINE_PKCS12}" = yes ]] to [[ "${COMBINE_PKCS12}" != yes ]] in line 134? In my opinion, it is a bit more clear.

If that looks good, I'll merge this one. :)