Disable rdns for Kerberos tests
Greetings,
The name canonicalization support for Kerberos is doing us more harm
than good in the regression tests, so I propose we disable it. Patch
attached.
Thoughts?
Thanks,
Stephen
Attachments:
krbrdns_disable_v1.patchtext/x-diff; charset=us-asciiDownload
From 992d946d17c79d240ac6587998e2f94b12a726de Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 20 Feb 2023 17:53:48 -0500
Subject: [PATCH] For Kerberos testing, disable reverse DNS lookup
In our Kerberos test suite, there isn't much need to worry about the
normal canonicalization that Kerberos provides by looking up the reverse
DNS for the IP address connected to, and in some cases it can actively
cause problems (eg: capture portal wifi where the normally not
resolvable localhost address used ends up being resolved anyway, and
never to the domain we are using for testing, causing the entire
regression test to fail with errors about not being able to get a TGT
for the remote realm for cross-realm trust).
Therefore, disable it by adding rdns = false into the krb5.conf that's
generated for the test.
---
src/test/kerberos/t/001_auth.pl | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index d610ce63ab..b04c9dff56 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -108,6 +108,7 @@ kdc = FILE:$kdc_log
[libdefaults]
default_realm = $realm
+rdns = false
[realms]
$realm = {
--
2.34.1
On 21/02/2023 01:35, Stephen Frost wrote:
Greetings,
The name canonicalization support for Kerberos is doing us more harm
than good in the regression tests, so I propose we disable it. Patch
attached.Thoughts?
Makes sense. A brief comment in 001_auth.pl itself to mention why we
disable rdns would be nice.
- Heikki
Greetings,
* Heikki Linnakangas (hlinnaka@iki.fi) wrote:
On 21/02/2023 01:35, Stephen Frost wrote:
The name canonicalization support for Kerberos is doing us more harm
than good in the regression tests, so I propose we disable it. Patch
attached.Thoughts?
Makes sense. A brief comment in 001_auth.pl itself to mention why we disable
rdns would be nice.
Thanks for reviewing! Comments added and updated the commit message.
Unless there's anything else, I'll push this early next week.
Thanks again!
Stephen
Attachments:
krbrdns_disable_v2.patchtext/x-diff; charset=us-asciiDownload
From e0df250f30cdde1ee6d57ff24efbca3ecd923801 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 20 Feb 2023 17:53:48 -0500
Subject: [PATCH] For Kerberos testing, disable reverse DNS lookup
In our Kerberos test suite, there isn't much need to worry about the
normal canonicalization that Kerberos provides by looking up the reverse
DNS for the IP address connected to, and in some cases it can actively
cause problems (eg: capture portal wifi where the normally not
resolvable localhost address used ends up being resolved anyway, and
never to the domain we are using for testing, causing the entire
regression test to fail with errors about not being able to get a TGT
for the remote realm for cross-realm trust).
Therefore, disable it by adding rdns = false into the krb5.conf that's
generated for the test.
Reviewed-By: Heikki Linnakangas
Discussion: https://postgr.es/m/Y/QD2zDkDYQA1GQt@tamriel.snowman.net
---
src/test/kerberos/t/001_auth.pl | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index d610ce63ab..16b3e8ed23 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -100,6 +100,17 @@ $stdout =~ m/Kerberos 5 release ([0-9]+\.[0-9]+)/
or BAIL_OUT("could not get Kerberos version");
$krb5_version = $1;
+# Build the krb5.conf to use.
+#
+# Explicitly specify the default (test) realm and the KDC for
+# that realm to avoid the Kerberos library trying to look up
+# that information in DNS, and also because we're using a
+# non-standard KDC port.
+#
+# Reverse DNS is explicitly disabled to avoid any issue with
+# capture portals or other cases where the reverse DNS succeeds
+# and the Kerberos library uses that as the canonical name of
+# the host and tries to do a cross-realm lookup.
append_to_file(
$krb5_conf,
qq![logging]
@@ -108,6 +119,7 @@ kdc = FILE:$kdc_log
[libdefaults]
default_realm = $realm
+rdns = false
[realms]
$realm = {
--
2.34.1
On 25 February 2023 00:50:30 EET, Stephen Frost <sfrost@snowman.net> wrote:
Thanks for reviewing! Comments added and updated the commit message.
Unless there's anything else, I'll push this early next week.
s/capture portal/captive portal/. Other than that, looks good to me.
- Heikki
Greetings,
* Heikki Linnakangas (hlinnaka@iki.fi) wrote:
On 25 February 2023 00:50:30 EET, Stephen Frost <sfrost@snowman.net> wrote:
Thanks for reviewing! Comments added and updated the commit message.
Unless there's anything else, I'll push this early next week.
s/capture portal/captive portal/. Other than that, looks good to me.
Push, thanks again!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
Push, thanks again!
Why'd you only change HEAD? Isn't the test equally fragile in the
back branches?
regards, tom lane
Greetings,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Push, thanks again!
Why'd you only change HEAD? Isn't the test equally fragile in the
back branches?
We hadn't had any complaints about it and so I wasn't sure if it was
useful to back-patch it. I'm happy to do so though.
Thanks,
Stephen
Greetings,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Push, thanks again!
Why'd you only change HEAD? Isn't the test equally fragile in the
back branches?
Following on from this after some additional cross-platform testing,
turns out there's other options we should be disabling in these tests to
avoid depending on DNS for the test.
Attached is another patch which, for me at least, seems to prevent the
tests from causing any DNS requests to happen. This also means that the
tests run in a reasonable time even in cases where DNS is entirely
broken (the resolver set in /etc/resolv.conf doesn't respond).
Barring objections, my plan is to commit this change soon and to
back-patch both patches to supported branches.
Thanks!
Stephen
Attachments:
krbdns_disable_v1.patchtext/x-diff; charset=us-asciiDownload
From 604097f8acb6c53b5dd7dd710486282cf731ab08 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Mon, 3 Apr 2023 15:15:29 -0400
Subject: [PATCH] For Kerberos testing, disable DNS lookups
Similar to 8dff2f224, this disables DNS lookups by the Kerberos library
to look up the KDC and the realm while the Kerberos tests are running.
In some environments, these lookups can take a long time and end up
timing out and causing tests to fail. Further, since this isn't really
our domain, we shouldn't be sending out these DNS requests during our
tests.
---
src/test/kerberos/t/001_auth.pl | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index a0ed3a0a0b..458246b4d7 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -107,6 +107,11 @@ $krb5_version = $1;
# that information in DNS, and also because we're using a
# non-standard KDC port.
#
+# Also explicitly disable DNS lookups since this isn't really
+# our domain and we shouldn't be causing random DNS requests
+# to be sent out (not to mention that broken DNS environments
+# can cause the tests to take an extra long time and timeout).
+#
# Reverse DNS is explicitly disabled to avoid any issue with a
# captive portal or other cases where the reverse DNS succeeds
# and the Kerberos library uses that as the canonical name of
@@ -118,6 +123,8 @@ default = FILE:$krb5_log
kdc = FILE:$kdc_log
[libdefaults]
+dns_lookup_realm = false
+dns_lookup_kdc = false
default_realm = $realm
rdns = false
--
2.34.1
Greetings,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Push, thanks again!
Why'd you only change HEAD? Isn't the test equally fragile in the
back branches?
Back-patched.
Thanks!
Stephen
Greetings,
* Stephen Frost (sfrost@snowman.net) wrote:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
Push, thanks again!
Why'd you only change HEAD? Isn't the test equally fragile in the
back branches?Following on from this after some additional cross-platform testing,
turns out there's other options we should be disabling in these tests to
avoid depending on DNS for the test.Attached is another patch which, for me at least, seems to prevent the
tests from causing any DNS requests to happen. This also means that the
tests run in a reasonable time even in cases where DNS is entirely
broken (the resolver set in /etc/resolv.conf doesn't respond).Barring objections, my plan is to commit this change soon and to
back-patch both patches to supported branches.
Done.
Thanks!
Stephen