Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-14 Thread Michael Marineau 
On Aug 14, 2014 1:21 AM, Lennart Poettering lenn...@poettering.net
wrote:

 On Mon, 04.08.14 10:05, Michael Marineau (michael.marin...@coreos.com)
wrote:

 Patch looks pretty good, though I'd really prefer if we could do the
 UseDomain= thing as discussed in the other mail, and not propagate
 DHCP-supplied domain names unless explicitly requested.

 This would means we probably mean we'd need two new sd-network.h calls:

 int sd_network_get_link_route_domains(int ifindex, char **domains);
 int sd_network_get_link_search_domains(int ifindex, char **domains);

 The former would return the list of domains whose requests shall be
 routed to the specified interface, and the latter would be the list of
 domains we actively use for searching single-label domains in.

 Any domains configured statically for a link in the .network files would
 be listed in both lists. And depending on the UseDomains= settings the
 dhcp provides domains might be listed on none, both or only one of
 them. or something like that...

 
   src/network/networkd-link.c|  9 +
   src/network/sd-network.c   | 24 
   src/resolve/resolved-link.c| 20 
   src/resolve/resolved-link.h|  2 ++
   src/resolve/resolved-manager.c | 10 +-
   src/systemd/sd-network.h   |  3 +++
   6 files changed, 67 insertions(+), 1 deletion(-)
 
  diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
  index 172be64..42d528f 100644
  --- a/src/network/networkd-link.c
  +++ b/src/network/networkd-link.c
  @@ -2385,6 +2385,15 @@ int link_save(Link *link) {
   (address + 1 ?   : ));
 
   fputs(\n, f);
  +
  +if (link-network-dhcp_domainname 
  +link-dhcp_lease) {
  +const char *domainname;
  +
  +r =
sd_dhcp_lease_get_domainname(link-dhcp_lease, domainname);
  +if (r = 0)
  +fprintf(f, DOMAINNAME=%s\n,
  domainname);

 THis should be plural really, from the beginning. After all the newer
 DHCP specs allow a full list... and we want to allow a full list to be
 provided in the .network files too...

 Lennart

 --
 Lennart Poettering, Red Hat

Right now the search domains DHCP option is unsupported so it is indeed
singular. Also I had assumed since search domains is both a different DHCP
option and a different resolve.conf option that they would be recorded
separately but I suppose the two options is more of a legacy artifact than
meaningful distinction so it is equally as valid to squash them together
into the search domain list. I am happy to write a follow up patch to
implement the search domains option and support providing additional
domains in the .network file but I think this patch can stand alone since
it fixes a regression.
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-14 Thread Lennart Poettering 
On Thu, 14.08.14 02:21, Lennart Poettering (lenn...@poettering.net) wrote:

 On Mon, 04.08.14 10:05, Michael Marineau (michael.marin...@coreos.com) wrote:
 
 Patch looks pretty good, though I'd really prefer if we could do the
 UseDomain= thing as discussed in the other mail, and not propagate
 DHCP-supplied domain names unless explicitly requested.
 
 This would means we probably mean we'd need two new sd-network.h calls:
 
 int sd_network_get_link_route_domains(int ifindex, char **domains);
 int sd_network_get_link_search_domains(int ifindex, char **domains);
 
 The former would return the list of domains whose requests shall be
 routed to the specified interface, and the latter would be the list of
 domains we actively use for searching single-label domains in.
 
 Any domains configured statically for a link in the .network files would
 be listed in both lists. And depending on the UseDomains= settings the
 dhcp provides domains might be listed on none, both or only one of
 them. or something like that...

So, I have thought about this a bit more over night... I think this
would still be too complicated, and I don't really see the need to make
the dhcp search thing that configurable. I see no risk in strictly
making single-label host names searchable in all configured domains of
the interface and nothing else...

I think I am back at suggesting that there should be the follow .network 
settings:

   [Network]
   Domains=0pointer.de redhat.com

   [DHCP]
   UseDomains=yes|no

And one sd-network function:

   int sd_network_get_link_domains(int ifindex, char **domains);

And that should be it.

UseDomain= should have the effect of adding the domains from dhcp option
15 and 119 to the list of domains for the interface. And
sd_network_get_link_domains() should then return a single list, of
deduplicated entries, with the domains specified in Domains= first, and
then the dhcp domains possible added in at the end.

Zbigniew, I think this simplification would be beneficial, as I really
don't see the need to make the search vs. route domain thing
configurable

Tom, what's your take on all of this?

The domain data from DHCP should be thoroughly verified btw. For
example, if is_hostname() is true for some domain acquired via DHCP we
should ignore it, and we should make sure that the domain name is
actually valid, and so on.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-14 Thread Tom Gundersen 
On Thu, Aug 14, 2014 at 1:11 PM, Lennart Poettering
lenn...@poettering.net wrote:

 UseDomain= should have the effect of adding the domains from dhcp option
 15 and 119 to the list of domains for the interface. And
 sd_network_get_link_domains() should then return a single list, of
 deduplicated entries, with the domains specified in Domains= first, and
 then the dhcp domains possible added in at the end.

 Zbigniew, I think this simplification would be beneficial, as I really
 don't see the need to make the search vs. route domain thing
 configurable

 Tom, what's your take on all of this?


Sorry for taking forever to answer to this thread. I have been going
back and forth in my mind about how this should look.

I think in the end I essentially agree with Lennart's last suggestion.
Let's make this dead-simple and collapse the search/route domains for
each link into a single list. I think this is fine given that we
restrict the search behaviour to single-labels.

My only hesitation has been that I can imagine someone wanting to add
search domains that do not imply anything about routing. However, I
think in this case it does not make much sense to make this per-link,
but it should rather be a global SearchDomains= option (in
resolved.conf) or something to that effect.

Zbigniew, Michael, what do you think?

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-14 Thread Lennart Poettering 
On Thu, 14.08.14 13:27, Tom Gundersen (t...@jklm.no) wrote:

 
 On Thu, Aug 14, 2014 at 1:11 PM, Lennart Poettering
 lenn...@poettering.net wrote:
 
  UseDomain= should have the effect of adding the domains from dhcp option
  15 and 119 to the list of domains for the interface. And
  sd_network_get_link_domains() should then return a single list, of
  deduplicated entries, with the domains specified in Domains= first, and
  then the dhcp domains possible added in at the end.
 
  Zbigniew, I think this simplification would be beneficial, as I really
  don't see the need to make the search vs. route domain thing
  configurable
 
  Tom, what's your take on all of this?
 
 
 Sorry for taking forever to answer to this thread. I have been going
 back and forth in my mind about how this should look.
 
 I think in the end I essentially agree with Lennart's last suggestion.
 Let's make this dead-simple and collapse the search/route domains for
 each link into a single list. I think this is fine given that we
 restrict the search behaviour to single-labels.
 
 My only hesitation has been that I can imagine someone wanting to add
 search domains that do not imply anything about routing. However, I
 think in this case it does not make much sense to make this per-link,
 but it should rather be a global SearchDomains= option (in
 resolved.conf) or something to that effect.
 
 Zbigniew, Michael, what do you think?

Tom reminded me of the fact now, that at the systemd hackfest in Brno
last week (which really was more a talkfest) people suggested we
should actually make it possible that if you go to lets say
xhamster.com you never ever want this to be resolved via the redhat
VPN. That probably makes a lot of sense.

Hence, I would suggest adding a syntax of:

   [Network]
   Domains=*

which would have the effect to route all DNS traffic that is not
explicitly routed somewhereelse to this interface.

Internally, this would just set a boolean, which could be queried with:

   int sd_network_link_get_wildcard_domain(int ifindex);

or so, which would return 0 or 1 or negative -errno...

But then again, this doesn't have to be there from day one, we can add
that later... But of course, I'd love to see this done early on, too,
after all the porn usecase is a major one.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-14 Thread Tom Gundersen 
On Thu, Aug 14, 2014 at 1:47 PM, Lennart Poettering
lenn...@poettering.net wrote:
 On Thu, 14.08.14 13:27, Tom Gundersen (t...@jklm.no) wrote:


 On Thu, Aug 14, 2014 at 1:11 PM, Lennart Poettering
 lenn...@poettering.net wrote:
 
  UseDomain= should have the effect of adding the domains from dhcp option
  15 and 119 to the list of domains for the interface. And
  sd_network_get_link_domains() should then return a single list, of
  deduplicated entries, with the domains specified in Domains= first, and
  then the dhcp domains possible added in at the end.
 
  Zbigniew, I think this simplification would be beneficial, as I really
  don't see the need to make the search vs. route domain thing
  configurable
 
  Tom, what's your take on all of this?


 Sorry for taking forever to answer to this thread. I have been going
 back and forth in my mind about how this should look.

 I think in the end I essentially agree with Lennart's last suggestion.
 Let's make this dead-simple and collapse the search/route domains for
 each link into a single list. I think this is fine given that we
 restrict the search behaviour to single-labels.

 My only hesitation has been that I can imagine someone wanting to add
 search domains that do not imply anything about routing. However, I
 think in this case it does not make much sense to make this per-link,
 but it should rather be a global SearchDomains= option (in
 resolved.conf) or something to that effect.

 Zbigniew, Michael, what do you think?

 Tom reminded me of the fact now, that at the systemd hackfest in Brno
 last week (which really was more a talkfest) people suggested we
 should actually make it possible that if you go to lets say
 xhamster.com you never ever want this to be resolved via the redhat
 VPN. That probably makes a lot of sense.

 Hence, I would suggest adding a syntax of:

[Network]
Domains=*

 which would have the effect to route all DNS traffic that is not
 explicitly routed somewhereelse to this interface.

 Internally, this would just set a boolean, which could be queried with:

int sd_network_link_get_wildcard_domain(int ifindex);

 or so, which would return 0 or 1 or negative -errno...

 But then again, this doesn't have to be there from day one, we can add
 that later... But of course, I'd love to see this done early on, too,
 after all the porn usecase is a major one.


As discussed off-list, I agree with adding this API / behaviour.

Cheers,

Tom
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-13 Thread Lennart Poettering 
On Mon, 04.08.14 18:33, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

  Does this make any sense? Opinions?
 Yes, totally makes sense. But the name UseDomains is confusing though.
 IIUC, we have two separate concepts:
  1. using a specific interface (and a set of DNS resolvers tied to it)
 when resolving specific fqdns (resolve list)
  2. using specifc fqdns when a single-label name is given (search list)
 
 Your description sounds like DHCP.UseDomains=yes would mean using
 the DHCP-supplied list for 2. I think it should be used for 1 too.
 So maybe there should be
 
UseDomains=resolve|search|all
 
 (all in case we add futher options later on).

Makes sense, I agree.

Tom, can we get the domain list as an API into sd-network?

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-13 Thread Lennart Poettering 
On Mon, 04.08.14 10:05, Michael Marineau (michael.marin...@coreos.com) wrote:

Patch looks pretty good, though I'd really prefer if we could do the
UseDomain= thing as discussed in the other mail, and not propagate
DHCP-supplied domain names unless explicitly requested.

This would means we probably mean we'd need two new sd-network.h calls:

int sd_network_get_link_route_domains(int ifindex, char **domains);
int sd_network_get_link_search_domains(int ifindex, char **domains);

The former would return the list of domains whose requests shall be
routed to the specified interface, and the latter would be the list of
domains we actively use for searching single-label domains in.

Any domains configured statically for a link in the .network files would
be listed in both lists. And depending on the UseDomains= settings the
dhcp provides domains might be listed on none, both or only one of
them. or something like that...

 
  src/network/networkd-link.c|  9 +
  src/network/sd-network.c   | 24 
  src/resolve/resolved-link.c| 20 
  src/resolve/resolved-link.h|  2 ++
  src/resolve/resolved-manager.c | 10 +-
  src/systemd/sd-network.h   |  3 +++
  6 files changed, 67 insertions(+), 1 deletion(-)
 
 diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
 index 172be64..42d528f 100644
 --- a/src/network/networkd-link.c
 +++ b/src/network/networkd-link.c
 @@ -2385,6 +2385,15 @@ int link_save(Link *link) {
  (address + 1 ?   : ));
  
  fputs(\n, f);
 +
 +if (link-network-dhcp_domainname 
 +link-dhcp_lease) {
 +const char *domainname;
 +
 +r = sd_dhcp_lease_get_domainname(link-dhcp_lease, 
 domainname);
 +if (r = 0)
 +fprintf(f, DOMAINNAME=%s\n,
 domainname);

THis should be plural really, from the beginning. After all the newer
DHCP specs allow a full list... and we want to allow a full list to be
provided in the .network files too...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-04 Thread Lennart Poettering 
On Wed, 30.07.14 00:37, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

  +_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
  +_cleanup_free_ char *s = NULL, *p = NULL;
  +int r;
  +
  +assert_return(ifindex  0, -EINVAL);
  +assert_return(domainname, -EINVAL);
  +
  +if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
  +return -ENOMEM;
 Not terribly important, but please spell that as:
 
char p[sizeof(/run/systemd/netif/links/) + 
 DECIMAL_STRING_MAX(int)];
  snprintf(p, sizeof(p), /run/systemd/netif/links/%d,
  ifindex);

Actually, I'd even use normal sprintf() here, not snprintf(), after all,
we carefully sized the string anyway, hence no need to enforce the size
limit here. In particular since snprintf() doesn't add a trailing NULL
if it truncates the string, which makes the whole excercise kinda
pointless... an snprintf() without something like char_array_0() invoked
right after it always raises my eyebrows. Using sprintf() here makes is
clear to me that the buffer was carefully sized before, so I think would
be preferrable...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-04 Thread Zbigniew Jędrzejewski-Szmek 
On Mon, Aug 04, 2014 at 05:21:46PM +0200, Lennart Poettering wrote:
 On Wed, 30.07.14 00:37, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
 
   +_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
   +_cleanup_free_ char *s = NULL, *p = NULL;
   +int r;
   +
   +assert_return(ifindex  0, -EINVAL);
   +assert_return(domainname, -EINVAL);
   +
   +if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
   +return -ENOMEM;
  Not terribly important, but please spell that as:
  
 char p[sizeof(/run/systemd/netif/links/) + 
  DECIMAL_STRING_MAX(int)];
 snprintf(p, sizeof(p), /run/systemd/netif/links/%d,
 ifindex);
 
 Actually, I'd even use normal sprintf() here, not snprintf(), after all,
 we carefully sized the string anyway, hence no need to enforce the size
 limit here. Using sprintf() here makes is
 clear to me that the buffer was carefully sized before, so I think would
 be preferrable...
On my TODO list is adding snprintf_check (or snprintf_assert, or snprintf_sized,
not sure about the name), which would wrap the snprintf in an assert
to check that there was no truncation.

 In particular since snprintf() doesn't add a trailing NULL
 if it truncates the string, which makes the whole excercise kinda
 pointless... an snprintf() without something like char_array_0() invoked
 right after it always raises my eyebrows. 
Are you sure?

snprintf(3) implies that the terminating byte is always written. Testing
confirms that fwiw.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-04 Thread Lennart Poettering 
On Tue, 29.07.14 14:48, Michael Marineau (michael.marin...@coreos.com) wrote:

 When the code for generating resolv.conf was moved from networkd to
 resolved the DHCP domain name code was dropped.

Hmm, we really should figure out how we want to support all of this in
the long run, between networkd and resolved.

Quite frankly I believe the entire domain and search list logic is
idiotic. It's an invitation for a security breach without bounds. In a
world where DNSSEC is supposed to validate full hostnames, and what they
refer to it's simply wrong to mangle user input like that.

But then again, I figure we cannot just not support it. Administrators
traditionally used that and most would probably defend it as the most
useful thing on earth. But I guess we can at least try to make this less
broken...

For example, I don't really understand where the effective difference
between resolv.conf's domain and search setting is supposed to be
for resolvers. Why would anyone configure domain, if he could just
as well configure search?

resolved's DNS resolver is actually aware of multiple interfaces and
their specific DNS servers (at least when used in conjuncion with
networkd). It will issue DNS requests in parallel on all interfaces, in
order to deal with the VPN vs. local LAN problem where a company VPN and
the local LAN might both define local, private names, and we should be
able to resolve them both at the same time. For this kind of setup it is
sometimes useful however to bind a specific domain exlcusively to one
interface though, i.e. to disable the logic of simulatenously asking all
interfaces after all. For example, declaring that *.redhat.com should
strictly go into the VPN is a good thing, in order not leak information
about redhat-internal hosts onto public DNS servers... Now, for this
kind of stuff we should allow defining a list of exclusive domains per
interface. This is different from a search list however, as this is
simply about where to route DNS requests to, and not about appending
suffixes.

I think if we apply search lists then we should probably restrict this
to single-label names, for security reasons. That way it will only
compete against LLMNR (since llmnr is used exclusively for single-label
names, too), but never be applied to fqdns. Given that one can trust
LLMNR and DHCP pretty much to the same degree that should be OK. 

So, maybe we should have just two options:

[Network]
Domains=list of domains

[DHCP]
UseDomains=yes/no (default: no)

Domains= configures a list of domains specific to the interface. This
would be used for DNS routing by resolved, as pointed out above. It
would also be used as search list for single-label names.

And UseDomains= in the [DHCP] section would have the result of adding a
search list supplied via DHCP (either option 119 or 15) to the manually
configured search list (at the end). It would be off by default, for
security reasons.

Does this make any sense? Opinions?

Note that this would be different from glibc's resolver, where the
search list is applied to *all* domains names, regardless if they are
single-label or not. Also, it will be different from existing DHCP
clients, as they all tend to apply the search list supplied from DHCP
without any restrictions.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-04 Thread Lennart Poettering 
On Mon, 04.08.14 17:27, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

 
 On Mon, Aug 04, 2014 at 05:21:46PM +0200, Lennart Poettering wrote:
  On Wed, 30.07.14 00:37, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) 
  wrote:
  
+_public_ int sd_network_get_domainname(int ifindex, char **domainname) 
{
+_cleanup_free_ char *s = NULL, *p = NULL;
+int r;
+
+assert_return(ifindex  0, -EINVAL);
+assert_return(domainname, -EINVAL);
+
+if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
+return -ENOMEM;
   Not terribly important, but please spell that as:
   
  char p[sizeof(/run/systemd/netif/links/) + 
   DECIMAL_STRING_MAX(int)];
snprintf(p, sizeof(p), /run/systemd/netif/links/%d,
ifindex);
  
  Actually, I'd even use normal sprintf() here, not snprintf(), after all,
  we carefully sized the string anyway, hence no need to enforce the size
  limit here. Using sprintf() here makes is
  clear to me that the buffer was carefully sized before, so I think would
  be preferrable...
 On my TODO list is adding snprintf_check (or snprintf_assert, or 
 snprintf_sized,
 not sure about the name), which would wrap the snprintf in an assert
 to check that there was no truncation.
 
  In particular since snprintf() doesn't add a trailing NULL
  if it truncates the string, which makes the whole excercise kinda
  pointless... an snprintf() without something like char_array_0() invoked
  right after it always raises my eyebrows. 
 Are you sure?
 
 snprintf(3) implies that the terminating byte is always written. Testing
 confirms that fwiw.

Hmm, indeed. Apparently glibc always does that. Old solaris and windows
don't however, but we don't care about that...

So I figure we can drop char_array_0() invocations at most places,
possibly even get rid of the macro definition entirely...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-04 Thread Mantas Mikulėnas 
On Aug 4, 2014 7:05 PM, Lennart Poettering lenn...@poettering.net wrote:

 On Tue, 29.07.14 14:48, Michael Marineau (michael.marin...@coreos.com)
wrote:

  When the code for generating resolv.conf was moved from networkd to
  resolved the DHCP domain name code was dropped.

 Hmm, we really should figure out how we want to support all of this in
 the long run, between networkd and resolved.

 Quite frankly I believe the entire domain and search list logic is
 idiotic. It's an invitation for a security breach without bounds. In a
 world where DNSSEC is supposed to validate full hostnames, and what they
 refer to it's simply wrong to mangle user input like that.

 But then again, I figure we cannot just not support it. Administrators
 traditionally used that and most would probably defend it as the most
 useful thing on earth. But I guess we can at least try to make this less
 broken...

On the one hand, it certainly saves a lot of time typing, and I don't see
how it affects DNSSEC given that the validator always sees full names.

On the other hand, it *does* break TLS certificate validation, as well as
Kerberos authentication, both of which want an exact FQDN match.

So 'ping' and 'mtr' is probably 99% of my use of the search list; full
domain name everywhere else. So I probably wouldn't miss the search list
much, myself.

(glibc has a nice alternative, $HOSTALIASES, allowing each user to
configure alias › domain name mappings. Interestingly, setuid programs
ignore that completely for security reasons, so it's unusable with
ping/mtr/traceroute, precisely where I'd want it most.)


 For example, I don't really understand where the effective difference
 between resolv.conf's domain and search setting is supposed to be
 for resolvers. Why would anyone configure domain, if he could just
 as well configure search?

If I remember correctly from the resolv.conf manpage or somewhere such,
there's *no* difference other than 'domain' being limited to one suffix.

 resolved's DNS resolver is actually aware of multiple interfaces and
 their specific DNS servers (at least when used in conjuncion with
 networkd). It will issue DNS requests in parallel on all interfaces, in
 order to deal with the VPN vs. local LAN problem where a company VPN and
 the local LAN might both define local, private names, and we should be
 able to resolve them both at the same time. For this kind of setup it is
 sometimes useful however to bind a specific domain exlcusively to one
 interface though, i.e. to disable the logic of simulatenously asking all
 interfaces after all. For example, declaring that *.redhat.com should
 strictly go into the VPN is a good thing, in order not leak information
 about redhat-internal hosts onto public DNS servers... Now, for this
 kind of stuff we should allow defining a list of exclusive domains per
 interface. This is different from a search list however, as this is
 simply about where to route DNS requests to, and not about appending
 suffixes.

In other words, the Linux resolver is finally as smart as Windows has been
for a decade :D

 I think if we apply search lists then we should probably restrict this
 to single-label names, for security reasons. That way it will only
 compete against LLMNR (since llmnr is used exclusively for single-label
 names, too), but never be applied to fqdns. Given that one can trust
 LLMNR and DHCP pretty much to the same degree that should be OK.

Well, I imagine admins *do* filter DHCP packets from untrusted devices.
I've even seen switches capable of this.

 So, maybe we should have just two options:

 [Network]
 Domains=list of domains

 [DHCP]
 UseDomains=yes/no (default: no)

 Domains= configures a list of domains specific to the interface. This
 would be used for DNS routing by resolved, as pointed out above. It
 would also be used as search list for single-label names.

 And UseDomains= in the [DHCP] section would have the result of adding a
 search list supplied via DHCP (either option 119 or 15) to the manually
 configured search list (at the end). It would be off by default, for
 security reasons.

 Does this make any sense? Opinions?

 Note that this would be different from glibc's resolver, where the
 search list is applied to *all* domains names, regardless if they are
 single-label or not.

The one label restriction sounds like an improvement, though I wonder if
someone actually relies on the old behavior.

I always disliked how Windows would first attempt resolving
google.com.example.com. and only then google.com. unless I manually added
. at the top of the list...

-- 
Mantas Mikulėnas graw...@gmail.com
// sent from phone
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-04 Thread Zbigniew Jędrzejewski-Szmek 
On Mon, Aug 04, 2014 at 06:05:05PM +0200, Lennart Poettering wrote:
 On Tue, 29.07.14 14:48, Michael Marineau (michael.marin...@coreos.com) wrote:
 
  When the code for generating resolv.conf was moved from networkd to
  resolved the DHCP domain name code was dropped.
 
 Hmm, we really should figure out how we want to support all of this in
 the long run, between networkd and resolved.
 
 Quite frankly I believe the entire domain and search list logic is
 idiotic. It's an invitation for a security breach without bounds. In a
 world where DNSSEC is supposed to validate full hostnames, and what they
 refer to it's simply wrong to mangle user input like that.
 
 But then again, I figure we cannot just not support it. Administrators
 traditionally used that and most would probably defend it as the most
 useful thing on earth. But I guess we can at least try to make this less
 broken...
 
 For example, I don't really understand where the effective difference
 between resolv.conf's domain and search setting is supposed to be
 for resolvers. Why would anyone configure domain, if he could just
 as well configure search?
 
 resolved's DNS resolver is actually aware of multiple interfaces and
 their specific DNS servers (at least when used in conjuncion with
 networkd). It will issue DNS requests in parallel on all interfaces, in
 order to deal with the VPN vs. local LAN problem where a company VPN and
 the local LAN might both define local, private names, and we should be
 able to resolve them both at the same time. For this kind of setup it is
 sometimes useful however to bind a specific domain exlcusively to one
 interface though, i.e. to disable the logic of simulatenously asking all
 interfaces after all. For example, declaring that *.redhat.com should
 strictly go into the VPN is a good thing, in order not leak information
 about redhat-internal hosts onto public DNS servers... Now, for this
 kind of stuff we should allow defining a list of exclusive domains per
 interface. This is different from a search list however, as this is
 simply about where to route DNS requests to, and not about appending
 suffixes.
 
 I think if we apply search lists then we should probably restrict this
 to single-label names, for security reasons. That way it will only
 compete against LLMNR (since llmnr is used exclusively for single-label
 names, too), but never be applied to fqdns. Given that one can trust
 LLMNR and DHCP pretty much to the same degree that should be OK. 
 
 So, maybe we should have just two options:
 
 [Network]
 Domains=list of domains
 
 [DHCP]
 UseDomains=yes/no (default: no)
 
 Domains= configures a list of domains specific to the interface. This
 would be used for DNS routing by resolved, as pointed out above. It
 would also be used as search list for single-label names.
 
 And UseDomains= in the [DHCP] section would have the result of adding a
 search list supplied via DHCP (either option 119 or 15) to the manually
 configured search list (at the end). It would be off by default, for
 security reasons.
 
 Does this make any sense? Opinions?
Yes, totally makes sense. But the name UseDomains is confusing though.
IIUC, we have two separate concepts:
 1. using a specific interface (and a set of DNS resolvers tied to it)
when resolving specific fqdns (resolve list)
 2. using specifc fqdns when a single-label name is given (search list)

Your description sounds like DHCP.UseDomains=yes would mean using
the DHCP-supplied list for 2. I think it should be used for 1 too.
So maybe there should be

   UseDomains=resolve|search|all

(all in case we add futher options later on).

 Note that this would be different from glibc's resolver, where the
 search list is applied to *all* domains names, regardless if they are
 single-label or not. Also, it will be different from existing DHCP
 clients, as they all tend to apply the search list supplied from DHCP
 without any restrictions.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-04 Thread Michael Marineau 
When the code for generating resolv.conf was moved from networkd to
resolved the DHCP domain name code was dropped.
---

This is a refresh of the patch on recent master with a little bit of
cleanup from the last. Regarding the robustness/correctness/etc of
setting the domain resolv.conf attribute from DNS, I don't think that
is practical to address in this patch. The implementation is already
clearly incomplete because networkd doesn't handle search domains
which is actually an entirely different option. My goal here is to just
fix the regression from when resolved was first introduced.

The most common setup is for domain to correspond to the domain suffix
for the local host name. If you are concerned about which interface's
domain attribute wins and lands in resolv.conf, there is a related issue
of which interface's host name winds up being applied as the host's
transient host name. What ever interface wins the two should probably
match but this is complicated significantly by the two being handled by
different daemons, resolved vs hostnamed, with two different
integration points with networkd, reading state files in /run vs dbus
method calls. I don't have a good recommendation to make sense of any of
this right now.

 src/network/networkd-link.c|  9 +
 src/network/sd-network.c   | 24 
 src/resolve/resolved-link.c| 20 
 src/resolve/resolved-link.h|  2 ++
 src/resolve/resolved-manager.c | 10 +-
 src/systemd/sd-network.h   |  3 +++
 6 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 172be64..42d528f 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -2385,6 +2385,15 @@ int link_save(Link *link) {
 (address + 1 ?   : ));
 
 fputs(\n, f);
+
+if (link-network-dhcp_domainname 
+link-dhcp_lease) {
+const char *domainname;
+
+r = sd_dhcp_lease_get_domainname(link-dhcp_lease, 
domainname);
+if (r = 0)
+fprintf(f, DOMAINNAME=%s\n, domainname);
+}
 }
 
 if (link-dhcp_lease) {
diff --git a/src/network/sd-network.c b/src/network/sd-network.c
index bfb8321..a427a27 100644
--- a/src/network/sd-network.c
+++ b/src/network/sd-network.c
@@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char ***ret) {
 return network_get_strv(NTP, ifindex, ret);
 }
 
+_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
+_cleanup_free_ char *s = NULL, *p = NULL;
+int r;
+
+assert_return(ifindex  0, -EINVAL);
+assert_return(domainname, -EINVAL);
+
+if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
+return -ENOMEM;
+
+r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL);
+if (r == -ENOENT)
+return -ENODATA;
+else if (r  0)
+return r;
+else if (!s)
+return -EIO;
+
+*domainname = s;
+s = NULL;
+
+return 0;
+}
+
 static inline int MONITOR_TO_FD(sd_network_monitor *m) {
 return (int) (unsigned long) m - 1;
 }
diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c
index 2c02f09..9d582e4 100644
--- a/src/resolve/resolved-link.c
+++ b/src/resolve/resolved-link.c
@@ -74,6 +74,7 @@ Link *link_free(Link *l) {
 while (l-dns_servers)
 dns_server_free(l-dns_servers);
 
+free(l-domainname);
 free(l);
 return NULL;
 }
@@ -188,10 +189,29 @@ clear:
 return r;
 }
 
+static int link_update_domainname(Link *l) {
+char *domainname = NULL;
+int r;
+
+assert(l);
+
+free(l-domainname);
+l-domainname = NULL;
+
+r = sd_network_get_domainname(l-ifindex, domainname);
+if (r  0)
+return r;
+
+l-domainname = domainname;
+
+return 0;
+}
+
 int link_update_monitor(Link *l) {
 assert(l);
 
 link_update_dns_servers(l);
+link_update_domainname(l);
 link_allocate_scopes(l);
 link_add_rrs(l, false);
 
diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h
index 38bb392..eed9f42 100644
--- a/src/resolve/resolved-link.h
+++ b/src/resolve/resolved-link.h
@@ -68,6 +68,8 @@ struct Link {
 
 RateLimit mdns_ratelimit;
 RateLimit llmnr_ratelimit;
+
+char *domainname;
 };
 
 int link_new(Manager *m, Link **ret, int ifindex);
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index 1b6dc8a..8f28eaf 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -648,6 +648,7 @@ int manager_write_resolv_conf(Manager *m) {
 static const char path[] = /run/systemd/resolve/resolv.conf;

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-08-02 Thread Zbigniew Jędrzejewski-Szmek 
On Tue, Jul 29, 2014 at 04:54:24PM -0700, Michael Marineau wrote:
 On Tue, Jul 29, 2014 at 3:37 PM, Zbigniew Jędrzejewski-Szmek
 zbys...@in.waw.pl wrote:
  On Tue, Jul 29, 2014 at 02:48:18PM -0700, Michael Marineau wrote:
  When the code for generating resolv.conf was moved from networkd to
  resolved the DHCP domain name code was dropped.
  ---
 
  This is a resend, rebased since some recent changes changed how this
  patch needed to be implemented.
 
   src/network/networkd-link.c| 13 +
   src/network/sd-network.c   | 24 
   src/resolve/resolved-link.c| 20 
   src/resolve/resolved-link.h|  2 ++
   src/resolve/resolved-manager.c | 10 +-
   src/systemd/sd-network.h   |  3 +++
   6 files changed, 71 insertions(+), 1 deletion(-)
 
  diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
  index 3b8b7ed..827c428 100644
  --- a/src/network/networkd-link.c
  +++ b/src/network/networkd-link.c
  @@ -2451,6 +2451,19 @@ int link_save(Link *link) {
   (address + 1 ?   : ));
 
   fputs(\n, f);
  +
  +fprintf(f, DOMAINNAME=);
  +
  +if (link-network-dhcp_domainname 
  +link-dhcp_lease) {
  +const char *domainname;
  +
  +r = 
  sd_dhcp_lease_get_domainname(link-dhcp_lease, domainname);
  +if (r = 0)
  +fputs(domainname, f);
  +}
  +
  +fputs(\n, f);
  Is it really necessary to write anything if the name is not available?
  Other parts of this function don't write anyting in similar cases.
 
 I was just matching the above lines which may write DNS= or NTP= with
 blank values. I don't think it matters either way. Omitting
 DOMAINNAME= if it is blank certainly looks a little cleaner since the
 writes get squashed into a single fprintf. Will update.
You're right, I misread the surrounding code. It would probably be nice
to not write anything in thos cases but it should be a separate patch anyway,
that does all the places at once.

   if (link-dhcp_lease) {
  diff --git a/src/network/sd-network.c b/src/network/sd-network.c
  index bfb8321..a427a27 100644
  --- a/src/network/sd-network.c
  +++ b/src/network/sd-network.c
  @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char 
  ***ret) {
   return network_get_strv(NTP, ifindex, ret);
   }
 
  +_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
  +_cleanup_free_ char *s = NULL, *p = NULL;
  +int r;
  +
  +assert_return(ifindex  0, -EINVAL);
  +assert_return(domainname, -EINVAL);
  +
  +if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
  +return -ENOMEM;
  Not terribly important, but please spell that as:
 
 char p[sizeof(/run/systemd/netif/links/) + 
  DECIMAL_STRING_MAX(int)];
 snprintf(p, sizeof(p), /run/systemd/netif/links/%d, ifindex);
 
 This was copied verbatim from similar functions in this file, should I
 update the style of the others to match your suggestion? Why the
 preference of manually calculating a buffer length than using
 asprintf?
You're right, in this case asprintf is totally appropriate.

  +r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL);
  +if (r == -ENOENT)
  +return -ENODATA;
  +else if (r  0)
  +return r;
  +else if (!s)
  +return -EIO;
  +
  +*domainname = s;
  +s = NULL;
  +
  +return 0;
  +}
  +
   static inline int MONITOR_TO_FD(sd_network_monitor *m) {
   return (int) (unsigned long) m - 1;
   }
  diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c
  index 6ac7c5b..f6b7f6a 100644
  --- a/src/resolve/resolved-link.c
  +++ b/src/resolve/resolved-link.c
  @@ -77,6 +77,7 @@ Link *link_free(Link *l) {
   while (l-dns_servers)
   dns_server_free(l-dns_servers);
 
  +free(l-domainname);
   free(l);
   return NULL;
   }
  @@ -191,10 +192,29 @@ clear:
   return r;
   }
 
  +static int link_update_domainname(Link *l) {
  +char *domainname = NULL;
  +int r;
  +
  +assert(l);
  +
  +free(l-domainname);
  +l-domainname = NULL;
  +
  +r = sd_network_get_domainname(l-ifindex, domainname);
  +if (r  0)
  +return r;
  +
  +l-domainname = domainname;
  +
  +return 0;
  +}
  +
   int link_update_monitor(Link *l) {
   assert(l);
 
   link_update_dns_servers(l);
  +link_update_domainname(l);
   link_allocate_scopes(l);
   link_add_rrs(l);
 
  diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h
  index f58bd54..9730aec 100644
  --- a/src/resolve/resolved-link.h

[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-07-29 Thread Michael Marineau 
When the code for generating resolv.conf was moved from networkd to
resolved the DHCP domain name code was dropped.
---

This is a resend, rebased since some recent changes changed how this
patch needed to be implemented.

 src/network/networkd-link.c| 13 +
 src/network/sd-network.c   | 24 
 src/resolve/resolved-link.c| 20 
 src/resolve/resolved-link.h|  2 ++
 src/resolve/resolved-manager.c | 10 +-
 src/systemd/sd-network.h   |  3 +++
 6 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 3b8b7ed..827c428 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -2451,6 +2451,19 @@ int link_save(Link *link) {
 (address + 1 ?   : ));
 
 fputs(\n, f);
+
+fprintf(f, DOMAINNAME=);
+
+if (link-network-dhcp_domainname 
+link-dhcp_lease) {
+const char *domainname;
+
+r = sd_dhcp_lease_get_domainname(link-dhcp_lease, 
domainname);
+if (r = 0)
+fputs(domainname, f);
+}
+
+fputs(\n, f);
 }
 
 if (link-dhcp_lease) {
diff --git a/src/network/sd-network.c b/src/network/sd-network.c
index bfb8321..a427a27 100644
--- a/src/network/sd-network.c
+++ b/src/network/sd-network.c
@@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char ***ret) {
 return network_get_strv(NTP, ifindex, ret);
 }
 
+_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
+_cleanup_free_ char *s = NULL, *p = NULL;
+int r;
+
+assert_return(ifindex  0, -EINVAL);
+assert_return(domainname, -EINVAL);
+
+if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
+return -ENOMEM;
+
+r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL);
+if (r == -ENOENT)
+return -ENODATA;
+else if (r  0)
+return r;
+else if (!s)
+return -EIO;
+
+*domainname = s;
+s = NULL;
+
+return 0;
+}
+
 static inline int MONITOR_TO_FD(sd_network_monitor *m) {
 return (int) (unsigned long) m - 1;
 }
diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c
index 6ac7c5b..f6b7f6a 100644
--- a/src/resolve/resolved-link.c
+++ b/src/resolve/resolved-link.c
@@ -77,6 +77,7 @@ Link *link_free(Link *l) {
 while (l-dns_servers)
 dns_server_free(l-dns_servers);
 
+free(l-domainname);
 free(l);
 return NULL;
 }
@@ -191,10 +192,29 @@ clear:
 return r;
 }
 
+static int link_update_domainname(Link *l) {
+char *domainname = NULL;
+int r;
+
+assert(l);
+
+free(l-domainname);
+l-domainname = NULL;
+
+r = sd_network_get_domainname(l-ifindex, domainname);
+if (r  0)
+return r;
+
+l-domainname = domainname;
+
+return 0;
+}
+
 int link_update_monitor(Link *l) {
 assert(l);
 
 link_update_dns_servers(l);
+link_update_domainname(l);
 link_allocate_scopes(l);
 link_add_rrs(l);
 
diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h
index f58bd54..9730aec 100644
--- a/src/resolve/resolved-link.h
+++ b/src/resolve/resolved-link.h
@@ -68,6 +68,8 @@ struct Link {
 
 RateLimit mdns_ratelimit;
 RateLimit llmnr_ratelimit;
+
+char *domainname;
 };
 
 int link_new(Manager *m, Link **ret, int ifindex);
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index a8715bd..253a97e 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -522,6 +522,7 @@ int manager_write_resolv_conf(Manager *m) {
 const char *path = /run/systemd/resolve/resolv.conf;
 _cleanup_free_ char *temp_path = NULL;
 _cleanup_fclose_ FILE *f = NULL;
+const char *domainname = NULL;
 unsigned count = 0;
 DnsServer *s;
 Iterator i;
@@ -542,13 +543,20 @@ int manager_write_resolv_conf(Manager *m) {
   # resolv.conf(5) in a different way, replace the symlink by a\n
   # static file or a different symlink.\n\n, f);
 
-HASHMAP_FOREACH(l, m-links, i)
+HASHMAP_FOREACH(l, m-links, i) {
 LIST_FOREACH(servers, s, l-dns_servers)
 write_resolve_conf_server(s, f, count);
 
+if (!domainname  l-domainname)
+domainname = l-domainname;
+}
+
 LIST_FOREACH(servers, s, m-dns_servers)
 write_resolve_conf_server(s, f, count);
 
+if (domainname)
+fprintf(f, domain %s\n, domainname);
+
 r = fflush_and_check(f);
 if (r  0)

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-07-29 Thread Zbigniew Jędrzejewski-Szmek 
On Tue, Jul 29, 2014 at 02:48:18PM -0700, Michael Marineau wrote:
 When the code for generating resolv.conf was moved from networkd to
 resolved the DHCP domain name code was dropped.
 ---
 
 This is a resend, rebased since some recent changes changed how this
 patch needed to be implemented.
 
  src/network/networkd-link.c| 13 +
  src/network/sd-network.c   | 24 
  src/resolve/resolved-link.c| 20 
  src/resolve/resolved-link.h|  2 ++
  src/resolve/resolved-manager.c | 10 +-
  src/systemd/sd-network.h   |  3 +++
  6 files changed, 71 insertions(+), 1 deletion(-)
 
 diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
 index 3b8b7ed..827c428 100644
 --- a/src/network/networkd-link.c
 +++ b/src/network/networkd-link.c
 @@ -2451,6 +2451,19 @@ int link_save(Link *link) {
  (address + 1 ?   : ));
  
  fputs(\n, f);
 +
 +fprintf(f, DOMAINNAME=);
 +
 +if (link-network-dhcp_domainname 
 +link-dhcp_lease) {
 +const char *domainname;
 +
 +r = sd_dhcp_lease_get_domainname(link-dhcp_lease, 
 domainname);
 +if (r = 0)
 +fputs(domainname, f);
 +}
 +
 +fputs(\n, f);
Is it really necessary to write anything if the name is not available?
Other parts of this function don't write anyting in similar cases.

  
  if (link-dhcp_lease) {
 diff --git a/src/network/sd-network.c b/src/network/sd-network.c
 index bfb8321..a427a27 100644
 --- a/src/network/sd-network.c
 +++ b/src/network/sd-network.c
 @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char 
 ***ret) {
  return network_get_strv(NTP, ifindex, ret);
  }
  
 +_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
 +_cleanup_free_ char *s = NULL, *p = NULL;
 +int r;
 +
 +assert_return(ifindex  0, -EINVAL);
 +assert_return(domainname, -EINVAL);
 +
 +if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
 +return -ENOMEM;
Not terribly important, but please spell that as:

   char p[sizeof(/run/systemd/netif/links/) + 
DECIMAL_STRING_MAX(int)];
   snprintf(p, sizeof(p), /run/systemd/netif/links/%d, ifindex);

 +r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL);
 +if (r == -ENOENT)
 +return -ENODATA;
 +else if (r  0)
 +return r;
 +else if (!s)
 +return -EIO;
 +
 +*domainname = s;
 +s = NULL;
 +
 +return 0;
 +}
 +
  static inline int MONITOR_TO_FD(sd_network_monitor *m) {
  return (int) (unsigned long) m - 1;
  }
 diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c
 index 6ac7c5b..f6b7f6a 100644
 --- a/src/resolve/resolved-link.c
 +++ b/src/resolve/resolved-link.c
 @@ -77,6 +77,7 @@ Link *link_free(Link *l) {
  while (l-dns_servers)
  dns_server_free(l-dns_servers);
  
 +free(l-domainname);
  free(l);
  return NULL;
  }
 @@ -191,10 +192,29 @@ clear:
  return r;
  }
  
 +static int link_update_domainname(Link *l) {
 +char *domainname = NULL;
 +int r;
 +
 +assert(l);
 +
 +free(l-domainname);
 +l-domainname = NULL;
 +
 +r = sd_network_get_domainname(l-ifindex, domainname);
 +if (r  0)
 +return r;
 +
 +l-domainname = domainname;
 +
 +return 0;
 +}
 +
  int link_update_monitor(Link *l) {
  assert(l);
  
  link_update_dns_servers(l);
 +link_update_domainname(l);
  link_allocate_scopes(l);
  link_add_rrs(l);
  
 diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h
 index f58bd54..9730aec 100644
 --- a/src/resolve/resolved-link.h
 +++ b/src/resolve/resolved-link.h
 @@ -68,6 +68,8 @@ struct Link {
  
  RateLimit mdns_ratelimit;
  RateLimit llmnr_ratelimit;
 +
 +char *domainname;
  };
  
  int link_new(Manager *m, Link **ret, int ifindex);
 diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
 index a8715bd..253a97e 100644
 --- a/src/resolve/resolved-manager.c
 +++ b/src/resolve/resolved-manager.c
 @@ -522,6 +522,7 @@ int manager_write_resolv_conf(Manager *m) {
  const char *path = /run/systemd/resolve/resolv.conf;
  _cleanup_free_ char *temp_path = NULL;
  _cleanup_fclose_ FILE *f = NULL;
 +const char *domainname = NULL;
  unsigned count = 0;
  DnsServer *s;
  Iterator i;
 @@ -542,13 +543,20 @@ int manager_write_resolv_conf(Manager *m) {
# resolv.conf(5) in a different way, replace the symlink by 
 a\n
# static file or a different symlink.\n\n, f);
  
 -

Re: [systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-07-29 Thread Michael Marineau 
On Tue, Jul 29, 2014 at 3:37 PM, Zbigniew Jędrzejewski-Szmek
zbys...@in.waw.pl wrote:
 On Tue, Jul 29, 2014 at 02:48:18PM -0700, Michael Marineau wrote:
 When the code for generating resolv.conf was moved from networkd to
 resolved the DHCP domain name code was dropped.
 ---

 This is a resend, rebased since some recent changes changed how this
 patch needed to be implemented.

  src/network/networkd-link.c| 13 +
  src/network/sd-network.c   | 24 
  src/resolve/resolved-link.c| 20 
  src/resolve/resolved-link.h|  2 ++
  src/resolve/resolved-manager.c | 10 +-
  src/systemd/sd-network.h   |  3 +++
  6 files changed, 71 insertions(+), 1 deletion(-)

 diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
 index 3b8b7ed..827c428 100644
 --- a/src/network/networkd-link.c
 +++ b/src/network/networkd-link.c
 @@ -2451,6 +2451,19 @@ int link_save(Link *link) {
  (address + 1 ?   : ));

  fputs(\n, f);
 +
 +fprintf(f, DOMAINNAME=);
 +
 +if (link-network-dhcp_domainname 
 +link-dhcp_lease) {
 +const char *domainname;
 +
 +r = sd_dhcp_lease_get_domainname(link-dhcp_lease, 
 domainname);
 +if (r = 0)
 +fputs(domainname, f);
 +}
 +
 +fputs(\n, f);
 Is it really necessary to write anything if the name is not available?
 Other parts of this function don't write anyting in similar cases.

I was just matching the above lines which may write DNS= or NTP= with
blank values. I don't think it matters either way. Omitting
DOMAINNAME= if it is blank certainly looks a little cleaner since the
writes get squashed into a single fprintf. Will update.



  if (link-dhcp_lease) {
 diff --git a/src/network/sd-network.c b/src/network/sd-network.c
 index bfb8321..a427a27 100644
 --- a/src/network/sd-network.c
 +++ b/src/network/sd-network.c
 @@ -175,6 +175,30 @@ _public_ int sd_network_get_ntp(int ifindex, char 
 ***ret) {
  return network_get_strv(NTP, ifindex, ret);
  }

 +_public_ int sd_network_get_domainname(int ifindex, char **domainname) {
 +_cleanup_free_ char *s = NULL, *p = NULL;
 +int r;
 +
 +assert_return(ifindex  0, -EINVAL);
 +assert_return(domainname, -EINVAL);
 +
 +if (asprintf(p, /run/systemd/netif/links/%d, ifindex)  0)
 +return -ENOMEM;
 Not terribly important, but please spell that as:

char p[sizeof(/run/systemd/netif/links/) + 
 DECIMAL_STRING_MAX(int)];
snprintf(p, sizeof(p), /run/systemd/netif/links/%d, ifindex);

This was copied verbatim from similar functions in this file, should I
update the style of the others to match your suggestion? Why the
preference of manually calculating a buffer length than using
asprintf?


 +r = parse_env_file(p, NEWLINE, DOMAINNAME, s, NULL);
 +if (r == -ENOENT)
 +return -ENODATA;
 +else if (r  0)
 +return r;
 +else if (!s)
 +return -EIO;
 +
 +*domainname = s;
 +s = NULL;
 +
 +return 0;
 +}
 +
  static inline int MONITOR_TO_FD(sd_network_monitor *m) {
  return (int) (unsigned long) m - 1;
  }
 diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c
 index 6ac7c5b..f6b7f6a 100644
 --- a/src/resolve/resolved-link.c
 +++ b/src/resolve/resolved-link.c
 @@ -77,6 +77,7 @@ Link *link_free(Link *l) {
  while (l-dns_servers)
  dns_server_free(l-dns_servers);

 +free(l-domainname);
  free(l);
  return NULL;
  }
 @@ -191,10 +192,29 @@ clear:
  return r;
  }

 +static int link_update_domainname(Link *l) {
 +char *domainname = NULL;
 +int r;
 +
 +assert(l);
 +
 +free(l-domainname);
 +l-domainname = NULL;
 +
 +r = sd_network_get_domainname(l-ifindex, domainname);
 +if (r  0)
 +return r;
 +
 +l-domainname = domainname;
 +
 +return 0;
 +}
 +
  int link_update_monitor(Link *l) {
  assert(l);

  link_update_dns_servers(l);
 +link_update_domainname(l);
  link_allocate_scopes(l);
  link_add_rrs(l);

 diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h
 index f58bd54..9730aec 100644
 --- a/src/resolve/resolved-link.h
 +++ b/src/resolve/resolved-link.h
 @@ -68,6 +68,8 @@ struct Link {

  RateLimit mdns_ratelimit;
  RateLimit llmnr_ratelimit;
 +
 +char *domainname;
  };

  int link_new(Manager *m, Link **ret, int ifindex);
 diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
 index a8715bd..253a97e 100644
 --- a/src/resolve/resolved-manager.c
 +++ b/src/resolve/resolved-manager.c
 @@ -522,6 +522,7 @@ int

[systemd-devel] [PATCH] resolved: re-add support for getting local domain from DHCP

2014-07-22 Thread Michael Marineau 
When the code for generating resolv.conf was moved from networkd to
resolved the DHCP domain name code was dropped.
---
 src/network/networkd-link.c|  2 ++
 src/network/sd-network.c   |  4 
 src/resolve/resolved-link.c| 31 +++
 src/resolve/resolved-link.h|  2 ++
 src/resolve/resolved-manager.c |  7 +++
 src/systemd/sd-network.h   |  3 +++
 6 files changed, 49 insertions(+)

diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c
index 0a6f524..afca172 100644
--- a/src/network/networkd-link.c
+++ b/src/network/networkd-link.c
@@ -2524,9 +2524,11 @@ int link_save(Link *link) {
 fprintf(f,
 DHCP_LEASE=%s\n
 DHCP_USE_DNS=%s\n
+DHCP_USE_DOMAINNAME=%s\n
 DHCP_USE_NTP=%s\n,
 link-lease_file,
 yes_no(link-network-dhcp_dns),
+yes_no(link-network-dhcp_domainname),
 yes_no(link-network-dhcp_ntp));
 } else
 unlink(link-lease_file);
diff --git a/src/network/sd-network.c b/src/network/sd-network.c
index 91d6275..5dfdd59 100644
--- a/src/network/sd-network.c
+++ b/src/network/sd-network.c
@@ -202,6 +202,10 @@ _public_ int sd_network_dhcp_use_dns(int ifindex) {
 return network_get_boolean(DHCP_USE_DNS, ifindex);
 }
 
+_public_ int sd_network_dhcp_use_domainname(int ifindex) {
+return network_get_boolean(DHCP_USE_DOMAINNAME, ifindex);
+}
+
 _public_ int sd_network_dhcp_use_ntp(int ifindex) {
 return network_get_boolean(DHCP_USE_NTP, ifindex);
 }
diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c
index 078301a..c0b19a6 100644
--- a/src/resolve/resolved-link.c
+++ b/src/resolve/resolved-link.c
@@ -77,6 +77,7 @@ Link *link_free(Link *l) {
 while (l-link_dns_servers)
 dns_server_free(l-link_dns_servers);
 
+free(l-dhcp_domainname);
 free(l);
 return NULL;
 }
@@ -249,11 +250,41 @@ clear:
 return r;
 }
 
+static int link_update_dhcp_domainname(Link *l) {
+_cleanup_dhcp_lease_unref_ sd_dhcp_lease *lease = NULL;
+const char *domainname = NULL;
+int r;
+
+assert(l);
+
+free(l-dhcp_domainname);
+l-dhcp_domainname = NULL;
+
+r = sd_network_dhcp_use_dns(l-ifindex);
+if (r = 0)
+return r;
+
+r = sd_network_get_dhcp_lease(l-ifindex, lease);
+if (r  0)
+return r;
+
+r = sd_dhcp_lease_get_domainname(lease, domainname);
+if (r  0)
+return r;
+
+l-dhcp_domainname = strdup(domainname);
+if (!l-dhcp_domainname)
+return -ENOMEM;
+
+return 0;
+}
+
 int link_update_monitor(Link *l) {
 assert(l);
 
 link_update_dhcp_dns_servers(l);
 link_update_link_dns_servers(l);
+link_update_dhcp_domainname(l);
 link_allocate_scopes(l);
 
 return 0;
diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h
index bd32a70..8ea3acd 100644
--- a/src/resolve/resolved-link.h
+++ b/src/resolve/resolved-link.h
@@ -65,6 +65,8 @@ struct Link {
 
 RateLimit mdns_ratelimit;
 RateLimit llmnr_ratelimit;
+
+char *dhcp_domainname;
 };
 
 int link_new(Manager *m, Link **ret, int ifindex);
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index 9672843..09f7695 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -497,6 +497,7 @@ int manager_write_resolv_conf(Manager *m) {
 const char *path = /run/systemd/resolve/resolv.conf;
 _cleanup_free_ char *temp_path = NULL;
 _cleanup_fclose_ FILE *f = NULL;
+const char *domainname = NULL;
 unsigned count = 0;
 DnsServer *s;
 Iterator i;
@@ -523,11 +524,17 @@ int manager_write_resolv_conf(Manager *m) {
 
 LIST_FOREACH(servers, s, l-dhcp_dns_servers)
 write_resolve_conf_server(s, f, count);
+
+if (!domainname  l-dhcp_domainname)
+domainname = l-dhcp_domainname;
 }
 
 LIST_FOREACH(servers, s, m-dns_servers)
 write_resolve_conf_server(s, f, count);
 
+if (domainname)
+fprintf(f, domain %s\n, domainname);
+
 r = fflush_and_check(f);
 if (r  0)
 goto fail;
diff --git a/src/systemd/sd-network.h b/src/systemd/sd-network.h
index e454705..826cec7 100644
--- a/src/systemd/sd-network.h
+++ b/src/systemd/sd-network.h
@@ -79,6 +79,9 @@ int sd_network_get_dhcp_lease(int ifindex, sd_dhcp_lease 
**ret);
 /* Returns true if link is configured to respect DNS entries received by DHCP 
*/
 int sd_network_dhcp_use_dns(int ifindex);
 
+/* Returns true if link is configured to use the domain name received by DHCP 
*/
+int