Skip to content

Letsencrypt traefik v2 acme json#1553

Merged
erik-wramner merged 6 commits intodocker-mailserver:masterfrom
MichaelSp:letsencrypt-traefik-acme-json
Jul 16, 2020
Merged

Letsencrypt traefik v2 acme json#1553
erik-wramner merged 6 commits intodocker-mailserver:masterfrom
MichaelSp:letsencrypt-traefik-acme-json

Conversation

@MichaelSp
Copy link
Copy Markdown
Contributor

@MichaelSp MichaelSp commented Jun 30, 2020

Will extract certificates from acme.json as written by traefik v2 for usage in dovecot and postfix.
Also watches acme.json for changes.

For this to work the file has to be mounted/present at /etc/letsencrypt/acme.json

This suppose to replace #976 and fix #1525

Will extract certificates from acme.json as written by traefik for usage in dovecot and postfix.
Also watches acme.json for changes. For this to work the file has to be mounted/present at `/etc/letsencrypt/acme.json`
@erik-wramner
Copy link
Copy Markdown
Contributor

Thanks for the PR. Many of the changes seem to be indentation fixes, which is not ideal, but never mind.

First question is if this is compatible with non-letsencrypt setups? I think it should be (the file is ignored if missing), am I right?

Second question is traefik. I'm not sure how common it is to use traefik for letsencrypt in this community? Most issues I've seen mention caddy. It would have been nice to have a solution not tied to traefik (though of course supporting one product is better than supporting none...). Is the traefik format supported by other tools, for example caddy?

@MichaelSp
Copy link
Copy Markdown
Contributor Author

indentation fixes: yes, you are right. Not ideal, but also not easy to keep the files clean. I had to laugh when reading this comment from almost 1 year ago: # Not fixing indentation yet to reduce diff (fix later in separate commit) :) I tried to find a sweet-spot.

  1. If there is no acme.json nothing happens.
  2. I cant say much about the rest of the community. It certainly scratches my itch. I was also not able to find any documentation on the caddy format. The traefik acme.json is also poorly documented. Here is an example: https://github.com/tomav/docker-mailserver/blob/32c732e276b2637cdda1fd1cc8b5ba318a765687/test/config/letsencrypt/acme-changed.json The le is something like "Encryption Provider Identifier" and can be chosen freely.

@youtous
Copy link
Copy Markdown
Contributor

youtous commented Jul 7, 2020

It would be nice to use a different variable than $DOMAINNAME / $HOSTNAME to allow certificates from wildcard name.

The traefik acme.json is also poorly documented.

And is not compatible with traefik v1.
Concerning this point, how do you handle errors when loading an acme.json from traefik v1?

@MichaelSp MichaelSp changed the title Letsencrypt traefik acme json Letsencrypt traefik v2 acme json Jul 7, 2020
@MichaelSp
Copy link
Copy Markdown
Contributor Author

MichaelSp commented Jul 7, 2020

Yes, this is traefik v2 (updated the PR-name).

I'll update the PR to support something like CERTIFICATE_DOMAIN with fall-back to DOMAINNAME / HOSTNAME

If the acme.json is not in v2-format, it will not find any matching domain and will not extract any certificate

@MichaelSp
Copy link
Copy Markdown
Contributor Author

Since I don't use DNS-01 (which is required to generated wildcard certs), I couldn't find out how a acme.json would look like in this case.
Can you give me an example? Is it this?

 {
  "le": {
    "Account": {},
    "Certificates": [
      {
        "domain": {
          "main": "*.example.com",
          "sans": []
        },
        "certificate": "X",
        "key": "Y",
        "Store": "default"
      }
    ]
  }
}

/etc/letsencrypt/live/$HOSTNAME/key.pem  and /etc/letsencrypt/live/$HOSTNAME/fullchain.pem are watched and will trigger a reload if changed
@MichaelSp MichaelSp marked this pull request as draft July 8, 2020 06:46
@youtous
Copy link
Copy Markdown
Contributor

youtous commented Jul 9, 2020

I'm not using DNS-01 but I already faced an user asking for it.
According to https://stackoverflow.com/a/58171898, it should be something like (not tested):

 {
  "le": {
    "Account": {},
    "Certificates": [
      {
        "domain": {
          "main": "example.com",
          "sans": ["example.com", "*.example.com"]
        },
        "certificate": "X",
        "key": "Y",
        "Store": "default"
      }
    ]
  }
}

set SSL_DOMAIN=*.example.com to extract a wildcard certificate from traefiks acme.json store
@MichaelSp MichaelSp marked this pull request as ready for review July 13, 2020 21:27
@MichaelSp
Copy link
Copy Markdown
Contributor Author

again an unrelated test failure as in #1560

@erik-wramner
Copy link
Copy Markdown
Contributor

Indeed, it is irritating. Just do a nonsense change (or force push with no change?) to trigger a rebuild. I used to be able to trigger that manually, but I can't.

However, there are several changes in this PR that I don't understand:

  • disable_plaintext_auth=no (used to be yes in comments)
  • ssl=yes (used to be required)
  • hostname and postmaster address (used to be undefined)
  • quotas
  • recipient_delimiter

Why have you changed that?

@MichaelSp
Copy link
Copy Markdown
Contributor Author

Oh I'm sorry thanks for your careful observation.
I guess these files are changed by the tests. I shouldn't have committed the changes...

@erik-wramner
Copy link
Copy Markdown
Contributor

That's better. Unfortunately that stupid test that keeps failing is at it again. You have unusually bad luck, it normally happens less than one build in ten. Can you push a gain to trigger a rebuild, please? We really have to fix that, no time though. The problem with working for free :-).

@MichaelSp
Copy link
Copy Markdown
Contributor Author

now it's green @erik-wramner

@erik-wramner erik-wramner merged commit f206ad7 into docker-mailserver:master Jul 16, 2020
@wernerfred
Copy link
Copy Markdown
Member

Nice work. @MichaelSp will you update the Wiki with instructions on how to use traefik with docker-mailserver? Or do i misinterprete the current information in the wiki about traefik?

@MichaelSp
Copy link
Copy Markdown
Contributor Author

@wernerfred thanks for the heads up. I've just updated the wiki: https://github.com/tomav/docker-mailserver/wiki/Configure-SSL#traefik

@MohammedNoureldin
Copy link
Copy Markdown
Contributor

MohammedNoureldin commented Oct 14, 2021

I have a suggestion regarding this, isn't it better to make the extracted cert and key files part of the container itself, and not in the mounted volume?

  • One reason is that the extracted files are always reproduceable, and there is not need probably to persist them.
  • The second reason is, in case we want to mount a named volume as read only - certs:/etc/letsencrypt/:ro, this will not work, because we have to mount the whole directory, and thus the container will not be able to extract the files inside this read only named volume, which leaves no other solution but to mount the named volume in rw mode, which is a security issue.

Or maybe this whole logic should be provided as a separate container, as these extracted files are relevant not only to the mailserver, but potentially to other containers.

What do you think?

@wernerfred
Copy link
Copy Markdown
Member

Functionality somewhat broken currently anyways: See #2239
Maybe someone from this thread has an idea 💡

@polarathene
Copy link
Copy Markdown
Member

What do you think?

It could probably be extracted to the same location as SSL_TYPE=manual uses internally.

I'd like to refactor the related code, just pressed for time to spare and prioritize.

@MohammedNoureldin
Copy link
Copy Markdown
Contributor

MohammedNoureldin commented Oct 15, 2021

@polarathene, So the workflow should look like this:

  1. The user wants to use letsencrypt with Nginx and acma-companion, then only set SSL_TYPE=letsencrypt.
  2. The user wants to use letsencrypt with Traefik (with custom certificate path), then he needs firstly to set SSL_TYPE=manual, CERT_PATH=SOME_PATH and KEY_PATH=SOME_PATH, then he should somehow be able to tell the internal "certificate auto extractor" script (I don't know how it is called in the mailserver container) to extract the certs to the same place as defined in the env vars. And it should know that there are some certificates to be extracted because the script will check the existence of acme.json? And of course the script will still take care about restarting the required services to apply any certificate renewal.

Is that correct? Could your refacoting consider that please when you have time?

@polarathene
Copy link
Copy Markdown
Member

2. The user wants to use letsencrypt with Traefik (with custom certificate path), then he needs firstly to set SSL_TYPE=manual, CERT_PATH=SOME_PATH and KEY_PATH=SOME_PATH, then he should somehow be able to tell the internal "certificate auto extractor" script

Please explain this scenario.

SSL_TYPE=letsencrypt works for certbot, nginx-proxy+acme-companion, traefik. In the case of traefik it does not provide .pem files like the other two which can be directly mounted, instead acme.json is watched for changes to extract the certs to files.

I'm not entirely familiar with the other two, but I think check-for-changes.sh runs periodically to check the /etc/letsencrypt/live directory for changes too such as renewed certs. That is what should toggle Postfix and Dovecot reloading within the container.

SSL_TYPE=manual with it's associated ENV for providing a cert and key file is a different SSL_TYPE mode. You provide the files with the path to access them in the container, and it will copy those certs to an internal location /etc/dms/tls IIRC during setup. AFAIK, the source locations are not watched for changes like SSL_TYPE=letsencrypt.


Are you asking to use Traefik with acme.json and extract the cert files to a location you provide? What is the requirement for that? You can find third-party options unrelated to docker-mailserver that can perform that functionality if you need it.

@MohammedNoureldin
Copy link
Copy Markdown
Contributor

MohammedNoureldin commented Oct 15, 2021

@polarathene if I understand correctly, you think that Docker mailserver uses the acme.json directly?

No, it doesn't, Docker mailserver extracts the .crt and .key files to be used by Dovecot and Postfix.

And extracting the certificates directly inside the same folder where the acme.json lives is not good, this is exactly my point, because you may have the acme.json stored in a Docker named volume, and named volumes can be mounted only as whole (namely as folder), not using single files. And in case of acme.json, it is highly recommended to mount its named volume in read only mode, but this read-only mode will prohibit the .crt and .key files from being generated (or extracted) by the script, because the extraction will fail bacause of lack of permissions in that folder.

Hopefully I could explain it. Please let me know if I misunderstood or missed anything.

@NorseGaud
Copy link
Copy Markdown
Member

And extracting the certificates directly inside the same folder where the acme.json lives is not good, this is exactly my point, because you may have the acme.json stored in a Docker named volume, and named volumes can be mounted only as whole (namely as folder), not using single files.

We can allow a variable to be passed in to override the location of the acme.json. Would that work for you? I have a PR open for some changes of this code and could include it.

@MohammedNoureldin
Copy link
Copy Markdown
Contributor

MohammedNoureldin commented Oct 15, 2021

Hi @NorseGaud,

it is actually about overriding the location of the extracted (generated) .crt and .key. Would that be possible?

To best honest, I am still thinking about it, and I am not sure if this extraction should happen inside Docker mailserver at all. Actually IMO the correct workflow should be that Docker mailserver watches not acme.json, but .key and .crt, because these are the files that are interesting for Docker mailserver.

So in simple words, the ideal solution should be actually to be able to monitor the .key and .crt files, without caring where they come from. The container should not care about acme.json. Even if it knows and includes some extraction logic, this should be optional in case someone wants to manage the extraction externally, and also I restarting the services should be related to .crt and .key files, not to acme.json.

Does it make sense to you?

@polarathene
Copy link
Copy Markdown
Member

if I understand correctly, you think that Docker mailserver uses the acme.json directly?

@MohammedNoureldin

My understanding is the certs content is available in the acme.json and we extract that out into .pem files. Those files get placed in the container /etc/letsencrypt/live AFAIK, as part of the SSL_TYPE=letsencrypt support.

Hopefully I could explain it. Please let me know if I misunderstood or missed anything.

You can use a bind volume mount instead of a data volume. Or mount to a different location in the container and add a symlink to the acme.json pointing to that location from /etc/letsencrypt should also work.

Likewise you can mount a data volume in the reverse direction, although it's a hassle and I don't recommend it, it would allow you to bindmount read-only the acme.json to the docker-mailserver container.

I'd personally advise the symlink approach if you insist on using a data volume instead of a bind mount.


I have a PR open for some changes of this code and could include

@NorseGaud

Delay that to a follow up please.

@MohammedNoureldin

This comment has been minimized.

@polarathene
Copy link
Copy Markdown
Member

it is actually about overriding the location of the extracted (generated) .crt and .key. Would that be possible?

No, you just need to read the acme.json from a different location (that you mount to).

To best honest I am still thinking about it, and I am not sure if this extraction should happen inside Docker mailserver at all. Actually IMO the correct workflow should be that Docker mailserver watches not acme.json, but .key and .crt, because these are the files that are interesting for Docker mailserver.

This is a feature that other users (including a maintainer) wants, I would remove it otherwise. You are welcome to handle acme.json yourself and extract them to provide to `docker-mailserver.

docker-mailserver has SSL_TYPE=manual where you can provide the .pem encode format files to use directly instead. Adopting service restart support when those files are updated needs to be added when someone has time for it, although you could possibly try mimicking the /etc/letsencrypt/live structure and use SSL_TYPE=letsencrypt instead which would presumably work already this way.

So in simple words, the ideal solution should be actually to be able to monitor the .key and .crt files, without caring where they come from

That is something I want myself, but I have quite a bit to do, and only have so much time for docker-mailserver. Presently other tasks for the project take priority, but I will get to implementing that in time.

We can probably leverage @NorseGaud work on check-for-changes.sh which makes that easier, but would need some refactoring.

Even if it knows and includes some extraction logic, this should be optional in case someone wants to manage the extraction externally,

It is optional.... don't provide an acme.json file.

restarting the services should be related to .crt and .key files, not to acme.json.

AFAIK, that is supported. acme.json is Traefik specific. Other Let's Encrypt provisioners should trigger the restart feature as their certs are detected as changed in /etc/letsencrypt/live. Yes, it'd be nice if we can adopt the behaviour for other SSL_TYPE modes, as mentioned above, that'll happen with time.


Does it make sense to you?

Please open a feature request and ping me.

This is an old PR you've bumped and it notifies all participants previously involved. I don't think it's appropriate to continue this discussion here. Thank you.

@polarathene polarathene mentioned this pull request Nov 1, 2021
7 tasks
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.

Update LetsEncrypt certificates

7 participants