On Thu, 2013-05-23 at 15:06 -0400, Simo Sorce wrote:
> On Thu, 2013-05-23 at 21:02 +0300, Alexander Bokovoy wrote:
> > On Thu, 23 May 2013, Simo Sorce wrote:
> > >On Thu, 2013-05-23 at 10:42 -0400, Simo Sorce wrote:
> > >> CLDAP fixes for:
> > >> https://fedorahosted.org/freeipa/ticket/3639
> > >>
> > >> Should be pretty straightforward.
> > >> (pending testing)
> > >>
> > >> Alexander,
> > >> please check they work for your 2012 setup too.
> > >
> > >Alexander found a couple of typos and then the patches didn't work for
> > >him.
> > >
> > >The bug was that I forgot to consider the successful case in the switch
> > >statement I introduced at the last minute ... silly me.
> > >
> > >Tested this new set and works for me, Alexander please confirm.
> > Works for me now. There is still slight difference from what we see
> > against Windows Server 2012.
> > 
> > ----------------------------------------------------------------------------------
> > $ ldapsearch -LL -H cldap://red.bird.clone -b "" -s base 
> > '(&(NtVer=\06\00\00\00)(AAC=\00\00\00\00))' netlogon                        
> >              
> > version: 1
> > 
> > dn:
> > netlogon::
> > FwAAAP0DAADBEtlp7qtnRa3yDLzj68BuBGJpcmQFY2xvbmUAwBgDcmVkwBgEQklSRAA
> >   
> > FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACAAAAfwAAAQAAAAAAAAAAAAUAAAD/
> >   ////
> > 
> > $ ldapsearch -LL -H cldap://red.bird.clone -b "" -s base 
> > '(&(NtVer=\06\00\00\00)(AAC=\00\00\00\00)(DnsDOmain=bird.clone))' netlogon
> > version: 1
> > 
> > dn:
> > netlogon::
> > FwAAAP0DAADBEtlp7qtnRa3yDLzj68BuBGJpcmQFY2xvbmUAwBgDcmVkwBgEQklSRAA
> >   
> > FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACAAAAfwAAAQAAAAAAAAAAAAUAAAD/
> >   ////
> > 
> > $ ldapsearch -LL -H cldap://red.bird.clone -b "" -s base 
> > '(&(NtVer=\06\00\00\00)(AAC=\00\00\00\00)(DnsDOmain=bird.clone1))' netlogon
> > version: 1
> > 
> > dn:
> > netlogon:
> > 
> > $ ldapsearch -LL -H cldap://red.bird.clone -b "" -s base 
> > '(&(NtVer=\00\00\55\00)(AAC=\00\00\00\00)(DnsDOmain=bird.clone))' netlogon
> > version: 1
> > 
> > dn:
> > netlogon:
> > ----------------------------------------------------------------------------------
> > 
> > As you can see, incorrect parameters still return empty dn and netlogon
> > attributes while Windows Server 2012 returns empty response:
> > 
> > $ ldapsearch  -LL -H cldap://altai.ad.lan -b "" -s base 
> > '(&(NtVer=\00\00\00\55\00)(AAC=\00\00\00\00))' netlogon
> > version: 1
> > 
> > Yet, since for trusts we care about explicit request with our domain name 
> > _and_ the
> > case when DnsDomain is not specified, everything continues to work.
> > 
> > So ACK.
> 
> I can easily avoid returning the empty netlogon field, which is what I
> wanted to do.
> I'll see if I can also avoid returning the DN.
> 
> Let me try just one more revision.

It was a simple fix, attached patches omit LDAP_RES_SERAHC_ENTRY
completely as they were supposed to, and only return a
LDAP_RES_SEARCH_RESULT record.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
>From fd1fb069a36e2810dc45751ab452d7c5406f3e6c Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Thu, 23 May 2013 10:06:22 -0400
Subject: [PATCH 2/2] CLDAP: Return empty reply on non-fatal errors

Windows DCs return an empty reply when a legal request cannot satisfied.
If we get EINVAL or ENOENT it means the information requested could not be
found or input parameters were bogus.
Always return an empty reply in these cases.

On any other internal error just return, the request may have been legit but we
can't really handle it right now, pretend we never saw it and hope the next
attempt will succeed.

Fixes: https://fedorahosted.org/freeipa/ticket/3639

Signed-off-by: Simo Sorce <s...@redhat.com>
---
 .../ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c | 24 ++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c
index 307110c123c2d898c910371da9ebeb2edfa0f1b5..468b92bbacd98d6c786dec0aed622489e9a8af23 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c
@@ -218,12 +218,14 @@ static void ipa_cldap_respond(struct ipa_cldap_ctx *ctx,
         return;
     }
 
-    /* result */
-    ret = ber_printf(be, "{it{s{{s[O]}}}}", req->id,
+    if (nbtblob->bv_len != 0) {
+        /* result */
+        ret = ber_printf(be, "{it{s{{s[O]}}}}", req->id,
                          LDAP_RES_SEARCH_ENTRY, "", "netlogon", nbtblob);
-    if (ret == LBER_ERROR) {
-        LOG("Failed to encode CLDAP reply\n");
-        goto done;
+        if (ret == LBER_ERROR) {
+            LOG("Failed to encode CLDAP reply\n");
+            goto done;
+        }
     }
     /* done */
     ret = ber_printf(be, "{it{ess}}", req->id,
@@ -264,7 +266,17 @@ static void ipa_cldap_process(struct ipa_cldap_ctx *ctx,
     LOG_TRACE("CLDAP Request received");
 
     ret = ipa_cldap_netlogon(ctx, req, &reply);
-    if (ret) {
+    switch (ret) {
+    case 0:
+        /* all fine */
+        break;
+    case EINVAL:
+    case ENOENT:
+        /* bad request, return empty reply as windows does */
+        memset(&reply, 0, sizeof(struct berval));
+        break;
+    default:
+        /* internal error, just get out */
         goto done;
     }
 
-- 
1.8.1.4

>From 47256da0944c1c346cbae9b8c7c8a13cb210844d Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@redhat.com>
Date: Thu, 23 May 2013 10:04:11 -0400
Subject: [PATCH 1/2] CLDAP: Fix domain handling in netlogon requests

1. Stop using getdomainname() as it is often not properly initialized
2. The code using getdomainname() was not working anyway it was trying to
look at the function call output in hostname which is always empty at that
point.
3. Always check the requested domain matches our own, we cannot reply to
anything else anyway.

Pre-requisite to fix: https://fedorahosted.org/freeipa/ticket/3639

Signed-off-by: Simo Sorce <s...@redhat.com>
---
 .../ipa-cldap/ipa_cldap_netlogon.c                 | 83 ++++++++++++----------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
index 180a067ff8d95f984bd91233f5fb5811c9e140b5..dda933d6d4df4f95c9b70f1bd62c329c788c3a6f 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_netlogon.c
@@ -215,14 +215,14 @@ int ipa_cldap_netlogon(struct ipa_cldap_ctx *ctx,
                        struct berval *reply)
 {
     char hostname[MAXHOSTNAMELEN + 1]; /* NOTE: lenght hardcoded in kernel */
-    char domname[MAXHOSTNAMELEN + 1]; /* NOTE: lenght hardcoded in kernel */
+    char *host = NULL;
     char *domain = NULL;
     char *guid = NULL;
     char *sid = NULL;
     char *name = NULL;
     uint32_t ntver = 0;
     uint32_t t;
-    char *p;
+    char *dot;
     int ret;
     int len;
     int i;
@@ -295,52 +295,63 @@ int ipa_cldap_netlogon(struct ipa_cldap_ctx *ctx,
         goto done;
     }
 
-    /* If no domain is provide the client is asking for our own domain,
-     * read our own domain name from the system */
-    if (!domain) {
-        ret = getdomainname(domname, MAXHOSTNAMELEN);
-        if (ret == -1) {
-            ret = errno;
-            goto done;
-        }
-        domname[MAXHOSTNAMELEN] = '\0';
-        p = strchr(hostname, '.');
-        if (p) {
-            domain = strdup(p + 1);
-            if (!domain) {
-                ret = ENOMEM;
-                goto done;
-            }
-        }
-    }
-
-    /* FIXME: we support only NETLOGON_NT_VERSION_5EX for now */
-    if (!(ntver & NETLOGON_NT_VERSION_5EX)) {
-        ret = EINVAL;
-        goto done;
-    }
-
-    ret = ipa_cldap_get_domain_entry(ctx, domain, &guid, &sid, &name);
-    if (ret) {
-        goto done;
-    }
-
+    /* TODO: get our own domain at plugin initialization, and avoid
+     * gethostname() */
     ret = gethostname(hostname, MAXHOSTNAMELEN);
     if (ret == -1) {
         ret = errno;
         goto done;
     }
+    /* Make double sure it is terminated */
     hostname[MAXHOSTNAMELEN] = '\0';
-    p = strchr(hostname, '.');
-    if (p) {
-        *p = '\0';
+    dot = strchr(hostname, '.');
+    if (!dot) {
+        /* this name is not fully qualified, therefore invalid */
+        ret = EINVAL;
+        goto done;
     }
+    *dot = '\0';
 
-    ret = ipa_cldap_encode_netlogon(hostname, domain,
+    /* this is the unqualified host name */
+    host = strdup(hostname);
+    if (!host) {
+        ret = ENOMEM;
+        goto done;
+    }
+
+    /* If a domain is provided, check it is our own.
+     * If no domain is provided the client is asking for our own domain. */
+    if (domain) {
+        ret = strcasecmp(domain, dot + 1);
+        if (ret != 0) {
+            ret = EINVAL;
+            goto done;
+        }
+    } else {
+        domain = strdup(dot + 1);
+        if (!domain) {
+            ret = ENOMEM;
+            goto done;
+        }
+    }
+
+    /* FIXME: we support only NETLOGON_NT_VERSION_5EX for now */
+    if (!(ntver & NETLOGON_NT_VERSION_5EX)) {
+        ret = EINVAL;
+        goto done;
+    }
+
+    ret = ipa_cldap_get_domain_entry(ctx, domain, &guid, &sid, &name);
+    if (ret) {
+        goto done;
+    }
+
+    ret = ipa_cldap_encode_netlogon(host, domain,
                                     guid, sid, name,
                                     ntver, reply);
 
 done:
+    free(host);
     free(domain);
     free(guid);
     free(sid);
-- 
1.8.1.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to