Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nickwest
Copy link

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.

When any cert exists, do not generate self signed cert or postfix will server both/either
Comment out smtpd_tls_cecert or smtpd_tls_cert config if matching cert does not exist
@CLAassistant
Copy link

CLAassistant commented Feb 27, 2022

CLA assistant check
All committers have signed the CLA.

@huan
Copy link
Owner

huan commented Feb 28, 2022

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!

@nickwest
Copy link
Author

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.

if [[ -f /etc/postfix/cert/smtp.ec.cert && -f /etc/postfix/cert/smtp.cert ]]; then
ec_cert_subject=`openssl x509 -noout -subject -in /etc/postfix/cert/smtp.ec.cert`
rsa_cert_subject=`openssl x509 -noout -subject -in /etc/postfix/cert/smtp.cert`
ec_cert_cn=`perl -e 'print join "\n", @cn = $ARGV[0] =~ /(?<=CN\s=\s).*/g;' "$ec_cert_subject" | sort -`
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used perl here because busy-box grep doesn't include PCRE regex and I needed Lookaround constructors to extract the domains/CN

@czocher
Copy link
Contributor

czocher commented Mar 3, 2022

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.

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.

4 participants