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

profiles: add support for resolved #222

Closed
wants to merge 1 commit into from

Conversation

pbrezina
Copy link
Member

Resolved is enabled by default since Fedora 33 so we need to reflect
this change in our profiles.

It should be OK to enabled it unconditionaly. The module is part of
systemd so it basically can not be uninstalled and it can be safely
disabled through systemctl disable --now systemd-resolved.service.

Resolves:
#221

@pbrezina
Copy link
Member Author

@mcatanzaro
Copy link

Yup that looks good, thanks.

CC @keszybz

@mcatanzaro
Copy link

I belatedly realize that it might not even be needed, since I see the systemd-libs package's scriptlet modifies /etc/authselect/user-nsswitch.conf. But probably still a good idea to do this anyway.

@pbrezina
Copy link
Member Author

That will work for sssd and winbind profiles since those two do not set hosts map. But it needs to be modified for nis and minimal.

@keszybz
Copy link

keszybz commented Jul 14, 2020

I belatedly realize that it might not even be needed, since I see the systemd-libs package's scriptlet modifies /etc/authselect/user-nsswitch.conf. But probably still a good idea to do this anyway.

I think it's much better to modify this here. The scriptlets are ugly and fragile and they were always meant to be temporary.

ethers: files {exclude if "with-custom-ethers"}
group: files {if "with-altfiles":altfiles }systemd {exclude if "with-custom-group"}
hosts: files resolve [!UNAVAIL=return] dns myhostname {exclude if "with-custom-hosts"}
Copy link

Choose a reason for hiding this comment

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

No, the order should be
resolve [!UNAVAIL=return] myhostname files dns

Justification: systemd-resolved caches files internally, so that the file doesn't need to be parsed by each client. Some people put long lists here, e.g. blacklists. So resolve should be before files. It should also be before dns, so that dns only works as fallback. And resolve implements the same synthetization that myhostname does, so myhostname should be after resolve. And it should be right after resolve to minimize the difference in behaviour for synthetized names in case systemd-resolved is active and in case it stopped working. Additionally myhostname is more trusted than dns, so it should be before it. It is also more up-to-date than files most of the time, and should be before it too.

Choose a reason for hiding this comment

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

OK... sorry @pbrezina for telling you to use the wrong order, that's my bad.

What about mdns4_minimal (which is not part of this authselect config, but which we also have enabled by default)?

Choose a reason for hiding this comment

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

According to nss-resolve(8), the recommended order is:

hosts: files mymachines resolve [!UNAVAIL=return] dns myhostname

But we don't use mymachines in Fedora due to some bug related to interaction with freeipa, so that should be ignored. The difference from your suggestion is the placement of files.

systemd-resolved caches files internally, so that the file doesn't need to be parsed by each client. Some people put long lists here, e.g. blacklists. So resolve should be before files.

Then I suggest updating the nss-resolve manpage.

Copy link

Choose a reason for hiding this comment

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

That's a good question. nss-mdns is enabled by %post scriptlet, and it inserts itself directly before dns. So I think we should keep this order, i.e. have something like

resolve [!UNAVAIL=return] myhostname files mdns4_minimal [NOTFOUND=return] dns

But maybe this does the wrong thing to do? This means that resolved will be used to resolve mdns names (if it is running).

Copy link

Choose a reason for hiding this comment

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

According to nss-resolve(8), the recommended order is:

hosts: files mymachines resolve [!UNAVAIL=return] dns myhostname

This is the old version. The package in rawhide and https://www.freedesktop.org/software/systemd/man/nss-resolve.html#Example have the updated one.

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIK resolved also has some support for mdns. Is nss_mdns even needed when resolved is used?

Choose a reason for hiding this comment

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

In theory, I would think it would not be needed. I think Zbigniew wants to keep it around anyway though, "just in case"...? After all, resolved can be disabled without editing nsswitch.conf.

Copy link

Choose a reason for hiding this comment

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

Yeah, the assumption is that people might want to disable systemd-resolved.service and things should continue working as much as possible in that case, falling back to existing mechanisms for name resolution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, so I will keep it without nss_mdns for now as there was no request to add it to these profiles so far.

@pbrezina
Copy link
Member Author

See new patch.

@keszybz
Copy link

keszybz commented Jul 20, 2020

LGTM.

Copy link
Member

@thalman thalman left a comment

Choose a reason for hiding this comment

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

ACK, thanks

@pbrezina
Copy link
Member Author

  • master
    • eb4ef2c111b3b439bda66cc0ac8764343e9d6d6f - profiles: add support for resolved

@pbrezina pbrezina closed this Jul 22, 2020
@mcatanzaro
Copy link

Hi @pbrezina, somehow this never got committed to master! We should reopen and land it. But there are a couple issues. First, this PR changes the hosts line to:

hosts:      resolve [!UNAVAIL=return] myhostname files dns {exclude if "with-custom-hosts"}

But we had some unexpected difficulty with this in https://bugzilla.redhat.com/show_bug.cgi?id=1867830#c34. We wound up switching to:

hosts:      files resolve [!UNAVAIL=return] myhostname dns {exclude if "with-custom-hosts"}

I also wonder about the nis version of this line in /profiles/nis/nsswitch.conf. Notice that it no longer includes nis! Is that a mistake? It should probably have nis after files, like before, right?

TODO: update our Fedora package. We have a beta freeze exception in https://bugzilla.redhat.com/show_bug.cgi?id=1867830 that should cover this update.

@pbrezina
Copy link
Member Author

pbrezina commented Sep 3, 2020

Ah, I did not push the changes only made a build in Fedora. Well, at least it didn't go upstream :-)

See new patch. Can you summarize here what is the problem and why files must go before resolved? Doesn't resolved cache it already?

@keszybz
Copy link

keszybz commented Sep 3, 2020

@mcatanzaro apologies, I think we need to do it slightly differently.

hosts: resolve [!UNAVAIL=return] myhostname files dns {exclude if "with-custom-hosts"}

This should become

hosts: resolve [!UNAVAIL=return] files myhostname dns {exclude if "with-custom-hosts"}

The only change is to switch files and myhostname, so that explicit admin config has higher priority. This is also what resolved does internally, so we get same rules if resolve succeeds and if it fails for lookups that are covered by config in files. (systemd/systemd@d296c20f1f)

But let's not move files to the front unconditionally. Instead, we should do this only when enabling nss-mdns. Since we're rewriting those lines with scriptlets to insert stuff, we can move files a bit earlier too. The advantage will be that we'll get slightly better performance when nss-mdns is not installed.

@mcatanzaro
Copy link

@keszybz I took that from https://src.fedoraproject.org/rpms/systemd/blob/master/f/systemd.spec#_664. There you have files first, then resolve, then myhostname, then dns. And it has to be that way because you say files has to be before nss-mdns, which will get inserted before resolve. authselect doesn't know about nss-mdns, so having it first unconditionally seems a lot easier than having it first only if nss-mdns is used.

@keszybz
Copy link

keszybz commented Sep 3, 2020

Yep, systemd.spec was updated yesterday, before my comment above.

having it first unconditionally seems a lot easier than having it first only if nss-mdns is used

Well, it's not "easier" in any significant way: we have to have a sed script either way, and having the sed script arrange things in a different order doesn't make it more complicated or brittle or anything.

@mcatanzaro
Copy link

OK, then we can rely on hosts: resolve [!UNAVAIL=return] files myhostname dns {exclude if "with-custom-hosts"} and leave rearranging things to the sed script. (In which case, we could have just as well expected the sed script to add resolved in the first place, as we do for nss-mdns, but whatever.)

@keszybz
Copy link

keszybz commented Sep 3, 2020

we could have just as well expected the sed script to add resolve

True. Nevertheless the argument for having resolve there by default is that it'll (nowadays) be always installed, while nss-mdns is optional. Ideally, we wouldn't have any sed scripts. And having a sed script to implement the default configuration is particularly ugly.

@pbrezina
Copy link
Member Author

pbrezina commented Sep 4, 2020

Maybe its time to finally drop the scriptlets, it has caused enough troubles. Packages should not touch the configuration, just document it and let the change on user. Since resolved should now be enabled by default there is no reason why it shouldn't be included in default glibc nsswitch.conf so there is no reason for systemd to use scriptlet.

Resolved is enabled by default since Fedora 33 so we need to reflect
this change in our profiles.

It should be OK to enabled it unconditionaly. The module is part of
systemd so it basically can not be uninstalled and it can be safely
disabled through `systemctl disable --now systemd-resolved.service`.

Resolves:
authselect#221
@pbrezina
Copy link
Member Author

pbrezina commented Sep 4, 2020

See the new patch.

@keszybz
Copy link

keszybz commented Sep 4, 2020

Yeah, I think that's good.

@pbrezina
Copy link
Member Author

pbrezina commented Sep 4, 2020

But this is pretty much the same as the version in rawhide. The only difference is that myhostname and files switched places. So how does this solves the mdns issue?

@keszybz
Copy link

keszybz commented Sep 4, 2020

The mdns issue is solved by adjusted scriptlets both in systemd and mdns.

If we were to drop scriptlets, we would indeed need to add mdns here, but so far this hasn't happened and I don't think we should do it now.

Maybe its time to finally drop the scriptlets, it has caused enough troubles.

Actually the scriptlets were not the issue... They were wrong but because of a faulty understanding of what should be done. A bunch of people looked at this and we all missed that mdns and resolved would interact badly.

Packages should not touch the configuration, just document it and let the change on user.

I disagree here. This is a low-level setting that we should adjust automatically. We just need to figure out how exactly it needs to look.

Since resolved should now be enabled by default there is no reason why it shouldn't be included in default glibc nsswitch.conf so there is no reason for systemd to use scriptlet.

I fully agree that the default config should not require a scriptlet. We have been trying for years to get various nss modules provided by systemd into the default glibc config, unsuccessfully. I would love to see that finally happen.

@mcatanzaro
Copy link

@keszybz won't this break the systemd package scriptlet? See: https://bugzilla.redhat.com/show_bug.cgi?id=1867830#c62

@pbrezina
Copy link
Member Author

pbrezina commented Oct 1, 2020

I'd like to get this answered before I push it to avoid further changes.

@mcatanzaro
Copy link

I think it will break the systemd package scriptlet, but that's not authselect's problem. That will need to be fixed in the systemd package.

@keszybz
Copy link

keszybz commented Nov 23, 2020

@keszybz won't this break the systemd package scriptlet? See: https://bugzilla.redhat.com/show_bug.cgi?id=1867830#c62

With this change the systemd scriptlet will not do anything because grep -E -q '^hosts:.* resolve' will succeed. I think it's OK to merge this. With this patch, we'll have the "optimum" configuration here, and the scriptlets will only have to adjust it when installing nss-mdns.

@keszybz
Copy link

keszybz commented Nov 23, 2020

I think systemd is OK. https://src.fedoraproject.org/rpms/nss-mdns/pull-request/4 is for nss-mdns.

@pbrezina
Copy link
Member Author

  • master
    • c5294c5 - profiles: add support for resolved

@pbrezina pbrezina closed this Nov 25, 2020
@pbrezina pbrezina deleted the resolved branch May 5, 2022 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants