scripts: merge new setup.sh version for 10.2.0 again

[georglauterbach]

Description

This PR marks the feature freeze phase for v10.2.0 release. The current :edge image will be tested for roughly one week and if no bugs get reported v10.2.0 will be released. During this testing period, only PRs that do not change functionaility will be merged.

I have prepared a draft for the release which I will release after this PR will have been merged on September 20th 2021. I have also moved all other open PRs into the new v10.3.0 milestone. If you think your PR should be released in v10.2.0 ,move the PR back and add the notice in the release notes.

If you want to support us give the latest :edge image a try and report any issues you might (hopefully not) encounter back to this PR.

This PR shall be merged to get the new setup.sh functionality again. Merging this PR is convenient and does not introduce needless rebasing and merge conflicts.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (non-breaking change that does improve existing functionality)

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (README.md or the documentation under docs/)
• If necessary I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[wernerfred]

I added the release checklist to your description. Now as setup.sh is in the container we do not need to update the version in README anymore.
CHANGELOG currently missing, can be copied from drafted release

[casperklein]

mv setup.sh.10.2.0 setup.sh added to checklist.

[casperklein]

Within the last two weeks, there was a regression (at least in my setup).

With a current master build, I see this on container startup

[ TASKLOG ]  Configuring mail server
cp: cannot stat '/etc/dovecot/ssl/dovecot.key': No such file or directory
cp: cannot stat '/etc/dovecot/ssl/dovecot.pem': No such file or directory

If I switch back to my master build from 2021-08-29, the error is gone. I noticed this yesterday, but had not time so far to dig deeper.

[NorseGaud]

Within the last two weeks, there was a regression (at least in my setup).

With a current master build, I see this on container startup

[ TASKLOG ]  Configuring mail server
cp: cannot stat '/etc/dovecot/ssl/dovecot.key': No such file or directory
cp: cannot stat '/etc/dovecot/ssl/dovecot.pem': No such file or directory

If I switch back to my master build from 2021-08-29, the error is gone. I noticed this yesterday, but had not time so far to dig deeper.

@casperklein I see that when I use the latest dovecot. I had to completely switch back to the dovecot we install by default. The mkcert.sh has started to fail:

root@xkr:/# OPENSSLCONFIG="/usr/share/dovecot/dovecot-openssl.cnf" /usr/share/dovecot/mkcert.sh
/etc/dovecot/dovecot.pem already exists, won't overwrite

[NorseGaud]

Deployed all of this into prod and it looks to be working

[polarathene]

EDIT: Oh nvm. It's unrelated 😅

The mkcert.sh has started to fail

Is it this file that apparently hasn't been touched since 2016? Can we swap that out for smallstep binary?

We already advise this in the generate self-signed cert docs, and have test certs generated in the same way instead of via openssl. Just need to add the binary into the Dockerfile.

Looks like mkcert.sh is called only in the Dockerfile and setup-stack.sh:

docker-mailserver/Dockerfile

Lines 128 to 130 in 2bc3e82

	mkdir /etc/dovecot/ssl && \
	chmod 755 /etc/dovecot/ssl && \
	./mkcert.sh 2>&1 && \

docker-mailserver/target/scripts/startup/setup-stack.sh

Lines 179 to 190 in 2bc3e82

	if [[ ! -f /etc/dovecot/ssl/dovecot.pem ]]
	then
	_notify 'inf' 'Generating default Dovecot cert'
	/usr/share/dovecot/mkcert.sh
	if [[ ${ONE_DIR} -eq 1 ]]
	then
	mkdir -p /var/mail-state/lib-dovecot
	cp /etc/dovecot/ssl/dovecot.key /var/mail-state/lib-dovecot/
	cp /etc/dovecot/ssl/dovecot.pem /var/mail-state/lib-dovecot/
	fi
	fi

Related Dovecot docs. This isn't something that would be used in production (self-signed certs), and looks fine to use smallstep instead of openssl AFAIK.

[georglauterbach]

@casperklein @NorseGaud is this Dovecot thing solved?:)

---

@polarathene I have no objections with swapping mkcert.sh for the smallstep binary, but you'd need to implement this change. Maybe you can do so in another PR and link the milestone v10.2.0 and we add this to the release notes?

[casperklein]

@casperklein I see that when I use the latest dovecot.

Yes, I forgot to mention, that I also use the community dovecot version.

However, I did a quick version check yesterday, both of my images got dovecot-core: 2:2.3.16-2+debian10.
So I think, there must be an other root cause.

[casperklein]

Ok, I got it. The "good" news is, the problem only occurs, when using the Dovecot community repo (what DMS did in the past, but not nowadays). So our default builds are not affected - sorry for the noise.

In #2158, three "obsolete" lines where removed from Dockerfile:

docker-mailserver/Dockerfile

Lines 117 to 120 in ed6421c

	# adapt mkcert for Dovecot community repo
	sed -i 's/CERTDIR=.*/CERTDIR=\/etc\/dovecot\/ssl/g' /usr/share/dovecot/mkcert.sh && \
	sed -i 's/KEYDIR=.*/KEYDIR=\/etc\/dovecot\/ssl/g' /usr/share/dovecot/mkcert.sh && \
	sed -i 's/KEYFILE=.*/KEYFILE=\$KEYDIR\/dovecot.key/g' /usr/share/dovecot/mkcert.sh && \

It's true, using the default DMS Dockerfile, these lines are obsolete now.
If you however use the dovecot community repo, to get the most recent version, these lines are necessary.

Makes it sense to re-add these lines?

Contra: These lines are not needed for our default build.

Pro: It's pretty easy for everyone, to add the dovecot community repo to the Dockerfile in order to use it (just two lines of copy/paste from https://repo.dovecot.org/#debian). With the removal of the "obsolete" lines, which fix mkcert.sh, it is not that easy any more. Beside the copy/paste, you'll also have to fix mkcert.sh on your own, to be compatible with DMS.

IMO: Re-adding these three lines won't hurt and makes it easy to use the dovecot repo.

[polarathene]

IMO: Re-adding these three lines won't hurt and makes it easy to use the dovecot repo.

What is the steps to replace Dovecot with the community version? If it modifies the Dockerfile or extends it, would it not make sense to just have this covered in the docs so that it's copy/paste and a common resource?

I don't know what the benefits are beyond perhaps using a newer release, but we could make that easier to inform users what's required vs them each figuring it out and maintaining a small modification individually.

---

That said, all this compatibility concern seems to be is the /etc/ssl/certs / /etc/ssl/private default paths (community edition) are changed to unified /etc/dovecot/ssl instead (what DMS uses by default with modified mkcert.sh), and the private key from dovecot.pem to dovecot.key to avoid conflict with the cert filename (also dovecot.pem).

mkcert.sh is only useful for self-signed cert generation as well, we don't really need to run it? In the Dockerfile, we call mkcert.sh to create and supply the cert files at /etc/dovecot/ssl, and setup-stack.sh only runs mkcert.sh on startup with ONE_DIR=1 (new default since end of August), but only if the cert is missing from the mail-state volume.

This seems... redundant as the Docker image should already supply it, and after 1 year this file will become an expired cert (mkcert.sh doesn't care as it's only creating a cert if one doesn't exist yet, it will not replace with a renewed cert).

Additionally, I would say this whole thing is rather pointless. I would have to verify that nothing is trying to use Dovecot earlier in the flow and failing on the lack of that ssl_key value in 10-ssl.conf not having a file/cert between this part of setup-stack.sh and when _set_certificate method is called. Our SSL configuration step based on ENV that I worked on should replace this mkcert.sh generated cert, unless SSL_TYPE was not set by the user, the default presumably uses one of these cases:

docker-mailserver/target/scripts/startup/setup-stack.sh

Lines 1059 to 1074 in 2bc3e82

	'' )
	# no SSL certificate, plain text access
	# TODO: Postfix configuration still responds to TLS negotiations using snakeoil cert from default config
	# TODO: Dovecot `ssl = yes` also allows TLS, both cases this is insecure and should probably instead enforce no TLS?
	# Dovecot configuration
	# WARNING: This may not be corrected(reset?) if `SSL_TYPE` is changed and internal config state persisted
	sed -i -e 's|^#disable_plaintext_auth = yes|disable_plaintext_auth = no|g' /etc/dovecot/conf.d/10-auth.conf
	sed -i -e 's|^ssl = required|ssl = yes|g' "${DOVECOT_CONFIG_SSL}"
	_notify 'warn' "(INSECURE!) SSL configured with plain text access. DO NOT USE FOR PRODUCTION DEPLOYMENT."
	;;
	* )
	# Unknown option, default behavior, no action is required
	_notify 'warn' "(INSECURE!) ENV var 'SSL_TYPE' is invalid. DO NOT USE FOR PRODUCTION DEPLOYMENT."
	;;

We could have self-signed cert generation there or just disable SSL in Dovecot as the cases seem to imply is meant to happen.. Seeing as we're meant to be priding ourselves in secure and easy configurations by default, failing early on SSL_TYPE not being set explicitly is probably better behaviour.

I think we can have the SSL_TYPE (env docs) change the value empty to communicate it is an invalid value instead, triggering failure to startup? An additional disabled value can explicitly provide the current no/disabled SSL cases above?

I think the other cases have conditional checks for being valid to continue, but none output an error message if those conditions aren't met? Do I add error handling with the _notify 'err' "Error message" and return 1 lines?

---

I don't think we should do any mkcert.sh compatibility support. Given the above that seems to be the wrong approach. I think we can drop mkcert.sh entirely if nothing else calls it beyond the two times I identified above. dovecot.key is only referenced by two Dovecot 10-ssl.conf files ssl_key option and the two cp commands from setup-stack.sh shown above.

[georglauterbach]

IMO: Re-adding these three lines won't hurt and makes it easy to use the dovecot repo.

What is the steps to replace Dovecot with the community version? If it modifies the Dockerfile or extends it, would it not make sense to just have this covered in the docs so that it's copy/paste and a common resource?

I don't know what the benefits are beyond perhaps using a newer release, but we could make that easier to inform users what's required vs them each figuring it out and maintaining a small modification individually.

I agree and would like to see this in the docs too :)

---

I think we can have the SSL_TYPE (env docs) change the value empty to communicate it is an invalid value instead, triggering failure to startup? An additional disabled value can explicitly provide the current no/disabled SSL cases above?

SGTM

I think the other cases have conditional checks for being valid to continue, but none output an error message if those conditions aren't met? Do I add error handling with the _notify 'err' "Error message" and return 1 lines?

SGTM too

---

I don't think we should do any mkcert.sh compatibility support. Given the above that seems to be the wrong approach. I think we can drop mkcert.sh entirely if nothing else calls it beyond the two times I identified above. dovecot.key is only referenced by two Dovecot 10-ssl.conf files ssl_key option and the two cp commands from setup-stack.sh shown above.

I agree again, we should remove mkcert.sh entirely if nothing breaks.

[polarathene]

SGTM

I would need advice on how to invoke init failure to exit (assuming we have a way to do that).

SGTM too

👍

I agree again, we should remove mkcert.sh entirely if nothing breaks.

I don't think it'll cause any breakage. I'll take care of it after resolving the bullseye TLS test failure. smallstep is much easier to deal with than openssl IMO. Better compatibility with community Dovecot and easier to maintain is a win :)

[georglauterbach]

SGTM

I would need advice on how to invoke init failure to exit (assuming we have a way to do that).

I think you can use this:

docker-mailserver/target/scripts/start-mailserver.sh

Lines 220 to 224 in 2bc3e82

	function _defunc
	{
	_notify 'fatal' 'Please fix your configuration. Exiting...'
	exit 1
	}

[casperklein]

@polarathene Good points. I fully agree.

I don't know what the benefits are beyond perhaps using a newer release

Nothing more. Per design you get faster security updates + new/updated features(?) (still same major version, so no breaking changes)

[casperklein]

I agree again, we should remove mkcert.sh entirely if nothing breaks.

I think we can drop mkcert.sh entirely if nothing else calls it beyond the two times I identified above.

Is this aimed for the 10.2 or a later release?

[polarathene]

Is this aimed for the 10.2 or a later release?

I'm presently waiting on a test run for the bullseye TLS test fix to pass, then open a PR and call it a night :)

I'll put together the mkcert.sh replacement PR tomorrow for review. Up to other maintainers if it should go into 10.2 or future release beyond that.

[georglauterbach]

Is this aimed for the 10.2 or a later release?

I'm presently waiting on a test run for the bullseye TLS test fix to pass, then open a PR and call it a night :)

I'll put together the mkcert.sh replacement PR tomorrow for review. Up to other maintainers if it should go into 10.2 or future release beyond that.

I'm fine with this going into 10.2.0.

[casperklein]

Marked as draft, until all remaining PRs are merged. After that, we start the "feature freeze" again.

[georglauterbach]

This PR shall be merged to get the new setup.sh functionality again. Merging this PR is convenient and does not introduce needless rebasing and merge conflicts. @wernerfred @NorseGaud if you approve, you can directly merge this PR to get the new setup.sh version (again) as stated in #2197 (comment).

[github-actions]

Documentation preview for this PR is ready! 🎉

Built with commit: 5cd55dd

[casperklein]

Was that by accident (putting the URL in a separate line prefixed by two spaces)

[georglauterbach]

Yes:)

[NorseGaud]

Sorry I'm late, I've been traveling and haven't had any time to review things.