Add ECDSA support

[OpenLarry]

I added the ECDSA certificate support like we have discussed in #2426 .

I tried to modify the bash script as less as possible to reduce the time for code review/testing and I tried to use the same coding style like before. However, I decided to move some code into separate functions to avoid code duplication.

A few notes:

• I did not add a separate option (at least not yet) to disable ECDSA certificates when using the integrated Letsencrypt client. I don't think that it makes sense to disable ECDSA, because there should be no drawback when it stays enabled for everyone?
• If SKIP_LETSENCRYPT=y (or the installed certificate isn't issued by Letsencrypt) and there is no custom ECDSA cert, the script will add a symlink from the ECDSA certificate to the RSA certificate.
• ECDSA and RSA certificates are checked and renewed independently from each other, so it's likely that they have different validity periods after migration.
• I've combined this if-statement with the statement above. I think it's a bug to continue the script if there isn't a certificate, but we have an old one that has non-matching hashes. (However it should happen rarely - or even never?)

I've tested migration (using custom and Letsencrypt certificates) and fresh installations. I've checked all services using testssl.sh and it reports two certificates or one certificate depending on whether an ECDSA certificate exists or not. Looks good to me, so I'm looking forward to feedback! :-)

Of course I can add a few lines to the documentation as well.

Closes #2426

[andryyy]

I have not looked into it at all. What about TLSA?
Any changes when updating?

I

[OpenLarry]

Damn... yes.

Didn't think about that yet because I don't use DNSSEC.
A second certificate also requires a second TLSA record for each offered service.
The automatic generation of DNS records in Mailcow's UI could probably be extended easily to show the required records for both certificates.

But that makes migration from an RSA-only setup indeed more complex for users that are already using DNSSEC.

Any ideas how to solve that easily?

• Check if TLSA records are present and disable the ECDSA certificate if it's not listed?
• Show a warning during the update process and let the user decide (and add an option to disable the ECDSA certificates in mailcow.conf)?
• Combination of both?

[andryyy]

And that’s where hacky starts and delivery may fail.

[mkuron]

That means we need a configuration option where users explicitly have to turn on ECDSA. I was hoping to avoid that (we have enough such options already), but there is no way around it. I guess we could turn it on by default on new installs though (from generate_config.sh).

[andryyy]

Can we get docs for this? :)

[OpenLarry]

Sure! :-) If the PR is ready to get merged and the functionality is more or less complete, then I'll add something about it to the documentation.

[andryyy]

Looks complete. :) Do you think this is ready to be merged?

A few words in the docs are enough.

[OpenLarry]

I've added a few words about it to documentation and I updated the chapter slightly. I hope it's all right. :-)

I tried to test my code with every scenario that came into my mind. So I hope it works. 😃

[OpenLarry]

Is anything still missing here?
Do you need help? :-)

[patschi]

@andryyy Anything new in terms of merging this?

[Knight1]

I wonder what happens with all the Software when they load the same certificate twice.

[OpenLarry]

I've already checked the services implementation and documentation here.

Since it's documented OpenSSL functionality that loading the same certificate twice actually does nothing and postfix/dovecot/nginx don't care about the certificate handling/parsing itself, it should (and does!) work flawlessly in my opinion. :-)

[patschi]

What's the exact state with this PR, @OpenLarry? Now as #2509 was merged in the past, I'd guess that most of the code is outdated and not compatible anymore. Any input from @mhofer117 for this?

[mhofer117]

I had this included in #2509 initially because I thought ECDSA would be merged first. Then removed it so #2509 could be merged. However my changes are still ready here: OpenLarry#1

Personally I have no need for ECDSA certs so i'm not maintaining it, but that PR should solve almost all conflicts

[OpenLarry]

I've already merged the mentioned PR by mhofer117 locally and it seems to work. But I wasn't able to test everything yet.

I will try to do that in the next two days and update this PR afterwards. :-)

[OpenLarry]

Sorry for the late reply, had a lot of stuff to do during the last days...

I've merged the PR by @mhofer117 containing the ECDSA adjustments for the SNI part and it looks good to me. But I haven't tested the SNI part yet, because I have no need for that. Maybe someone else wants to try that out and verify that everything works properly?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[MAGICCC]

Just saw your rebase on master, possible to change the base branch to staging and rerebase? :p

[OpenLarry]

Sure! I will do that in the coming days and also clean up this branch. :-)

[SecT0uch]

Using ECDSA with last acme.sh deploy hook. Really need that to be merged.

Any news ?

[christianbur]

@SecT0uch
Do you mean this?
I added the ECDSA support over a year ago.

[SecT0uch]

@christianbur
No, I mean mailcow recognizing ecdsa-cert.pem and ecdsa-key.pem intruduced with that commit

I had a previous version of the deploy script that was working well.

Edit: I have mailcow up-to-date and don't want to play with symlinks in docker

[OpenLarry]

I've squashed all messy commits together and rebased it on the current staging branch. Seems to work (and is already working for me for years).

[andryyy]

Yes, keep in mind there are like 47838374 different setups and thinks will get dirty. We will need to be prepared. Thank you, Aaron! :)

[SecT0uch]

What's the status on that PR ?

[christianbur]

@SecT0uch
Why did you break my acme.sh deploy plugin? -> acmesh-official/acme.sh#4170

If you want to store ECDSA certificates in the (RSA) cert files (cert.pem, key.pem), this would have been possible simply with symlinks. With your change in acme.sh, you destroyed my plugin and also my mail setup, because DANE/TLSA was broken.

@SecT0uch -> Please undo the change as soon as possible.

Create symlink in directory ./mailcow-dockerized/data/assets/ssl

ln -s ecdsa-cert.pem cert.pem
ln -s ecdsa-key.pem key.pem

$ ./mailcow-dockerized/data/assets/ssl# ls -al 
lrwxrwxrwx 1 root root 10 Aug 24 12:11 cert.pem -> ecdsa-cert.pem
lrwxrwx 1 root root 9 Aug 24 12:11 key.pem -> ecdsa-key.pem
-rw-r--r-- 1 root root 5934 Aug 24 11:54 ecdsa-cert.pem
-rw-r--r-- 1 root root 3247 Aug 24 11:54 ecdsa-key.pem

[SecT0uch]

Because you are including custom changes in a public repository that breaks a default mailcow installation.

You broke my installation in the first place with your changes on the acme plugin.
Since clearly this commit won't be merged, I don't see the point in breaking everyone else's installation.

If you want to keep using the custom changes you made on mailcow, you should fork the acme plugin and make your changes on your side.

[DerLinkman]

Hello everyone,

as i have taken the position to maintain the repository and i'm not 100% sure what your PR does i would kindly ask you to describe what the benefits of ECDSA are and if it will break existing installations.

I've saw, that you've changed also the dovecot.conf for alt_certs. How does dovecot/postfix behave if these certs are not generated?