Add DoT support for DNS updates #7678

thalman · 2024-11-01T14:27:50Z

DNS-over-TLS is a new standard for encrypting DNS traffic.

SSSD does not implement the DoT itself but relies on other components of the system. This modification allows as to set a DoT for dynamic DNS updates

:config: New set of options dyndns_dot, dyndns_dot_cacert,
dyndns_dot_cert and dyndns_dot_key allows to enable
DNS-over-TLS for DNS updates.

:relnote: The DoT for dynamic DNS updates is supported now.
It requires new version of nsupdate from BIND 9.20.3+

pbrezina · 2024-11-04T13:12:26Z

Hi, this looks pretty straight-forwards. Code looks good, I will test it.

I will also try to see if we can get dyndns tests into PR CI, but this one might be tricky.

alexey-tikhonov · 2024-11-04T13:20:53Z

It requires new version of nsupdate from BIND 9.20.3+

Is this tested in runtime?

abbra · 2024-11-04T13:39:15Z

src/man/sssd-ad.5.xml

+                </varlistentry>
+
+                <varlistentry>
+                    <term>dyndns_dot_cacert (string)</term>


Will it be possible to specify a directory? OpenSSL in CentOS Stream 10 and Fedora 41+ is switching to default not using a single file CA bundle due to performance issues, so we should expect people wanting to use a directory of certs as well for custom bundles.

No, I don't see any option in nsupdate. https://bind9.readthedocs.io/en/v9.20.1/manpages.html#id126 It all expects a file.

May be we should ask for that?

pbrezina · 2024-11-07T14:43:19Z

@thalman Could you add a debug message that would show the nsupdate command line that is executed? Currently, it only shows the data we send.

pbrezina · 2024-11-07T15:26:13Z

This PR does not work as is at the moment, it is necessary to specify -H, server and port, i.e.:

nsupdate -g -S -H master.ipa.test

realm IPA.TEST
server master.ipa.test 853
...

Otherwise it tries to lookup the server using SOA record over port 53, which works. But then it continues and try to establish TLS on port 53 which timeouts... So right now, we need to explicitly set the hostname, server and port. However, documentation says that this should not be needed, so there's probably a bug in nsupdate. Opened: https://gitlab.isc.org/isc-projects/bind9/-/issues/5028

There is dyndns_server that we can use, but we should be able to parse it correctly:

master.ipa.test -> -H master.ipa.test, server master.ipa.test 853
master.ipa.test:555 -> -H master.ipa.test, server master.ipa.test 555
172.16.100.10:555#master.ipa.test -> -H master.ipa.test, server 172.16.100.10 555

pbrezina · 2024-11-07T15:33:22Z

To test this, you can install https://copr.fedorainfracloud.org/coprs/antorres/freeipa-unbound-dns/ on ipa container:

dnf copr enable antorres/freeipa-unbound-dns -y
dnf upgrade freeipa\* -y
ipa-dns-install --dns-over-tls --dot-forwared 1.1.1.1#one.one.one.one

# allow outside requests to 53 non encrypted
sed -i "s/listen-on { 127.0.0.1; };/listen-on { any; };/" /etc/named.conf
systemctl restart named

on client container, renamed it so we have a dns name managed by ipa server:

kinit [email protected]
ipa-join --hostname client.ipa.test

cat /etc/sssd.conf
[sssd]
config_file_version = 2
services = nss, pam
domains = ipa.test

[domain/ipa.test]
id_provider = ipa
access_provider = ipa
ipa_server = _srv_
ipa_domain = ipa.test
ipa_hostname = client.test
use_fully_qualified_names = true
debug_level = 0xfff0
dyndns_update = true
#dyndns_dot = true
ipa_hostname = client.ipa.test

thalman · 2024-11-18T13:06:50Z

The code has been changed according to our discussion. dyndns_dot has been removed and I check dyndns_server instead. The dyndns_server option can be an URI. Scheme dns+tls activates DoT support.

thalman · 2024-11-22T12:31:09Z

We should include #7712 first and rebase to fit the changes.

alexey-tikhonov · 2024-11-22T13:19:42Z

We should include #7712 first and rebase to fit the changes.

@thalman, a mistype? FWIW, #7712 was merged.

thalman · 2024-11-22T18:15:08Z

We should include #7712 first and rebase to fit the changes.

@thalman, a mistype? FWIW, #7712 was merged.

Sorry, my mistake - #7713 is the right one

pbrezina

Haven't tried it yet, but I did my first read through. See comments inside.

src/man/sssd-ad.5.xml

src/providers/be_dyndns.c

pbrezina · 2024-11-27T13:41:23Z

src/providers/be_dyndns.c

    if (argv == NULL) {
        return NULL;
    }

+    if (! sss_is_valid_dns_scheme(server_uri->scheme)) {
+        DEBUG(SSSDBG_IMPORTANT_INFO, "Invalid DNS scheme: %s, using dns://\n", server_uri->scheme);


Maybe add syslog message if you choose to continue?

Moved to sss_log.

Can you also keep DEBUG? We usually log to both.

pbrezina · 2024-11-27T13:47:53Z

src/util/util.c

@@ -1091,3 +1091,81 @@ errno_t sss_getenv(TALLOC_CTX *mem_ctx,

    return value != NULL ? EOK : ENOENT;
 }
+
+errno_t sss_parse_uri(TALLOC_CTX *mem_ctx, const char *uri, struct sss_parsed_uri **_parsed_uri)


Maybe rename this to sss_parse_dns_uri? This is not usable by common uris.

Also, you can use libcurl to parse it for you https://curl.se/libcurl/c/libcurl-url.html CURLUPART_FRAGMENT will be the DoT server hostname.

Renamed. It can actually parse ordinary URL but the DNS URI is special.

Unfortunately standard libraries like CURL or libldap (this also has URI parsing function) fail on DNS URIs, so we have to parse it on our own.

Ok, that's unfortunate.

It can "parse" them, but content after # is treated as hostname instead of uri fragment. That's why I think it is only for this kind of DNS URI.

pbrezina

The patch kind of works, but it still has issues.

First, I had to apply #7727 otherwise the arguments to nsupdate where added in reverse order nsupdate master.ipa.test -H ... which made nsupdate fail on the hostname.

Additionally, dns+tls should imply port 853 if other port is not set. Otherwise nsupdate will attempt to talk on port 53 which will fail. Until https://gitlab.isc.org/isc-projects/bind9/-/issues/5028 is fixed and backported, we need to workaround it.

And also see my previous comments.

I have prepared containers where you can test it:

services:
  client:
    image: ${REGISTRY}/dyndns-dot-client:f41
  ipa:
    image: quay.io/sssd/dyndns-dot-ipa:f41

You can add fake IP to the client:

sudo ip addr add 192.168.100.41 dev eth0

pbrezina · 2024-11-28T12:01:33Z

src/providers/be_dyndns.c

@@ -1078,7 +1091,7 @@ struct tevent_req *be_nsupdate_send(TALLOC_CTX *mem_ctx,
        goto done;
    }

-    args = be_nsupdate_args(state, auth_type, force_tcp);
+    args = be_nsupdate_args(state, auth_type, force_tcp, server_uri, dot_cacert, dot_cert, dot_key);


Keep 80 chars limit.

pbrezina · 2024-11-28T12:01:44Z

src/providers/be_dyndns.c

    if (argv == NULL) {
        return NULL;
    }

+    if (!sss_is_valid_dns_scheme(server_uri)) {
+        sss_log(SSS_LOG_WARNING, "Invalid DNS scheme in SSSD config file: %s, using dns://\n", server_uri->scheme);


Keep 80 chars limit.

pbrezina · 2024-11-28T12:01:56Z

src/providers/be_dyndns.c

+        have_dot_cert = (dot_cert != NULL && dot_cert[0] != 0);
+        have_dot_key = (dot_key != NULL && dot_key[0] != 0);
+        if (have_dot_key != have_dot_cert) {
+            DEBUG(SSSDBG_OP_FAILURE, "The dyndns_dot_cert and dyndns_dot_key must be set both (or none of them)\n");


Keep 80 chars limit.

pbrezina · 2024-11-28T12:02:18Z

src/util/util.c

@@ -1091,3 +1091,81 @@ errno_t sss_getenv(TALLOC_CTX *mem_ctx,

    return value != NULL ? EOK : ENOENT;
 }
+
+errno_t sss_parse_dns_uri(TALLOC_CTX *mem_ctx, const char *uri, struct sss_parsed_dns_uri **_parsed_uri)


Keep 80 chars limit.

pbrezina · 2024-11-28T12:02:28Z

src/util/util.h

+    char *data;
+};
+
+errno_t sss_parse_dns_uri(TALLOC_CTX *ctx, const char *uri, struct sss_parsed_dns_uri **_parsed_uri);


Keep 80 chars limit.

DNS-over-TLS is a new standard for encrypting DNS traffic. SSSD does not implement the DoT itself but relies on other components of the system. This modification allows as to set a DoT for dynamic DNS updates :config: the `dyndns_server` option is extended so it can be in form of URI (dns+tls://1.2.3.4:853#servername). New set of options `dyndns_dot_cacert`, `dyndns_dot_cert` and `dyndns_dot_key` allows to configure DNS-over-TLS communication. :relnote: The DoT for dynamic DNS updates is supported now. It requires new version of `nsupdate` from BIND 9.20.1+

thalman requested a review from pbrezina November 1, 2024 14:28

pbrezina self-assigned this Nov 1, 2024

abbra reviewed Nov 4, 2024

View reviewed changes

thalman force-pushed the dot_nsupdate branch from 2010bd9 to c019ad0 Compare November 15, 2024 16:30

alexey-tikhonov added branch: sssd-2-9 branch: sssd-2-10 labels Nov 18, 2024

thalman force-pushed the dot_nsupdate branch from c019ad0 to 40c22e4 Compare November 22, 2024 11:10

thalman added the Blocked label Nov 22, 2024

thalman force-pushed the dot_nsupdate branch 2 times, most recently from 5090c4e to f292ec5 Compare November 27, 2024 13:00

pbrezina requested changes Nov 27, 2024

View reviewed changes

thalman force-pushed the dot_nsupdate branch 2 times, most recently from 8487350 to d9b40dc Compare November 28, 2024 10:24

pbrezina mentioned this pull request Nov 28, 2024

child: add extra args in correct order #7727

Draft

pbrezina requested changes Nov 28, 2024

View reviewed changes

pbrezina reviewed Nov 28, 2024

View reviewed changes

thalman force-pushed the dot_nsupdate branch from d9b40dc to 98581d8 Compare November 28, 2024 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DoT support for DNS updates #7678

Add DoT support for DNS updates #7678

thalman commented Nov 1, 2024

pbrezina commented Nov 4, 2024

alexey-tikhonov commented Nov 4, 2024

abbra Nov 4, 2024

pbrezina Nov 6, 2024 •

edited

Loading

abbra Nov 6, 2024

pbrezina commented Nov 7, 2024

pbrezina commented Nov 7, 2024 •

edited

Loading

pbrezina commented Nov 7, 2024

thalman commented Nov 18, 2024 •

edited

Loading

thalman commented Nov 22, 2024

alexey-tikhonov commented Nov 22, 2024

thalman commented Nov 22, 2024

pbrezina left a comment

pbrezina Nov 27, 2024

thalman Nov 28, 2024

pbrezina Nov 28, 2024

pbrezina Nov 27, 2024

thalman Nov 28, 2024

pbrezina Nov 28, 2024

pbrezina left a comment

pbrezina Nov 28, 2024

pbrezina Nov 28, 2024

pbrezina Nov 28, 2024

pbrezina Nov 28, 2024

pbrezina Nov 28, 2024

Add DoT support for DNS updates #7678

Are you sure you want to change the base?

Add DoT support for DNS updates #7678

Conversation

thalman commented Nov 1, 2024

pbrezina commented Nov 4, 2024

alexey-tikhonov commented Nov 4, 2024

Choose a reason for hiding this comment

pbrezina Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbrezina commented Nov 7, 2024

pbrezina commented Nov 7, 2024 • edited Loading

pbrezina commented Nov 7, 2024

thalman commented Nov 18, 2024 • edited Loading

thalman commented Nov 22, 2024

alexey-tikhonov commented Nov 22, 2024

thalman commented Nov 22, 2024

pbrezina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbrezina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbrezina Nov 6, 2024 •

edited

Loading

pbrezina commented Nov 7, 2024 •

edited

Loading

thalman commented Nov 18, 2024 •

edited

Loading