Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wipe the ipa-ca DNS record when updating system records #6358

Closed
wants to merge 1 commit into from

Conversation

rcritten
Copy link
Contributor

Wipe the ipa-ca DNS record when updating system records

If a server with a CA has been marked as hidden and
contains the last A or AAAA address then that address
would remain in the ipa-ca entry.

This is because update-dns-system-records did not delete
values, it just re-computed them. So if no A or AAAA
records were found then the existing value was left.

Fixes: https://pagure.io/freeipa/issue/9195

Signed-off-by: Rob Crittenden rcritten@redhat.com

@rcritten rcritten added ipa-4-9 Mark for backport to ipa 4.9 ipa-4-10 Mark for backport to ipa 4.10 labels Jul 12, 2022
@f-trivino f-trivino self-requested a review July 12, 2022 10:59
@rcritten rcritten added the WIP Work in progress - not ready yet for review label Jul 12, 2022
@rcritten rcritten force-pushed the issue_9195 branch 5 times, most recently from 385c28b to 37ee85b Compare July 12, 2022 21:59
@rcritten rcritten force-pushed the issue_9195 branch 2 times, most recently from 461cb38 to 6a1e69b Compare July 18, 2022 15:43
@rcritten
Copy link
Contributor Author

I have lint fixes in my local tree. Waiting on CI to finish before pushing again.

@rcritten rcritten force-pushed the issue_9195 branch 3 times, most recently from 53b7de2 to 23c3d7c Compare July 19, 2022 21:09
@stale
Copy link

stale bot commented Sep 20, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Stale PR [Bot] label Sep 20, 2022
@rcritten
Copy link
Contributor Author

rebase

@stale
Copy link

stale bot commented Oct 14, 2022

This issue has been automatically closed as stale it has not had recent activity.

@stale stale bot closed this Oct 14, 2022
@stanislavlevin stanislavlevin removed the stale Stale PR [Bot] label Oct 14, 2022
@stanislavlevin
Copy link
Contributor

@rcritten, I'm really sorry, I will try to look at this PR next week unless somebody else would.

@stanislavlevin
Copy link
Contributor

I've checked locally the case described in the ticket. Proposed patch fixed the issue.

@stanislavlevin
Copy link
Contributor

The reproducer (test case) can be very tricky. __add_ca_records_from_hostname relies on getaddrinfo and its underlying nsswitch.conf while dig reads resolv.conf directly.

@rcritten rcritten added the re-run Trigger a new run of PR-CI label Oct 20, 2022
@rcritten rcritten added the needs review Pull Request is waiting for a review label Jan 11, 2023
@stanislavlevin
Copy link
Contributor

@rcritten,

[root@master1 /]# python3 -c 'from ipaserver.install.installutils import resolve_rrsets_nss; print(resolve_rrsets_nss("google.com"))'
[<DNS google.com IN A RRset: [<142.251.1.101>, <142.251.1.139>, <142.251.1.100>, <142.251.1.113>, <142.251.1.138>, <142.251.1.102>]>, <DNS google.com IN AAAA RRset: [<2a00:1450:4010:c1e::8a>, <2a00:1450:4010:c1e::66>, <2a00:1450:4010:c1e::65>, <2a00:1450:4010:c1e::8b>]>]

@rcritten
Copy link
Contributor Author

Yeah, the DNS thing is mind boggling to me but then again I'm all thumbs when it comes to DNS. I can un-revert the patch but the last time I did that CI failed. When I reproduced it locally I didn't test an external site but it definitely wouldn't resolve ones added to IPA.

@rcritten
Copy link
Contributor Author

Dropping the revert nss patch, let's see what happens.

@rcritten
Copy link
Contributor Author

Right. So the nss lookup fails to find the newly added IPv6 address. Bug in bind? Some cache somewhere?

@rcritten rcritten force-pushed the issue_9195 branch 2 times, most recently from e902f52 to f96ca9e Compare January 18, 2023 22:20
@rcritten
Copy link
Contributor Author

So IPv6 does work against google.com but not against the local DNS server when using nss. dig works fine.

At this point in the log the temporary IPv6 address has been added and it's resolvable using dig:

RUN ['dig', '+short', '-t', 'A', 'replica0.ipa.test.']
192.168.122.64

RUN ['dig', '+short', '-t', 'AAAA', 'replica0.ipa.test.']
2620:ffff:ffff:ffff:ffff:ffff:ffff:ffff

Google lookup works:

RUN ['python3', '-c', 'from ipaserver.install.installutils import resolve_rrsets_nss; print(resolve_rrsets_nss("google.com"))']
[<DNS google.com IN A RRset: [<172.253.62.138>, <172.253.62.102>, <172.253.62.101>, <172.253.62.139>, <172.253.62.113>, <172.253.62.100>]>, <DNS google.com IN AAAA RRset: [<2607:f8b0:4004:c07::64>, <2607:f8b0:4004:c07::66>, <2607:f8b0:4004:c07::65>, <2607:f8b0:4004:c07::8b>]>]

But nss lookup only returns A records:

RUN ['python3', '-c', 'from ipaserver.install.installutils import resolve_rrsets_nss; print(resolve_rrsets_nss("replica0.ipa.test"))']
[<DNS replica0.ipa.test IN A RRset: [<192.168.122.64>]>]

@stanislavlevin
Copy link
Contributor

When dig is invoked without @SERVER it reads resolv.conf.

https://linux.die.net/man/1/dig:

Unless it is told to query a specific name server, dig will try each of the servers listed in /etc/resolv.conf.

In this test run this config points nameserver 127.0.0.1.

dig queries dns server on 127.0.0.1:53.

2023-01-18 23:10:52,565    DEBUG  [ipatests.pytest_ipa.integration.host.Host.master.IPAOpenSSHTransport] RUN ['dig', '+short', '-t', 'A', 'replica0.ipa.test.']
...
2023-01-18 23:10:52,665    DEBUG  [ipatests.pytest_ipa.integration.host.Host.master.IPAOpenSSHTransport] RUN ['dig', '+short', '-t', 'AAAA', 'replica0.ipa.test.']
...

That dns server is bind (note that logs are buffered and they can be found in later tests' logs):

18-Jan-2023 23:10:52.101 info: client @0x7f9a185f2f68 127.0.0.1#60216 (replica0.ipa.test): query: replica0.ipa.test IN A +E(0)K (127.0.0.1)
18-Jan-2023 23:10:52.196 info: client @0x7f9a185f2f68 127.0.0.1#48749 (replica0.ipa.test): query: replica0.ipa.test IN AAAA +E(0)K (127.0.0.1)

That's expected result.

When resolve_ip_addresses_nss is invoked then its underlying getaddrinfo (AF_UNSPEC for ipv4 and ipv6) reads /etc/nsswitch.conf to get the data sources for hosts database.
On Fedora NSS configuration is managed with authselect (IPA set it to sssd profile).
Today's sssd profile:
https://github.com/authselect/authselect/blob/master/profiles/sssd/nsswitch.conf

hosts: files myhostname {if "with-libvirt":libvirt libvirt_guest }{if "with-mdns4" and "with-mdns6":mdns_minimal [NOTFOUND=return] }{if "with-mdns4" and not "with-mdns6":mdns4_minimal [NOTFOUND=return] }{if not "with-mdns4" and "with-mdns6":mdns6_minimal [NOTFOUND=return] }resolve [!UNAVAIL=return] dns

Note: maybe it's good idea to collect that resolved config file in tests.

When getaddrinfo is called then all those services are queried step by step according to NSS rules.
For example, in this IPA environment google.com can be resolved by either resolve(systemd-resolved based) or dns (libresolv based) NSS service.

[user@localhost ~]$ getent hosts -s dns google.com
...
[user@localhost ~]$ getent hosts -s resolve google.com
...

systemd-resolved queries 127.0.0.1:53 via its configuration provided by IPA, libresolv via /etc/resolv.conf.

So, in any case the query :

2023-01-18 23:10:52,759    DEBUG  [ipatests.pytest_ipa.integration.host.Host.master.IPAOpenSSHTransport] RUN ['python3', '-c', 'from ipaserver.install.installutils import resolve_rrsets_nss; print(resolve_rrsets_nss("google.com"))']

goes to local bind

18-Jan-2023 23:10:52.866 info: client @0x7f9a185f2f68 127.0.0.1#41711 (google.com): query: google.com IN AAAA +E(0) (127.0.0.1)
18-Jan-2023 23:10:52.866 info: client @0x7f9a184ee568 127.0.0.1#57723 (google.com): query: google.com IN A +E(0) (127.0.0.1)

Note: actually only resolve NSS service is used in this case, dns was provided for illustration.

The source of answer for querying "replica0.ipa.test" is files NSS service.
master's /etc/hosts

127.0.0.1  localhost
::1  localhost
         192.168.122.66 replica1.ipa.test
            192.168.122.202 controller.ipa.test
            192.168.122.137 master.ipa.test
            192.168.122.64 replica0.ipa.test
            192.168.122.20 client0.ipa.test
2023-01-18 23:10:53,571    DEBUG  [ipatests.pytest_ipa.integration.host.Host.master.cmd184] RUN ['python3', '-c', 'from ipaserver.install.installutils import resolve_rrsets_nss; print(resolve_rrsets_nss("replica0.ipa.test"))']
2023-01-18 23:10:54,163    DEBUG  [ipatests.pytest_ipa.integration.host.Host.master.cmd184] [<DNS replica0.ipa.test IN A RRset: [<192.168.122.64>]>]

That's expected result.

@rcritten
Copy link
Contributor Author

I dropped systemd-resolved for this test because it too was doing odd things, probably also related to /etc/hosts.

I don't believe we've ever had the requirement that all possible hostname/IPs be in /etc/hosts.

@rcritten
Copy link
Contributor Author

Another point. The new IP is added for the replica and the lookups done on the master.

The whole point is to test that dns-update-system-records reflects the correct set of DNS records so relying on nss in any way seems wrong. The purpose of the patch switching to nss was to include hostnames in the ipa-ca DNS name that are not resolvable in DNS. I fail to see the use-case for adding DNS records for non-resolvable hosts except for small, isolated installations.

@rcritten
Copy link
Contributor Author

rcritten commented Feb 2, 2023

I dropped the test altogether. It won't work in a real IPv6 environment. The "host" ip contains only the IPv4 address so we have nothing to compare it against.

I think this is doable eventually but I manual testing is sufficient for now.

@rcritten rcritten force-pushed the issue_9195 branch 2 times, most recently from 2e7167a to 337e96c Compare February 2, 2023 14:37
Copy link
Contributor

@f-trivino f-trivino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @rcritten could you remove the temp_commit?

@f-trivino f-trivino removed the needs review Pull Request is waiting for a review label Feb 8, 2023
If a server with a CA has been marked as hidden and
contains the last A or AAAA address then that address
would remain in the ipa-ca entry.

This is because update-dns-system-records did not delete
values, it just re-computed them. So if no A or AAAA
records were found then the existing value was left.

Fixes: https://pagure.io/freeipa/issue/9195

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
@f-trivino f-trivino added the re-run Trigger a new run of PR-CI label Feb 9, 2023
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Feb 9, 2023
@f-trivino
Copy link
Contributor

tests are green, adding ack

@f-trivino f-trivino added the ack Pull Request approved, can be merged label Feb 9, 2023
@rcritten rcritten added the pushed Pull Request has already been pushed label Feb 9, 2023
@rcritten
Copy link
Contributor Author

rcritten commented Feb 9, 2023

master:

  • c38546d Wipe the ipa-ca DNS record when updating system records

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-9 Mark for backport to ipa 4.9 ipa-4-10 Mark for backport to ipa 4.10 pushed Pull Request has already been pushed
Projects
None yet
4 participants