Skip to content

security/stunnel: Make use of ACME certs#2854

Closed
johnnyslee wants to merge 2 commits intoopnsense:masterfrom
johnnyslee:stunnel-acme
Closed

security/stunnel: Make use of ACME certs#2854
johnnyslee wants to merge 2 commits intoopnsense:masterfrom
johnnyslee:stunnel-acme

Conversation

@johnnyslee
Copy link
Contributor

  • Add Chain intermediate CAs option (defaults disabled)
  • Expose restart action to GUI

@fichtner
Copy link
Member

Beyond the clear technical intent what is the benefit of the intermediate certificates here? The PR doesn’t explain and I’m sure @AdSchellevis won’t accept this without the use case that doesn’t work without it (it’s not about acme support per se).

@AdSchellevis
Copy link
Member

The use-case should be clear and preferably explained in a separate PR to the docs (https://docs.opnsense.org/manual/how-tos/stunnel.html) as well, as adding certs here will lead to unexpected results for people using it to pin trust here.

@johnnyslee
Copy link
Contributor Author

I'm using Stunnel with ACME cert to provide DNS-over-TLS (DoT) for my mobile devices.
Using public DoT won't resolve local names under home network. To avoid switching devices' DNS settings whenever network changes, I setup a private DoT that works for both mobile and local networks.
A valid server cert is required in this case. ACME cert needs its intermediates together to be valid.

Add an option, defaults disabled, to chain intermediate CAs which is
required when using ACME cert.
For example, we can now select "Restart Stunnel" from
`Service/ACME-Client/Automations>Run-Command>System-or-Plugin-Command`
in GUI.
@johnnyslee
Copy link
Contributor Author

There were now closed #2045 and #1905, meaning there were people tried to use Stunnel with ACME certs for their own purposes. So I thought this time adding the feature as a default-disabled option might put less pressure for this to be merged. Would adding <advanced>true</advanced> to the <field> be making any difference, maybe?
Currently it's difficult to explain my use-case in docs, because there are configs I injected in the generated named.conf that involves the usage of BIND's views feature which is not configurable by GUI in the current version of bind plugin.

@AdSchellevis
Copy link
Member

I'm not sure to be honest, if the use case is hard to explain, I'm not sure we should try to merge it in the product either. If someone can add a simple example in the documentation where this feature plays a role, I don't mind that much adding an advanced toggle to allow chaining certs, if no one can explain it, it might be better to just do this without the gui as it will only raise questions.

@johnnyslee
Copy link
Contributor Author

As simple as the following?

DNS-over-TLS server

(Not resolver, use Unbound, Stubby, etc. for DoT resolver instead)
Use Stunnel to add TLS layer to your domain name server to make it a DoT server:
Listen address external/internal address
Listen port 853
Target hostname name server (BIND, Dnsmasq, etc) address
Target port port of the above name server
Certificate a valid (not self-signed) one issued by a real CA
Chain intermediate CAs check the box if the selected certificate comes with them

Skipping the part "how to add views to BIND", then it's not difficult at all.

Also, openssl s_client -connect results,
without intermediates:

$ echo | openssl s_client -connect dns.domain.my:853 >/dev/null
depth=0 CN = dns.domain.my
verify error:num=20:unable to get local issuer certificate
verify return:1
depth=0 CN = dns.domain.my
verify error:num=21:unable to verify the first certificate
verify return:1
depth=0 CN = dns.domain.my
verify return:1
DONE

with intermediates:

$ echo | openssl s_client -connect dns.domain.my:853 >/dev/null
depth=2 C = US, ST = New Jersey, L = Jersey City, O = The USERTRUST Network, CN = USERTrust RSA Certification Authority
verify return:1
depth=1 C = AT, O = ZeroSSL, CN = ZeroSSL RSA Domain Secure Site CA
verify return:1
depth=0 CN = dns.domain.my
verify return:1
DONE

AdSchellevis added a commit that referenced this pull request Jul 4, 2022
commit e873aa41591442e16ec0581fa8b6e8696a1821ff
Author: Ad Schellevis <ad@opnsense.org>
Date:   Mon Jul 4 14:23:32 2022 +0200

    security/stunnel: Add option to chain intermediate CAs (#2854), better explain impact and add move to advanced

commit 1e86212
Author: Johnny S. Lee <6614805+johnnyslee@users.noreply.github.com>
Date:   Mon Feb 21 09:52:26 2022 +0800

    security/stunnel: Allow GUI usage of restart action

    For example, we can now select "Restart Stunnel" from
    `Service/ACME-Client/Automations>Run-Command>System-or-Plugin-Command`
    in GUI.

commit 005af92
Author: Johnny S. Lee <6614805+johnnyslee@users.noreply.github.com>
Date:   Mon Feb 21 09:45:28 2022 +0800

    security/stunnel: Add option to chain intermediate CAs

    Add an option, defaults disabled, to chain intermediate CAs which is
    required when using ACME cert.
@AdSchellevis
Copy link
Member

merged with small modifications in d162124 (move setting to advanced, explain impact)

@AdSchellevis AdSchellevis added the feature Adding new functionality label Jul 4, 2022
AdSchellevis added a commit that referenced this pull request Jul 4, 2022
…ip chain by default. ref #2854

While working on the documentation I noticed my previous comment was wrong, which also invalidates the need for an optional setting. When it comes to the "CAfile" setting, the chain shouldn't be provided, for the listener (the server cert) it shouldn't matter at all if you ship the chain since it's not part of the authentication.

This commits simplifies #2854 by removing the option. The current documentation online doesn't need any modifications for this.
@AdSchellevis
Copy link
Member

ok, my mistake, we can (and should) add the chain if available for the listener (6ee383d), for some reason I've totally overlooked how CAfile is populated which was my concern in the beginning. @johnnyslee can you try the current state in the master branch?

@johnnyslee
Copy link
Contributor Author

@AdSchellevis Tested. It works fine. Server cert is automatically bundled with its intermediate CAs. That's great.

@johnnyslee johnnyslee closed this Jul 4, 2022
@AdSchellevis
Copy link
Member

@johnnyslee sorry for the confusion and thanks for testing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Adding new functionality

Development

Successfully merging this pull request may close these issues.

3 participants