[PATCH] Add MDNS configuration option

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
Good evening,
    Attached is a patch that makes the MDNS name configurable.
Right now it only affects Avahi and supports using the default
of the NETBIOS name or deferring the name value to the Avahi
server. This has the effect of lower casing the hostname
on SMB registered Avahi services automatically.

I would appreciate a review on it.

-Kevin Anderson

0001-Add-mdns-name-configuration-option.patch (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
Good morning,

On Sun, Nov 26, 2017 at 5:42 PM, Kevin Anderson <[hidden email]> wrote:

>
> Good evening,
>     Attached is a patch that makes the MDNS name configurable.
> Right now it only affects Avahi and supports using the default
> of the NETBIOS name or deferring the name value to the Avahi
> server. This has the effect of lower casing the hostname
> on SMB registered Avahi services automatically.
>
> I would appreciate a review on it.
>
> -Kevin Anderson
Just a friendly ping for a review on this. :)

Thanks,
Kevin

0001-Add-mdns-name-configuration-option.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Mon, 2017-12-04 at 06:16 -0500, Kevin Anderson via samba-technical
wrote:

> Good morning,
>
> On Sun, Nov 26, 2017 at 5:42 PM, Kevin Anderson <[hidden email]> wrote:
> >
> > Good evening,
> >     Attached is a patch that makes the MDNS name configurable.
> > Right now it only affects Avahi and supports using the default
> > of the NETBIOS name or deferring the name value to the Avahi
> > server. This has the effect of lower casing the hostname
> > on SMB registered Avahi services automatically.
> >
> > I would appreciate a review on it.
> >
> > -Kevin Anderson
>
> Just a friendly ping for a review on this. :)

Can you give more of a rationale for why this is needed?

Also, have you (if required) sent in your DCO the the contributing
address?  (Not currently required if this is an own-time activity).

Thanks,

Andrew Bartlett

--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Tue, Dec 05, 2017 at 11:30:02PM +1300, Andrew Bartlett via samba-technical wrote:
> Can you give more of a rationale for why this is needed?

<https://lists.samba.org/archive/samba-technical/2017-October/123231.html>
<https://lists.samba.org/archive/samba-technical/2017-November/123667.html>

> Also, have you (if required) sent in your DCO the the contributing
> address?  (Not currently required if this is an own-time activity).

Iirc this is not required for individual contributions:

<https://www.samba.org/samba/devel/copyright-policy.html>

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
Good morning,

>> Can you give more of a rationale for why this is needed?
>
> <https://lists.samba.org/archive/samba-technical/2017-October/123231.html>
> <https://lists.samba.org/archive/samba-technical/2017-November/123667.html>

I can provide additional clarification if required but those two
threads sum things up in my opinion.

>> Also, have you (if required) sent in your DCO the the contributing
>> address?  (Not currently required if this is an own-time activity).
>
> Iirc this is not required for individual contributions:
>
> <https://www.samba.org/samba/devel/copyright-policy.html>
>

In addition to what Ralph stated this is a contribution on my personal time.

Thanks,
Kevin Anderson

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Tue, 2017-12-05 at 07:57 -0500, Kevin Anderson wrote:
> Good morning,
>
> > > Can you give more of a rationale for why this is needed?
> >
> > <https://lists.samba.org/archive/samba-technical/2017-October/123231.html>
> > <https://lists.samba.org/archive/samba-technical/2017-November/123667.html>
>
> I can provide additional clarification if required but those two
> threads sum things up in my opinion.

Thanks.  Can you just put that text into the commit?  It makes it
easier for us to remember the details later.  Also please update the
WHATSNEW.txt so the release announcement will include it.

Otherwise, I'm happy with the patch, so fix that up and hopefully Ralph
and I can review it into master.

Thanks!

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
Hi Andrew,

> > > > Can you give more of a rationale for why this is needed?
> > >
> > > <https://lists.samba.org/archive/samba-technical/2017-October/123231.html>
> > > <https://lists.samba.org/archive/samba-technical/2017-November/123667.html>
> >
> > I can provide additional clarification if required but those two
> > threads sum things up in my opinion.
>
> Thanks.  Can you just put that text into the commit?  It makes it
> easier for us to remember the details later.  Also please update the
> WHATSNEW.txt so the release announcement will include it.
I've updated WHATSNEW for both the addition of Time Machine support as well
as the MDNS change. I felt like adding the Time Machine note was appropriate
but if you would like me to remove it just let me know. I've also updated the
commit to include the reason behind the change as well. I tried to keep it
brief but if you'd like to see more information I can update that as well.

Thanks,
Kevin Anderson


0001-Add-mdns-name-configuration-option.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Tue, 2017-12-05 at 17:26 -0500, Kevin Anderson wrote:

> Hi Andrew,
>
> > > > > Can you give more of a rationale for why this is needed?
> > > >
> > > > <https://lists.samba.org/archive/samba-technical/2017-October/123231.html>
> > > > <https://lists.samba.org/archive/samba-technical/2017-November/123667.html>
> > >
> > > I can provide additional clarification if required but those two
> > > threads sum things up in my opinion.
> >
> > Thanks.  Can you just put that text into the commit?  It makes it
> > easier for us to remember the details later.  Also please update the
> > WHATSNEW.txt so the release announcement will include it.
>
> I've updated WHATSNEW for both the addition of Time Machine support as well
> as the MDNS change. I felt like adding the Time Machine note was appropriate
> but if you would like me to remove it just let me know. I've also updated the
> commit to include the reason behind the change as well. I tried to keep it
> brief but if you'd like to see more information I can update that as well.

Reviewed-by: Andrew Bartlett <[hidden email]>

Can I get a second team reviewer please?

Thanks,

Andrew Bartlett

--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba





Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Tue, Dec 05, 2017 at 05:26:18PM -0500, Kevin Anderson wrote:

> Hi Andrew,
>
> > > > > Can you give more of a rationale for why this is needed?
> > > >
> > > > <https://lists.samba.org/archive/samba-technical/2017-October/123231.html>
> > > > <https://lists.samba.org/archive/samba-technical/2017-November/123667.html>
> > >
> > > I can provide additional clarification if required but those two
> > > threads sum things up in my opinion.
> >
> > Thanks.  Can you just put that text into the commit?  It makes it
> > easier for us to remember the details later.  Also please update the
> > WHATSNEW.txt so the release announcement will include it.
>
> I've updated WHATSNEW for both the addition of Time Machine support as well
> as the MDNS change. I felt like adding the Time Machine note was appropriate
> but if you would like me to remove it just let me know. I've also updated the
> commit to include the reason behind the change as well. I tried to keep it
> brief but if you'd like to see more information I can update that as well.

doesn't apply to master anymore. Can you please check and post an updated patch?
Thanks!

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
Hi Ralph,
 
> doesn't apply to master anymore. Can you please check and post an updated patch?
> Thanks!

I've rebased the patch against current master.

Thanks,
Kevin Anderson

0001-Add-mdns-name-configuration-option.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Tue, 2017-12-05 at 17:51 -0500, Kevin Anderson wrote:
> Hi Ralph,
>  
> > doesn't apply to master anymore. Can you please check and post an updated patch?
> > Thanks!
>
> I've rebased the patch against current master.
>
> Thanks,
> Kevin Anderson

Reviewed-by: Andrew Bartlett <[hidden email]>

Ralph, or others: Can I get a second team reviewer please?

Thanks,

Andrew Bartlett
--
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT  
https://catalyst.net.nz/services/samba





Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Wed, Dec 06, 2017 at 01:42:07PM +1300, Andrew Bartlett wrote:

> On Tue, 2017-12-05 at 17:51 -0500, Kevin Anderson wrote:
> > Hi Ralph,
> >  
> > > doesn't apply to master anymore. Can you please check and post an updated patch?
> > > Thanks!
> >
> > I've rebased the patch against current master.
> >
> > Thanks,
> > Kevin Anderson
>
> Reviewed-by: Andrew Bartlett <[hidden email]>
>
> Ralph, or others: Can I get a second team reviewer please?

pushed.

Thanks Kevin!

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Wed, Dec 06, 2017 at 09:48:50AM +0100, Ralph Böhme wrote:

> On Wed, Dec 06, 2017 at 01:42:07PM +1300, Andrew Bartlett wrote:
> > On Tue, 2017-12-05 at 17:51 -0500, Kevin Anderson wrote:
> > > Hi Ralph,
> > >  
> > > > doesn't apply to master anymore. Can you please check and post an updated patch?
> > > > Thanks!
> > >
> > > I've rebased the patch against current master.
> > >
> > > Thanks,
> > > Kevin Anderson
> >
> > Reviewed-by: Andrew Bartlett <[hidden email]>
> >
> > Ralph, or others: Can I get a second team reviewer please?
>
> pushed.
failed with

[1891/3176] Compiling source3/smbd/avahi_register.c
../source3/smbd/avahi_register.c: In function ‘avahi_client_callback’:
../source3/smbd/avahi_register.c:123:3: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
   AvahiStringList *adisk = NULL;
   ^
cc1: all warnings being treated as errors

Updated patch attached.

Please review & push if happy.

-slow

--
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

mdns.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add MDNS configuration option

Samba - samba-technical mailing list
On Wed, Dec 06, 2017 at 10:36:52AM +0100, Ralph Böhme via samba-technical wrote:

> On Wed, Dec 06, 2017 at 09:48:50AM +0100, Ralph Böhme wrote:
> > On Wed, Dec 06, 2017 at 01:42:07PM +1300, Andrew Bartlett wrote:
> > > On Tue, 2017-12-05 at 17:51 -0500, Kevin Anderson wrote:
> > > > Hi Ralph,
> > > >  
> > > > > doesn't apply to master anymore. Can you please check and post an updated patch?
> > > > > Thanks!
> > > >
> > > > I've rebased the patch against current master.
> > > >
> > > > Thanks,
> > > > Kevin Anderson
> > >
> > > Reviewed-by: Andrew Bartlett <[hidden email]>
> > >
> > > Ralph, or others: Can I get a second team reviewer please?
> >
> > pushed.
>
> failed with
>
> [1891/3176] Compiling source3/smbd/avahi_register.c
> ../source3/smbd/avahi_register.c: In function ‘avahi_client_callback’:
> ../source3/smbd/avahi_register.c:123:3: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
>    AvahiStringList *adisk = NULL;
>    ^
> cc1: all warnings being treated as errors
>
> Updated patch attached.
>
> Please review & push if happy.

LGTM. Pushed !

Jeremy.

> --
> Ralph Boehme, Samba Team       https://samba.org/
> Samba Developer, SerNet GmbH   https://sernet.de/en/samba/

> From 5980731c48e53c3072b05d5cb5ca952a7d43151c Mon Sep 17 00:00:00 2001
> From: Kevin Anderson <[hidden email]>
> Date: Sat, 25 Nov 2017 23:03:59 -0500
> Subject: [PATCH] Add mdns name configuration option
>
> Add the mdns name configuration variable to control the mdns hostname.
> The default is to use the NETBIOS name of the system to match previous
> versions which is typically the hostname in all capitals. A value of mdns
> can be provided to defer the hostname to the mdns library.
>
> With the recent patch to support time machine being merged this patch
> allows for a user to configure the server name that is advertised to
> be lower cased through Avahi advertisements.
>
> Signed-off-by: Kevin Anderson <[hidden email]>
> Reviewed-by: Ralph Boehme <[hidden email]>
> ---
>  WHATSNEW.txt                          | 26 +++++++++++++++++++++++++-
>  docs-xml/smbdotconf/base/mdnsname.xml | 19 +++++++++++++++++++
>  lib/param/loadparm.c                  |  2 ++
>  lib/param/loadparm.h                  |  3 +++
>  lib/param/param_table.c               |  6 ++++++
>  source3/smbd/avahi_register.c         | 18 ++++++++++++++++--
>  6 files changed, 71 insertions(+), 3 deletions(-)
>  create mode 100644 docs-xml/smbdotconf/base/mdnsname.xml
>
> diff --git a/WHATSNEW.txt b/WHATSNEW.txt
> index 8f5986ec55e..007e5d914b0 100644
> --- a/WHATSNEW.txt
> +++ b/WHATSNEW.txt
> @@ -29,6 +29,29 @@ policy. Can be applied automatically by setting
>  
>   'server services = +gpoupdate'.
>  
> +Time Machine Support with vfs_fruit
> +===================================
> +Samba can be configured as a Time Machine target for Apple Mac devices
> +through the vfs_fruit module. When enabling a share for Time Machine
> +support the relevant Avahi records to support discovery will be published
> +for installations that have been built against the Avahi client library.
> +
> +Shares can be designated as a Time Machine share with the following setting:
> +
> +  'fruit:time machine = yes'
> +
> +Support for lower casing the MDNS Name
> +======================================
> +Allows the server name that is advertised through MDNS to be set to the
> +hostname rather than the Samba NETBIOS name. This allows an administrator
> +to make Samba registered MDNS records match the case of the hostname
> +rather than being in all capitals.
> +
> +This can be set with the following settings:
> +
> +  'mdns name = mdns'
> +
> +
>  smb.conf changes
>  ================
>  
> @@ -38,7 +61,8 @@ smb.conf changes
>    gpo update command     New
>    oplock contention limit            Removed
>    prefork children     New     1
> -
> +  mdns name                          Added                   netbios
> +  fruit:time machine                 Added                   false
>  
>  NT4-style replication based net commands removed
>  ================================================
> diff --git a/docs-xml/smbdotconf/base/mdnsname.xml b/docs-xml/smbdotconf/base/mdnsname.xml
> new file mode 100644
> index 00000000000..fba90ff9b84
> --- /dev/null
> +++ b/docs-xml/smbdotconf/base/mdnsname.xml
> @@ -0,0 +1,19 @@
> +<samba:parameter name="mdns name"
> +                 type="enum"
> +                 context="G"
> +                 enumlist="enum_mdns_name_values"
> +                 xmlns:samba="http://www.samba.org/samba/DTD/samba-doc">
> +
> +<description>
> + <para>This parameter controls the name that multicast DNS
> + support advertises as its' hostname.</para>
> +
> + <para>The default is to use the NETBIOS name which is typically
> + the hostname in all capital letters. </para>
> +
> + <para>A setting of mdns will defer the hostname configuration
> + to the MDNS library that is used.</para>
> +
> +</description>
> +<value type="default">netbios</value>
> +</samba:parameter>
> diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
> index d788ffbe36f..73b7901d7f6 100644
> --- a/lib/param/loadparm.c
> +++ b/lib/param/loadparm.c
> @@ -2906,6 +2906,8 @@ struct loadparm_context *loadparm_init(TALLOC_CTX *mem_ctx)
>  
>   lpcfg_do_global_parameter(lp_ctx, "client ldap sasl wrapping", "sign");
>  
> + lpcfg_do_global_parameter(lp_ctx, "mdns name", "netbios");
> +
>   lpcfg_do_global_parameter(lp_ctx, "ldap server require strong auth", "yes");
>  
>   lpcfg_do_global_parameter(lp_ctx, "follow symlinks", "yes");
> diff --git a/lib/param/loadparm.h b/lib/param/loadparm.h
> index e3c82164ca4..b5d79b912c7 100644
> --- a/lib/param/loadparm.h
> +++ b/lib/param/loadparm.h
> @@ -223,6 +223,9 @@ enum ldap_server_require_strong_auth {
>  /* DNS update settings */
>  enum dns_update_settings {DNS_UPDATE_OFF, DNS_UPDATE_ON, DNS_UPDATE_SIGNED};
>  
> +/* MDNS name sources */
> +enum mdns_name_values {MDNS_NAME_NETBIOS, MDNS_NAME_MDNS};
> +
>  /* LDAP SSL options */
>  enum ldap_ssl_types {LDAP_SSL_OFF, LDAP_SSL_START_TLS};
>  
> diff --git a/lib/param/param_table.c b/lib/param/param_table.c
> index f9052304bda..f9d3b55adf2 100644
> --- a/lib/param/param_table.c
> +++ b/lib/param/param_table.c
> @@ -127,6 +127,12 @@ static const struct enum_list enum_smb_signing_vals[] = {
>   {-1, NULL}
>  };
>  
> +static const struct enum_list enum_mdns_name_values[] = {
> + {MDNS_NAME_NETBIOS, "netbios"},
> + {MDNS_NAME_MDNS, "mdns"},
> + {-1, NULL}
> +};
> +
>  static const struct enum_list enum_tls_verify_peer_vals[] = {
>   {TLS_VERIFY_PEER_NO_CHECK,
>   TLS_VERIFY_PEER_NO_CHECK_STRING},
> diff --git a/source3/smbd/avahi_register.c b/source3/smbd/avahi_register.c
> index 91e8a439b84..50462b5c610 100644
> --- a/source3/smbd/avahi_register.c
> +++ b/source3/smbd/avahi_register.c
> @@ -111,9 +111,23 @@ static void avahi_client_callback(AvahiClient *c, AvahiClientState status,
>   int dk = 0;
>   AvahiStringList *adisk = NULL;
>   AvahiStringList *adisk2 = NULL;
> + const char *hostname = NULL;
> + enum mdns_name_values mdns_name = lp_mdns_name();
>  
>   DBG_DEBUG("AVAHI_CLIENT_S_RUNNING\n");
>  
> + switch (mdns_name) {
> + case MDNS_NAME_MDNS:
> + hostname = avahi_client_get_host_name(c);
> + break;
> + case MDNS_NAME_NETBIOS:
> + hostname = lp_netbios_name();
> + break;
> + default:
> + DBG_ERR("Unhandled mdns_name %d\n", mdns_name);
> + return;
> + }
> +
>   state->entry_group = avahi_entry_group_new(
>   c, avahi_entry_group_callback, state);
>   if (state->entry_group == NULL) {
> @@ -125,7 +139,7 @@ static void avahi_client_callback(AvahiClient *c, AvahiClientState status,
>  
>   error = avahi_entry_group_add_service(
>      state->entry_group, AVAHI_IF_UNSPEC,
> -    AVAHI_PROTO_UNSPEC, 0, lp_netbios_name(),
> +    AVAHI_PROTO_UNSPEC, 0, hostname,
>      "_smb._tcp", NULL, NULL, state->port, NULL);
>   if (error != AVAHI_OK) {
>   DBG_DEBUG("avahi_entry_group_add_service failed: %s\n",
> @@ -169,7 +183,7 @@ static void avahi_client_callback(AvahiClient *c, AvahiClientState status,
>  
>   error = avahi_entry_group_add_service_strlst(
>      state->entry_group, AVAHI_IF_UNSPEC,
> -    AVAHI_PROTO_UNSPEC, 0, lp_netbios_name(),
> +    AVAHI_PROTO_UNSPEC, 0, hostname,
>      "_adisk._tcp", NULL, NULL, 0, adisk);
>   avahi_string_list_free(adisk);
>   adisk = NULL;
> --
> 2.13.6
>