docker-simple-mail-forwarder icon indicating copy to clipboard operation
docker-simple-mail-forwarder copied to clipboard

Fix for issue where a signed cert and self signed cert can both exist

Open nickwest opened this issue 3 years ago • 4 comments

init-openssl.sh creates both RSA and EC self-signed certificates. When using a properly signed certificate of only one type (RSA or EC) the init-openssl.sh script will still generate the self-signed version of the other certificate type. This results in some requests to Postfix returning the self-signed cert rather than the signed cert.

This pull requests modifies init-openssl.sh to prevent the generation of self-signed certificates if either type of certificate already exists. If a certificate does exist it also checks for missing certificates and then uses sed to comment out the appropriate smtpd_tls_cert_file/key lines in main.cf.

When both or neither certificates are present it functions as it currently does.

nickwest avatar Feb 27 '22 21:02 nickwest

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Feb 27 '22 21:02 CLAassistant

Hello @nickwest , thank you very much for your improvement of the better compatible with CA settings!

I'd like to merge this PR after it gets at least one approval from the community so that we can make sure it works as expected.

P.S. add an unit test will be highly appreciated.

Have a great day!

huan avatar Feb 28 '22 00:02 huan

Hi @huan, I added a couple unit tests as requested. They verify that when both EC and RSA certs are present they share the same CN, domains, and are either both self signed or both CA signed. This ensures that regardless of which certificate is used the result will be equal.

The already present TLS tests will continue to verify that connections will work regardless of certificate matching and validity. Those tests also verify that having a single cert (the case prior to PR #51) is working properly.

If @Czocher is available they'd probably be a good reviewer/approver since this touches their work in PR #51 where EC certs were introduced.

I also see that PR #36 could introduce custom paths for certs. The unit tests in my PR do not account for that currently and would need to be changed if that PR is merged.

nickwest avatar Feb 28 '22 19:02 nickwest

Hi @nickwest I'm available but I don't have access to any proper production environment at the moment to try to deploy and test it. I can have a look at the code though.

czocher avatar Mar 03 '22 08:03 czocher