Re: [Freeipa-devel] CLDAP Netlogon fixes
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
> >
> > FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACfwAAAQUAAAD/
> >
> >
> > $ 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
> >
> > FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACfwAAAQUAAAD/
> >
> >
> > $ 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
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
---
.../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)
Re: [Freeipa-devel] CLDAP Netlogon fixes
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 > > FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACfwAAAQUAAAD/ > > > $ 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 > > FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACfwAAAQUAAAD/ > > > $ 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. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] CLDAP Netlogon fixes
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 FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACfwAAAQUAAAD/ $ 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 FXFxSRUQAABdEZWZhdWx0LUZpcnN0LVNpdGUtTmFtZQDAOhACfwAAAQUAAAD/ $ 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. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] CLDAP Netlogon fixes
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.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From ab77389b030891f7759a185a49243e3c539c2631 Mon Sep 17 00:00:00 2001
From: Simo Sorce
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
---
daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
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..e79a87334616824b7cbac03e320960cbc869fe82 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c
@@ -264,7 +264,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
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
---
.../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;
}
[Freeipa-devel] [PATCH 0059] Make testcert automagically when needed by unit test
Hi,
With this patch, there's no need to run make-testcert separately
before running make-test. Unit test framework will check whether
service.crt file exists, and if not, will generate one if needed.
New location of service.crt file is in ~/.ipa directory.
Part of https://fedorahosted.org/freeipa/ticket/3621
Tomas
From dfaa28eaac37a30a6181a9ddf27de169f39eb06b Mon Sep 17 00:00:00 2001
From: Tomas Babej
Date: Thu, 23 May 2013 12:05:36 +0200
Subject: [PATCH] Make testcert automagically when needed by unit test
framework
With this patch, there's no need to run make-testcert separately
before running make-test. Unit test framework will check whether
service.crt file exists, and if not, will generate one if needed.
New location of service.crt file is in ~/.ipa directory.
Part of https://fedorahosted.org/freeipa/ticket/3621
---
tests/test_xmlrpc/test_host_plugin.py| 29 ++
tests/test_xmlrpc/test_service_plugin.py | 28 +++---
make-testcert => tests/testcert.py | 66
3 files changed, 68 insertions(+), 55 deletions(-)
rename make-testcert => tests/testcert.py (68%)
diff --git a/tests/test_xmlrpc/test_host_plugin.py b/tests/test_xmlrpc/test_host_plugin.py
index 07faf77607284b2193716854b287208f563d9472..7dba8b788c4ac68690e4a7cbfc9f21af1c53c181 100644
--- a/tests/test_xmlrpc/test_host_plugin.py
+++ b/tests/test_xmlrpc/test_host_plugin.py
@@ -34,6 +34,7 @@ from tests.test_xmlrpc.xmlrpc_test import (Declarative, XMLRPC_test,
fuzzy_hex)
from tests.test_xmlrpc import objectclasses
import base64
+from tests import testcert
fqdn1 = u'testhost1.%s' % api.env.domain
@@ -55,18 +56,32 @@ dn4 = DN(('fqdn',fqdn4),('cn','computers'),('cn','accounts'),
api.env.basedn)
invalidfqdn1 = u'foo_bar.lab.%s' % api.env.domain
-# We can use the same cert we generated for the service tests
-fd = open('tests/test_xmlrpc/service.crt', 'r')
-servercert = fd.readlines()
-servercert = ''.join(servercert)
-servercert = x509.strip_header(servercert)
-fd.close()
-
sshpubkey = u'ssh-rsa B3NzaC1yc2EDAQABAAABAQDGAX3xAeLeaJggwTqMjxNwa6XHBUAikXPGMzEpVrlLDCZtv00djsFTBi38PkgxBJVkgRWMrcBsr/35lq7P6w8KGIwA8GI48Z0qBS2NBMJ2u9WQ2hjLN6GdMlo77O0uJY3251p12pCVIS/bHRSq8kHO2No8g7KA9fGGcagPfQH+ee3t7HUkpbQkFTmbPPN++r3V8oVUk5LxbryB3UIIVzNmcSIn3JrXynlvui4MixvrtX6zx+O/bBo68o8/eZD26QrahVbA09fivrn/4h3TM019Eu/c2jOdckfU3cHUV/3Tno5d6JicibyaoDDK7S/yjdn5jhaz8MSEayQvFkZkiF0L public key test'
sshpubkeyfp = u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B public key test (ssh-rsa)'
+servercert = ''
+
+# Create the testing server cert if it does not already exist
+# Returns True if successful, error message if not
+if not os.path.exists(testcert.CERTPATH):
+servercert_ret = testcert.main()
+
+if os.path.exists(testcert.CERTPATH):
+fd = open(testcert.CERTPATH, 'r')
+servercert = fd.readlines()
+servercert = ''.join(servercert)
+servercert = x509.strip_header(servercert)
+fd.close()
+
+
class test_host(Declarative):
+def setUp(self):
+super(Declarative, self).setUp()
+if servercert == '':
+raise SkipTest('Testcert generation problem: %s' %
+ servercert_ret)
+
cleanup_commands = [
('host_del', [fqdn1], {}),
('host_del', [fqdn2], {}),
diff --git a/tests/test_xmlrpc/test_service_plugin.py b/tests/test_xmlrpc/test_service_plugin.py
index 6f8dbbee713405083d92d65f1add170661527bf9..b7cecf0f602711323be1a7ecbf23b2f7f757f29f 100644
--- a/tests/test_xmlrpc/test_service_plugin.py
+++ b/tests/test_xmlrpc/test_service_plugin.py
@@ -28,6 +28,9 @@ from tests.test_xmlrpc.xmlrpc_test import fuzzy_hex
from tests.test_xmlrpc import objectclasses
import base64
from ipapython.dn import DN
+from tests import testcert
+import nose
+import os.path
fqdn1 = u'testhost1.%s' % api.env.domain
fqdn2 = u'testhost2.%s' % api.env.domain
@@ -39,17 +42,30 @@ host1dn = DN(('fqdn',fqdn1),('cn','computers'),('cn','accounts'),api.env.basedn)
host2dn = DN(('fqdn',fqdn2),('cn','computers'),('cn','accounts'),api.env.basedn)
host3dn = DN(('fqdn',fqdn3),('cn','computers'),('cn','accounts'),api.env.basedn)
-fd = open('tests/test_xmlrpc/service.crt', 'r')
-servercert = fd.readlines()
-servercert = ''.join(servercert)
-servercert = x509.strip_header(servercert)
-fd.close()
-
+servercert = ''
badservercert = 'MIICbzCCAdigAwIBAgICA/4wDQYJKoZIhvcNAQEFBQAwKTEnMCUGA1UEAxMeSVBBIFRlc3QgQ2VydGlmaWNhdGUgQXV0aG9yaXR5MB4XDTEwMDgwOTE1MDIyN1oXDTIwMDgwOTE1MDIyN1owKTEMMAoGA1UEChMDSVBBMRkwFwYDVQQDExBwdW1hLmdyZXlvYWsuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAwYbfEOQPgGenPn9vt1JFKvWm/Je3y2tawGWA3LXDuqfFJyYtZ8ib3TcBUOnLk9WK5g2qCwHaNlei7bj8ggIfr5hegAVe10cun+wYErjnYo7hsHYd+57VZezeipWrXu+7NoNd4+c4A5lk4A/xJay9j3bYx2oOM8BEox4xWYoWge1ljPrc5JK46f0X7AGW4F2VhnKPnf8rwSuzI1U8VGjutyM9TWNy3m9KMWeScjyG/ggIpOjUDMV7HkJL0Di61lznR9jXubpiEC7gWGbTp84eGl/Nn9bgK1AwHfJ2lHwfoY4uiL7ge1gyP6EvuUlHoBzdb7pekiX28iePjW3iEG9IawIDAQABoyIwIDARBglghkg
[Freeipa-devel] CLDAP Netlogon fixes
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.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From 1b46d2eba3314dc1cc9ea65bebf5aaa4ba738290 Mon Sep 17 00:00:00 2001
From: Simo Sorce
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 fund
or imput 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
---
daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
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..baa309d8761db5360f7bfe13731857dc2fe49785 100644
--- a/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c
+++ b/daemons/ipa-slapi-plugins/ipa-cldap/ipa_cldap_worker.c
@@ -264,7 +264,14 @@ 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 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 e9ce2ced13da7801065f57f244becfd4f8a1ab03 Mon Sep 17 00:00:00 2001
From: Simo Sorce
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
---
.../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,
+
Re: [Freeipa-devel] DNSSEC support design considerations
On Thu, 2013-05-23 at 14:35 +0200, Petr Spacek wrote: > > It looks that we agree on nearly all points (I apologize if > overlooked > something). I will prepare a design document for transition to RBTDB > and then > another design document for DNSSEC implementation. > ACK Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] DNSSEC support design considerations
On 22.5.2013 21:58, Simo Sorce wrote:
On Wed, 2013-05-22 at 17:01 +0200, Petr Spacek wrote:
Wow, it is pretty slow.
Yeah this is what I expected, crypto is not really fast.
[...]
The simplest way how to mitigate problem with slow start-up is:
1) Store signed version of the zone on the server's file system.
2) Load signed version from disk during start up.
3) In the background, do full zone reload+resign.
4) Switch old and new zones when it is done.
Maybe instead of 3/4 we can do something that requires less computation.
We can take the list of records in the zone, and load the list of
records from LDAP.
Here we set also the persistent search but we lock it so any update is
queued until we are done with the main resync task.
(We can temporarily also refuse DNS Updates I guess)
We cross check to find which records have been changed, which have been
removed, and which have been added.
Discard all the records that are unchanged (I assume the vast majority)
and then proceed to delete/modify/add the difference.
This would save a large amount of computation at every startup, even if
in the background the main issue here is not just time, but the fact
that you pegged the CPU to 98% for so long.
It will consume some computing power during start up, but the implementation
should be really simple. (BIND naturally can save and load zones :-))
I do not think the above would be much more difficult, and could save
quite a lot of computing if done in the right order and within a bind
database transaction I guess.
It sounds doable, I agree. Naturally, I plan to start with 'naive'/'in-memory
only' implementation and add optimizations when the 'naive' part works.
The idea is that _location is dynamic though, isn't it ?
[...]
This is how I understood the design. Is it correct? If it is, then the value
is static from server's point of view. The 'dynamics' is a result of moving
client, because client is asking different servers for an answer.
Uhmm true, so we could simply store all the fields from within the
plugin so that RBTDB can sign them too.
I think my only concern is if the client can ever load some data from
one server and then some other data from another and find mismatching
signatures.
I didn't find any note about cross-checks between DNS servers. IMHO it doesn't
matter, as long as signature matches the public key in the zone.
I think that some degree of inconsistency is natural part of DNS. Typically,
all changes are propagated from the 'master'/'root of the tree topology'
through multiple levels of slaves to the 'leaf slaves'.
Signatures contain timestamps and are periodically re-computed (in order of
weeks) and it takes some time to propagate new signatures through the whole tree.
What changes are going to be required in bind-dyndb-ldap to use RBTDB
from Bind ? Do we have interfaces already ? Or will it require
additional changes to the glue code we currently use to load our plugin
into bind ?
I have some proof-of-concept code. AFAIK no change to public interfaces are
necessary.
There are 40 functions each database driver have to implement. Currently, we
have own implementation for most of them and some of them are NULL because are
required only for DNSSEC.
The typical change from our implementation to the native one looks like this:
static isc_result_t
find(dns_db_t *db, dns_name_t *name, dns_dbversion_t *version,
dns_rdatatype_t type, unsigned int options, isc_stdtime_t now,
dns_dbnode_t **nodep, dns_name_t *foundname, dns_rdataset_t *rdataset,
dns_rdataset_t *sigrdataset)
{
- [next 200 lines of our code]
+ return dns_db_find(ldapdb->rbtdb, name, version, type, options, now,
+ nodep, foundname, rdataset, sigrdataset);
}
Most of the work is about understanding how the native database work.
I assume rbtdb is now pretty stable and semantic changes are quite
unlikely ?
BIND (with our patches) has defined interface for database backends. Either
bind-dyndb-ldap and RBTDB implement this interface, so semantic change is very
very unlikely.
The plan is to use the 'public' RBTDB interface to avoid any touch between
bind-dyndb-ldap and 'internal knobs' in RBTDB.
At the moment I'm able to load data from LDAP and push them to the native
database except the zone serial. It definitely needs more investigation, but
it seems doable.
Well if we store the data in the b permanently and synchronize at
startup I guess the serial problem vanishes completely ? (assuming we
use timestamp based serials)
Yes, basically we don't need to write it back to LDAP at all. The behaviour
should be same as with current implementation.
Do you want to go back to 'light side of the force'? So we should start with
designing some LDAP->nsupdate gateway and use that for zone maintenance. It
doesn't solve adding/reconfiguring of zones on run-time, but it could be
handled by some stand-alone daemon with an abstraction layer at proper place.
