On Tue, Jul 05, 2016 at 11:40:48PM +0200, Jakub Hrozek wrote:
> On Tue, Jul 05, 2016 at 09:02:24PM +0200, Lukas Slebodnik wrote:
> > On (05/07/16 12:27), Jakub Hrozek wrote:
> > >On Wed, Jun 29, 2016 at 05:38:55PM +0200, Lukas Slebodnik wrote:
> > >> On (29/06/16 13:12), Jakub Hrozek wrote:
> > >> >On Wed, Jun 29, 2016 at 12:48:36PM +0200, Lukas Slebodnik wrote:
> > >> >> On (28/06/16 11:50), Jakub Hrozek wrote:
> > >> >> >From b493cee9976b8dd62bea3d8f09b88ce809a40980 Mon Sep 17
00:00:00 2001
> > >> >> >From: Jakub Hrozek <jhrozek(a)redhat.com>
> > >> >> >Date: Thu, 19 Nov 2015 10:40:39 +0100
> > >> >> >Subject: [PATCH] LDAP: Change the default rfc2307 autofs
attribute mappings
> > >> >> >
> > >> >> >Resolves:
> > >> >> >
https://fedorahosted.org/sssd/ticket/2858
> > >> >> >
> > >> >> >The default attribute mappings we used to have:
> > >> >> > ldap_autofs_map_object_class automountMap
> > >> >> > ldap_autofs_map_name ou
> > >> >> > ldap_autofs_entry_object_class automount
> > >> >> > ldap_autofs_entry_key cn
> > >> >> > ldap_autofs_entry_value
automountInformation
> > >> >> >
> > >> >> >Was wrong. Instead, this patch switches to:
> > >> >> > ldap_autofs_map_object_class nisMap
> > >> >> > ldap_autofs_map_name nisMapName
> > >> >> > ldap_autofs_entry_object_class nisObject
> > >> >> > ldap_autofs_entry_key cn
> > >> >> > ldap_autofs_entry_value nisMapEntry
> > >> >> >
> > >> >> >Which are attributes that are available with servers
running the default
> > >> >> >rfc2307 schema. In addition, this patch adds a syslog and
DEBUG message
> > >> >> >that warns administrators to double-check their
configuration.
> > >> >> >
> > >> >> >We don't warn when the autofs provider is set to AD,
because that one
> > >> >> >is already correct.
> > >> >> >---
> > >> >> > src/man/sssd-ldap.5.xml | 17 ++++----
> > >> >> > src/providers/ldap/ldap_common.h | 6 +++
> > >> >> > src/providers/ldap/ldap_options.c | 83
++++++++++++++++++++++++++++++++++++++-
> > >> >> > src/providers/ldap/ldap_opts.c | 8 ++--
> > >> >> > src/providers/ldap/sdap_autofs.c | 17 ++++++++
> > >> >> > 5 files changed, 119 insertions(+), 12 deletions(-)
> > >> >> >
> > >> >> >diff --git a/src/providers/ldap/ldap_opts.c
b/src/providers/ldap/ldap_opts.c
> > >> >> >index
ff9bf0d8b6d4a8f677e08219e5105e3750b7a4a8..524579d4fcd478f20678bebf2c3ce18f61ed0cb9 100644
> > >> >> >--- a/src/providers/ldap/ldap_opts.c
> > >> >> >+++ b/src/providers/ldap/ldap_opts.c
> > >> >> >@@ -349,15 +349,15 @@ struct sdap_attr_map service_map[] =
{
> > >> >> > };
> > >> >> >
> > >> >> > struct sdap_attr_map rfc2307_autofs_mobject_map[] = {
> > >> >> >- { "ldap_autofs_map_object_class",
"automountMap", SYSDB_AUTOFS_MAP_OC, NULL },
> > >> >> >- { "ldap_autofs_map_name", "ou",
SYSDB_AUTOFS_MAP_NAME, NULL },
> > >> >> >+ { "ldap_autofs_map_object_class",
"nisMap", SYSDB_AUTOFS_MAP_OC, NULL },
> > >> >> >+ { "ldap_autofs_map_name",
"nisMapName", SYSDB_AUTOFS_MAP_NAME, NULL },
> > >> >> > SDAP_ATTR_MAP_TERMINATOR
> > >> >> > };
> > >> >> >
> > >> >> > struct sdap_attr_map rfc2307_autofs_entry_map[] = {
> > >> >> >- { "ldap_autofs_entry_object_class",
"automount", SYSDB_AUTOFS_ENTRY_OC, NULL },
> > >> >> >+ { "ldap_autofs_entry_object_class",
"nisObject", SYSDB_AUTOFS_ENTRY_OC, NULL },
> > >> >> > { "ldap_autofs_entry_key", "cn",
SYSDB_AUTOFS_ENTRY_KEY, NULL },
> > >> >> >- { "ldap_autofs_entry_value",
"automountInformation", SYSDB_AUTOFS_ENTRY_VALUE, NULL },
> > >> >> >+ { "ldap_autofs_entry_value",
"nisMapEntry", SYSDB_AUTOFS_ENTRY_VALUE, NULL },
> > >> >> > SDAP_ATTR_MAP_TERMINATOR
> > >> >> > };
> > >> >> >
> > >> >> >diff --git a/src/providers/ldap/sdap_autofs.c
b/src/providers/ldap/sdap_autofs.c
> > >> >> >index
c02c04d5ca5addbfd1552176cac5f74fdd592503..db41b650ddcda99e6c221e856c259fcc43a10436 100644
> > >> >> >--- a/src/providers/ldap/sdap_autofs.c
> > >> >> >+++ b/src/providers/ldap/sdap_autofs.c
> > >> >> >@@ -313,6 +313,23 @@ errno_t sdap_autofs_init(TALLOC_CTX
*mem_ctx,
> > >> >> > return ret;
> > >> >> > }
> > >> >> >
> > >> >> >+ if (id_ctx->opts->schema_type ==
SDAP_SCHEMA_AD) {
> > >> >> >+ if
(ldap_ad_autofs_schema_defaults(be_ctx->cdb,
> > >> >> >+
be_ctx->conf_path)) {
> > >> >> >+ DEBUG(SSSDBG_IMPORTANT_INFO,
> > >> >> >+ "Your configuration uses the ldap
autofs provider "
> > >> >> >+ "with schema set to \"ad\"
and default autofs attribute "
> > >> >> >+ "mappings. The default map changed in
this release, "
> > >> >> >+ "please make sure the sssd
configuration explicitly matches "
> > >> >> >+ "the server attributes.");
> > >> >> >+ sss_log(SSS_LOG_NOTICE,
> > >> >> >+ _("Your configuration uses the ldap
autofs provider "
> > >> >> >+ "with schema set to
\"ad\" and default autofs attribute "
> > >> >> >+ "mappings. The default map changed
in this release, "
> > >> >> >+ "please make sure the sssd
configuration explicitly matches "
> > >> >> >+ "the server attributes."));
> > >> >> Do we really need to log message for ad schema?
> > >> >> I thought we will log message about change just for rfc2307.
> > >> >
> > >> >I tried to catch this scenario:
> > >> > id_provider=ldap
> > >> > autofs_provider=ldap
> > >> > ldap_schema=ad
> > >> >
> > >> >With the previous versions, the attributes pointed to automountMap
etc.
> > >> >And there is a chance (unlikely) the admin extended the schema to
keep
> > >> >the config using defaults.
> > >> >
> > >> >>
> > >> >> IIRC AD does not have by default schema for autofs.
> > >> >
> > >> >Right, that's the reason we should to change the defaults. AD
only has
> > >> >the most basic rfc2307 and our previous defaults forced the AD
SSSD
> > >> >admins to either change the mappings in the config file or extend
the
> > >> >schema.
> > >>
> > >> The patch should fix ticket #2858;
> > >> and this ticket says: autofs_provider = ldap ldap_schema = rfc2307
> > >> and the 4th comment says: The beauty of using older schema (rfc2307) to
store
> > >> automount information is, that we do not need to extend AD schema -
which
> > >> is in many cases quite troublesome action.
> > >>
> > >> So the warning should be logged for rfc2307 and not for AD.
> > >> BTW it looks like we needn't change defaults for AD.
> > >
> > >This only works because we have:
> > > 977 /* fix schema to AD */
> > > 978 id_opts->schema_type = SDAP_SCHEMA_AD;
> > >in ad_common.c, otherwise setting ldap_schema=rfc2307 would start looking
> > >up usernames in memberUid and other rfc2307 stuff. If the reporter was
> > >using id_provider=ldap with schema set to ad, then nothing would work
> > >and also, we have a bit of schizophrenic situation now in the code where
> > >autofs_provider=ad uses a different set of options than ldap_schema=ad.
> > >
> > Fair enough and Ian confirmed that we should rather use nis* attributes
> > for AD. If you want we can change default for id_provider=ldap with schema set
> > to AD. It's up to you and it can be done later. If you do not want to change
it
> > now.
>
> I think I would prefer another patch in the next release (or even
> deferred). We have a recommended configuration that works out of the box
> and I admit I haven't done a lot of testing with ldap provider either.
>
> >
> > >I'm OK with saying that the preferred way is to use the
autofs_provider=ad,
> > >but IMO we should at least track this with a different ticket.
> > >
> > >A new patch is attached that just changes the rfc2307 schema, because I
> > >would like to include it in the next version, but let's also discuss
> > >what do we do with the AD LDAP schema.
> >
> >
> > >From 5765a2c5bf63e3caa33ae73c1e5342869c095758 Mon Sep 17 00:00:00 2001
> > >From: Jakub Hrozek <jhrozek(a)redhat.com>
> > >Date: Thu, 19 Nov 2015 10:40:39 +0100
> > >Subject: [PATCH] LDAP: Change the default rfc2307 autofs attribute mappings
> > >
> > >Resolves:
> > >
https://fedorahosted.org/sssd/ticket/2858
> > >
> > >The default attribute mappings we used to have:
> > > ldap_autofs_map_object_class automountMap
> > > ldap_autofs_map_name ou
> > > ldap_autofs_entry_object_class automount
> > > ldap_autofs_entry_key cn
> > > ldap_autofs_entry_value automountInformation
> > >
> > >Was wrong. Instead, this patch switches to:
> > > ldap_autofs_map_object_class nisMap
> > > ldap_autofs_map_name nisMapName
> > > ldap_autofs_entry_object_class nisObject
> > > ldap_autofs_entry_key cn
> > > ldap_autofs_entry_value nisMapEntry
> > >
> > >Which are attributes that are available with servers running the default
> > >rfc2307 schema. In addition, this patch adds a syslog and DEBUG message
> > >that warns administrators to double-check their configuration.
> > >
> > >We don't warn when the autofs provider is set to AD, because that one
> > >is already correct.
> > >---
> > > src/man/sssd-ldap.5.xml | 22 ++++++++-----
> > > src/providers/ldap/ldap_options.c | 66
+++++++++++++++++++++++++++++++++++++++
> > > src/providers/ldap/ldap_opts.c | 8 ++---
> > > 3 files changed, 85 insertions(+), 11 deletions(-)
> > >
> > >diff --git a/src/man/sssd-ldap.5.xml b/src/man/sssd-ldap.5.xml
> > >index
a30100408c6e77f9156878cb6ff63dfbf7b041d1..48f30a98ebb714bedfd5b595e3115151432810e6 100644
> > >--- a/src/man/sssd-ldap.5.xml
> > >+++ b/src/man/sssd-ldap.5.xml
> > >@@ -2505,7 +2505,9 @@ ldap_access_filter = (employeeType=admin)
> > > The object class of an automount map entry in
LDAP.
> > > </para>
> > > <para>
> > >- Default: automountMap
> > >+ Default: nisMap (rfc2307, autofs_provider=ad),
> > >+ automountMap (rfc2307bis, ipa, LDAP provider
> > >+ with AD schema)
> > Would it be easier to read following ?
> >
> > Default: nisMap (rfc2307, autofs_provider=ad), automountMap otherwise
> > or use other way. You needn't change if you think this way is better.
>
> yes, this is better.
>
> >
> > >@@ -438,6 +490,20 @@ int ldap_get_autofs_options(TALLOC_CTX *memctx,
> > > "connecting to the LDAP server.\n");
> > > }
> > >
> > >+ if (opts->schema_type == SDAP_SCHEMA_RFC2307 &&
> > >+ ldap_rfc2307_autofs_defaults(cdb, conf_path) == true) {
> > >+ DEBUG(SSSDBG_IMPORTANT_INFO,
> > >+ "Your configuration uses the autofs provider "
> > >+ "with schema set to rfc2307 and default attribute
mappings. "
> > >+ "The default map has changed in this release, please
make "
> > >+ "sure the configuration matches the server
attributes.");
> > Thank you for message with schema rfc2307.
> > But there is missing new line at the end :-)
>
> I will change this.
A new patch is attached.
From ef5693b03bdaad71527014ca87fc2192061689d5 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Thu, 19 Nov 2015 10:40:39 +0100
Subject: [PATCH] LDAP: Change the default rfc2307 autofs attribute mappings
Resolves:
https://fedorahosted.org/sssd/ticket/2858
The default attribute mappings we used to have:
ldap_autofs_map_object_class automountMap
ldap_autofs_map_name ou
ldap_autofs_entry_object_class automount
ldap_autofs_entry_key cn
ldap_autofs_entry_value automountInformation
Was wrong. Instead, this patch switches to:
ldap_autofs_map_object_class nisMap
ldap_autofs_map_name nisMapName
ldap_autofs_entry_object_class nisObject
ldap_autofs_entry_key cn
ldap_autofs_entry_value nisMapEntry
Which are attributes that are available with servers running the default
rfc2307 schema. In addition, this patch adds a syslog and DEBUG message
that warns administrators to double-check their configuration.
We don't warn when the autofs provider is set to AD, because that one
is already correct.
---
src/man/sssd-ldap.5.xml | 17 +++++-----
src/providers/ldap/ldap_options.c | 66 +++++++++++++++++++++++++++++++++++++++
src/providers/ldap/ldap_opts.c | 8 ++---
3 files changed, 80 insertions(+), 11 deletions(-)