Allow disabling DH param generation _or_ Use RFC 7919 DH group instead of self-generation

[polarathene]

Bug description

DH params generation is not possible to disable?:

acme-companion/app/entrypoint.sh

Lines 47 to 57 in 7f1b754

	function check_dh_group {
	# Credits to Steve Kamerman for the background Diffie-Hellman creation logic.
	# https://github.com/nginx-proxy/nginx-proxy/pull/589
	local DHPARAM_BITS="${DHPARAM_BITS:-2048}"
	re='^[0-9]*$'
	if ! [[ "$DHPARAM_BITS" =~ $re ]] ; then
	echo "Error: invalid Diffie-Hellman size of $DHPARAM_BITS !" >&2
	exit 1
	fi
	# If a dhparam file is not available, use the pre-generated one and generate a new one in the background.

nginx-proxy container however provides an ENV to disable generation with DHPARAM_GENERATION=false.

In my case this is for docs on a project with several popular approaches via Docker to automate cert renewal, but the project itself has it's own internal DH params file where it handles TLS connections that are not using HTTPS. AFAIK acme-companion and nginx-proxy should be fine provisioning certs without requiring DH params?

---

If DH params are actually required, or you'd prefer to not generate them.. you can use standardized RFC 7919 DH groups. The project I'm a maintainer of uses ffdhe4096.pem, these DH groups are generally advised to use vs generating your own. `openssl wiki advises adopting RFC 7919 too and provides the pem content too.

NOTE: You are currently generating with -dsaparam option (not acme-companion, only nginx-proxy due to some divergence), this is much faster but does pose some risk (2016 CVE advisory), as the openssl docs mention:

Beware that with such DSA-style DH parameters, a fresh DH key should be created for each use to avoid small-subgroup attacks that may be possible otherwise.

Using DH groups from RFC 7919 is also advised by Mozilla among others, and part of TLS v1.3 (which only negotiates these DH groups IIRC, rather than custom provided ones that TLS 1.2 or earlier allow).

In the project I help maintain, we have just added a copy of ffdhe4096.pem to the repo and use COPY during Dockerfile build.

acme-companion image version

No deployment, this is a feature request + advice.

nginx-proxy's Docker configuration

N/A

rendered nginx configuration

N/A

Containers logs

N/A

Docker host

N/A

[polarathene]

I actually brought this up earlier in the year (linked comment is hidden as off-topic) 😅

I just noticed the following PR, this change doubled the DH params size without proper justification and enabled the usage of -dsaparam

While on topic, this is probably relevant to the project too:

That said, nginx-proxy-letsencrypt-companion, still uses 2048-bit DH params (generated as well instead of using RFC 7919 groups), and provisions RSA certs with key size of 4096-bit size by default. 2048-bit is plenty secure.

This default was inherited from simp_le before switching to acme.sh (which defaults to 2048-bit like certbot), but due to the config support to change default RSA key size from simp_le 4096-bit, they've carried over that 4096-bit RSA default :\

[polarathene]

I am contributing a PR for this to nginx-proxy. I may follow up with a related PR to acme-companion afterwards.

[buchdag]

@polarathene I agree with you that the use of -dsaparam should not have been merged to nginx-proxy/nginx-proxy and that nginx-proxy/nginx-proxy and nginx-proxy/acme-companion should have the same DH parameters related options and generation method.

I'm not familiar with RFC 7919 yet, if I understand correctly it recommends use of pre-generated standardized DH parameters rather than generating your own ?

[polarathene]

if I understand correctly it recommends use of pre-generated standardized DH parameters rather than generating your own ?

Yes. I have actually had a PR ready to submit to nginx-proxy for a while now. I can open the PR with that if you like, although I have two changes I'd like to add, I've just been a bit strapped for time recently.

[buchdag]

I've read about RFC 7919 and I think we should go this way for both nginx-proxy and acme-companion (replacing DH param generation), so PRs are welcome. Regarding nginx-proxy, keep in mind that a new feature or a feature modification will require the corresponding tests to be merged.

[buchdag]

Don't hesitate to submit your PR in its current state so we can work toward completing it, adding / modifying tests if needed and merging it.

[polarathene]

Submitted. The additional changes are fairly low effort to add, I'm just lacking resources to address them atm.

Plenty to review in the meantime :)

As for acme-companion I probably won't have time to spare for at least a month or so. Most of it should be transferable, and perhaps when using these group parameters you can avoid adding site specific copies which simplifies the maintenance a bit :)