Re: [Freeipa-devel] DNS SOA serial managed by 389 DS plugin: design
On 11.2.2013 17:23, Simo Sorce wrote: On Mon, 2013-02-11 at 15:37 +0100, Petr Spacek wrote: Possible optimization Increment serial value at most once per second. Basic idea: Write current timestamp (no incrementation) and write serial value to the database with one second delay. Problem: How to solve LDAP server crash? Write timestamp immediately and (while reading) subtract 1 if timestamp == serial? I am not sure I understand the solution here ? Maybe a simpler solution is to always write the serial as time() + 1 ? I tried to solve following problem: (numbers represent time in second.millisecond format) 1.000 : new_serial = time() + 1 1.100 : record test.example.com. updated 1.100 : zone serial overwritten with new_serial 1.500 : zone transfer started 1.500 : search result for all records and zone serial stored 1.500 : search result is transferred to slaves 1.700 : record test2.example.com. updated 1.700 : zone serial overwritten with new_serial no changes from now Result: Zones on master and it's slave servers have serial = new_serial but the zone content is different (records under test2.example.com. are not equal). What I tried to describe above is that search for zone serial returns: if (serial value == time()) return (serial value - 1) else return (serial value) Example: 1.000 : new_serial = time() 1.100 : record test.example.com. updated 1.100 : zone serial overwritten with new_serial 1.500 : zone transfer started 1.500 : search result for all records and zone serial stored 1.500 : zone serial in search result is (new_serial - 1) 1.500 : snapshot is transferred to slaves 1.700 : record test2.example.com. updated 1.700 : zone serial overwritten with new_serial no changes from now 2.000 : now the search for serial value returns new_serial, i.e. slaves see value incremented by one from last zone transfer (1.500) 9.000 : serial value is unchanged from last search Expected result: Zone data can be inconsistent between master and slaves for only one second. Data will be consistent if directory server crashed at 1.701 - new zone transfer can be initiated after server restart. Requirement: DS plugin have to modify serial value during reads. Does getimeofday() cost too much to do it for all read-serial operations? Makes it sense? -- Petr^2 Spacek ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 370 ipa-kdb: remove memory leaks
All known memory leaks caused by unfreed allocated memory or unfreed LDAP results (which should be also done after unsuccessful searches) are fixed. One ipadb_need_retry result check was fixed as this function returns trust in case of a need for retry and not a zero. https://fedorahosted.org/freeipa/ticket/3413 --- This patch should be combined with Sumit's patch 0093 which would prevent kdb5kdc from crashing when it is being terminted. If you want to run krb5kdc in valgrind, check the command in the ticket which would let valgrind also show symbols in dlopen'ed ipa-kdb plugin. Martin From 3927ce572eef0d7fe6668201cec8d519fe7fa815 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Tue, 12 Feb 2013 11:59:22 +0100 Subject: [PATCH] ipa-kdb: remove memory leaks All known memory leaks caused by unfreed allocated memory or unfreed LDAP results (which should be also done after unsuccessful searches) are fixed. One ipadb_need_retry result check was fixed as this function returns trust in case of a need for retry and not a zero. https://fedorahosted.org/freeipa/ticket/3413 --- daemons/ipa-kdb/ipa_kdb.c| 4 daemons/ipa-kdb/ipa_kdb.h| 2 ++ daemons/ipa-kdb/ipa_kdb_common.c | 13 +++-- daemons/ipa-kdb/ipa_kdb_mspac.c | 18 +- 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c index 3527cefa10df67d3f17c730ab4483410c736a44f..55a932abdc3aeaa48892715196834a25f9b2c0e1 100644 --- a/daemons/ipa-kdb/ipa_kdb.c +++ b/daemons/ipa-kdb/ipa_kdb.c @@ -40,10 +40,14 @@ static void ipadb_context_free(krb5_context kcontext, { if (*ctx != NULL) { free((*ctx)-uri); +free((*ctx)-base); +free((*ctx)-realm_base); /* ldap free lcontext */ if ((*ctx)-lcontext) { ldap_unbind_ext_s((*ctx)-lcontext, NULL, NULL); } +free((*ctx)-supp_encs); +ipadb_mspac_struct_free((*ctx)-mspac); krb5_free_default_realm(kcontext, (*ctx)-realm); free(*ctx); *ctx = NULL; diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h index beff8b208685a7d561fc096c8e734333437bdb58..f472f02458e040b921b9f3f85bcc36fa954785d5 100644 --- a/daemons/ipa-kdb/ipa_kdb.h +++ b/daemons/ipa-kdb/ipa_kdb.h @@ -237,6 +237,8 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx); +void ipadb_mspac_struct_free(struct ipadb_mspac **mspac); + /* DELEGATION CHECKS */ krb5_error_code ipadb_check_allowed_to_delegate(krb5_context kcontext, diff --git a/daemons/ipa-kdb/ipa_kdb_common.c b/daemons/ipa-kdb/ipa_kdb_common.c index e04bae6673ddc97325883708d2bca2805f6ae4fa..956b4b611cbdedbe1a9b0415e9b10f762e46fdae 100644 --- a/daemons/ipa-kdb/ipa_kdb_common.c +++ b/daemons/ipa-kdb/ipa_kdb_common.c @@ -172,7 +172,7 @@ krb5_error_code ipadb_simple_search(struct ipadb_context *ipactx, /* first test if we need to retry to connect */ if (ret != 0 ipadb_need_retry(ipactx, ret)) { - +ldap_msgfree(res); ret = ldap_search_ext_s(ipactx-lcontext, basedn, scope, filter, attrs, 0, NULL, NULL, std_timeout, LDAP_NO_LIMIT, @@ -283,6 +283,7 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, int times; int ret; int c, i; +bool retry; for (c = 0; deref_attr_names[c]; c++) { /* count */ ; @@ -315,7 +316,8 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, /* retry once if connection errors (tot. max. 2 tries) */ times = 2; ret = LDAP_SUCCESS; -while (!ipadb_need_retry(ipactx, ret) times 0) { +retry = true; +while (retry) { times--; ret = ldap_search_ext_s(ipactx-lcontext, base_dn, scope, filter, @@ -323,11 +325,18 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, ctrl, NULL, std_timeout, LDAP_NO_LIMIT, res); +retry = ipadb_need_retry(ipactx, ret) times 0; + +if (retry) { +/* Free result before next try */ +ldap_msgfree(*res); +} } kerr = ipadb_simple_ldap_to_kerr(ret); done: +ldap_control_free(ctrl[0]); ldap_memfree(derefval.bv_val); free(ds); return kerr; diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 0780e81cb5507ed590cc9b0646ba5919c0084523..6abd51ec019ef45dd52d317b85dcb9584b49f06f 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -944,6 +944,7 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, goto done; } +ldap_msgfree(results); kerr = ipadb_deref_search(ipactx, basedn, LDAP_SCOPE_ONE, filter,
[Freeipa-devel] [PATCH 0111] Automatically reload invalid zone after each change in zone data
Hello, Automatically reload invalid zone after each change in zone data. https://fedorahosted.org/bind-dyndb-ldap/ticket/102 How to test: # create a invalid zone, e.g. zone without A records for names in NS records ipa dnszone-add zone.test --admin-email=blah.nonsense --name-server=ns.zone.test. --force # now dig ns.zone.test. should return SERVFAIL because zone doesn't have proper NS+A/ records dig ns.zone.test. # addition of arbitrary record should not crash the server ipa dnsrecord-add zone.test ns --txt-rec=blah # after this modification some error message should appear in log and dig ns.zone.test. should still return SERVFAIL dig ns.zone.test. # addition of valid A record should fix the problem ipa dnsrecord-add zone.test ns --a-rec=127.0.0.1 # now dig -t ANY ns.zone.test. should return NOERROR and zone should work # TXT and also A record should be visible dig -t ANY ns.zone.test. -- Petr^2 Spacek From de083cd61471384ccdcc0f02303390d92928a506 Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 12 Feb 2013 12:42:29 +0100 Subject: [PATCH] Automatically reload invalid zone after each change in zone data. https://fedorahosted.org/bind-dyndb-ldap/ticket/102 Signed-off-by: Petr Spacek pspa...@redhat.com --- NEWS | 3 +++ src/ldap_helper.c | 38 -- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 9f50db12de760ccaa7f4adb5cc03534ae72c47be..6dd09c118c86c4f5daf44bfbc5978600092b3d7f 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +[1] Automatically reload invalid zone after each change in zone data. +https://fedorahosted.org/bind-dyndb-ldap/ticket/102 + 2.5 = [1] Fix crash during per-zone cache flush. diff --git a/src/ldap_helper.c b/src/ldap_helper.c index 71b96ce5eef2a249f7d16c448c70e2c2068d271d..491817813229f988161f3cedc6c83055040ad3ae 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -3411,8 +3411,11 @@ update_record(isc_task_t *task, isc_event_t *event) ldap_psearchevent_t *pevent = (ldap_psearchevent_t *)event; isc_result_t result; ldap_instance_t *inst = NULL; - ldap_cache_t *cache = NULL; + ldap_cache_t *cache; isc_mem_t *mctx; + dns_zone_t *zone_ptr = NULL; + isc_boolean_t zone_found = ISC_FALSE; + isc_boolean_t zone_reloaded = ISC_FALSE; mctx = pevent-mctx; UNUSED(task); @@ -3431,13 +3434,16 @@ update_record(isc_task_t *task, isc_event_t *event) dns_name_init(prevname, NULL); dns_name_init(prevorigin, NULL); CHECK(dn_to_dnsname(mctx, pevent-dn, name, origin)); + zone_found = ISC_TRUE; +update_restart: if (PSEARCH_DEL(pevent-chgtype) || PSEARCH_MODDN(pevent-chgtype)) { log_debug(5, psearch_update: removing name from cache, dn: '%s', pevent-dn); } /* Get cache instance clean old record */ + cache = NULL; CHECK(zr_get_zone_cache(inst-zone_register, name, cache)); CHECK(discard_from_cache(cache, name)); @@ -3486,11 +3492,39 @@ update_record(isc_task_t *task, isc_event_t *event) if (inst-serial_autoincrement PSEARCH_ANY(pevent-chgtype)) { CHECK(soa_serial_increment(mctx, inst, origin)); } + cleanup: - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS zone_found !zone_reloaded + (result == DNS_R_NOTLOADED || result == DNS_R_BADZONE)) { + log_debug(1, reloading invalid zone after a change; + reload triggered by change in '%s', + pevent-dn); + + result = zr_get_zone_ptr(inst-zone_register, origin, zone_ptr); + if (result == ISC_R_SUCCESS) + result = dns_zone_load(zone_ptr); + if (zone_ptr != NULL) + dns_zone_detach(zone_ptr); + + if (result == ISC_R_SUCCESS || result == DNS_R_UPTODATE || + result == DNS_R_DYNAMIC || result == DNS_R_CONTINUE) { + /* zone reload succeeded, fire current event again */ + log_debug(1, restarting update_record after zone reload + caused by change in '%s', pevent-dn); + zone_reloaded = ISC_TRUE; + goto update_restart; + } else { + log_error_r(unable to reload invalid zone; +reload triggered by change in '%s', +pevent-dn); + } + + } else if (result != ISC_R_SUCCESS) { + /* error other than invalid zone */ log_error_r(update_record (psearch) failed, dn '%s' change type 0x%x. Records can be outdated, run `rndc reload`, pevent-dn, pevent-chgtype); + } if (dns_name_dynamic(name)) dns_name_free(name, inst-mctx); -- 1.7.11.7 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0112] Make log messages related to Kerberos more verbose
Hello, Make log messages related to Kerberos more verbose. This change should help people supporting bind-dyndb-ldap to figure out what is happening under covers. -- Petr^2 Spacek From a7cae08cacad019852067dd7ecf86cefbe35c70e Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 12 Feb 2013 13:49:32 +0100 Subject: [PATCH] Make log messages related to Kerberos more verbose. Signed-off-by: Petr Spacek pspa...@redhat.com --- src/krb5_helper.c | 30 +++--- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/krb5_helper.c b/src/krb5_helper.c index ffa6938d08a37d3470dd9184be2d8ab5c604afdf..56d6777acea0aa1e342fb6f0c073991f86760af0 100644 --- a/src/krb5_helper.c +++ b/src/krb5_helper.c @@ -60,15 +60,15 @@ check_credentials(krb5_context context, krberr = krb5_build_principal(context, mcreds.server, strlen(realm), realm, krbtgt, realm, NULL); - CHECK_KRB5(context, krberr, Failed to build tgt principal); + CHECK_KRB5(context, krberr, Failed to build 'krbtgt/REALM' principal); /* krb5_cc_retrieve_cred filters on both server and client */ mcreds.client = service; krberr = krb5_cc_retrieve_cred(context, ccache, 0, mcreds, creds); if (krberr) { const char * errmsg = krb5_get_error_message(context, krberr); - log_debug(2, Principal not found in cred cache (%s), + log_debug(2, Credentials are not present in cache (%s), errmsg); krb5_free_error_message(context, errmsg); result = ISC_R_FAILURE; @@ -79,7 +79,7 @@ check_credentials(krb5_context context, CHECK_KRB5(context, krberr, Failed to get timeofday); if (now (creds.times.endtime + MIN_TIME)) { - log_debug(2, Credentials expired); + log_debug(2, Credentials in cache expired); result = ISC_R_FAILURE; goto cleanup; } @@ -134,31 +134,35 @@ get_krb5_tgt(isc_mem_t *mctx, const char *principal, const char *keyfile) ret = setenv(KRB5CCNAME, str_buf(ccname), 1); if (ret == -1) { - log_error(Failed to set KRB5CCNAME environment variable); + log_error(Failed to set KRB5CCNAME environment variable to + '%s', str_buf(ccname)); result = ISC_R_FAILURE; goto cleanup; } krberr = krb5_cc_resolve(context, str_buf(ccname), ccache); CHECK_KRB5(context, krberr, - Failed to resolve ccache name %s, str_buf(ccname)); + Failed to resolve credentials cache name '%s', + str_buf(ccname)); /* get krb5_principal from string */ krberr = krb5_parse_name(context, principal, kprincpw); CHECK_KRB5(context, krberr, - Failed to parse the principal name %s, principal); + Failed to parse the principal name '%s', principal); /* check if we already have valid credentials */ result = check_credentials(context, ccache, kprincpw); if (result == ISC_R_SUCCESS) { - log_debug(2, Found valid cached credentials); + log_debug(2, Found valid Kerberos credentials in cache); goto cleanup; + } else { + log_debug(2, Attempting to acquire new Kerberos credentials); } /* open keytab */ krberr = krb5_kt_resolve(context, keyfile, keytab); CHECK_KRB5(context, krberr, - Failed to resolve keytab file %s, keyfile); + Failed to resolve keytab file '%s', keyfile); memset(my_creds, 0, sizeof(my_creds)); memset(options, 0, sizeof(options)); @@ -170,15 +174,19 @@ get_krb5_tgt(isc_mem_t *mctx, const char *principal, const char *keyfile) /* get tgt */ krberr = krb5_get_init_creds_keytab(context, my_creds, kprincpw, keytab, 0, NULL, options); - CHECK_KRB5(context, krberr, Failed to init credentials); + CHECK_KRB5(context, krberr, Failed to get initial credentials (TGT) +using principal '%s' and keytab '%s', +principal, keyfile); my_creds_ptr = my_creds; /* store credentials in cache */ krberr = krb5_cc_initialize(context, ccache, kprincpw); - CHECK_KRB5(context, krberr, Failed to initialize ccache); + CHECK_KRB5(context, krberr, Failed to initialize credentials cache +'%s', str_buf(ccname)); krberr = krb5_cc_store_cred(context, ccache, my_creds); - CHECK_KRB5(context, krberr, Failed to store ccache); + CHECK_KRB5(context, krberr, Failed to store credentials +in credentials cache '%s', str_buf(ccname)); result = ISC_R_SUCCESS; -- 1.7.11.7 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 370 ipa-kdb: remove memory leaks
On Tue, Feb 12, 2013 at 12:24:48PM +0100, Martin Kosek wrote: All known memory leaks caused by unfreed allocated memory or unfreed LDAP results (which should be also done after unsuccessful searches) are fixed. One ipadb_need_retry result check was fixed as this function returns trust in case of a need for retry and not a zero. https://fedorahosted.org/freeipa/ticket/3413 --- This patch should be combined with Sumit's patch 0093 which would prevent kdb5kdc from crashing when it is being terminted. If you want to run krb5kdc in valgrind, check the command in the ticket which would let valgrind also show symbols in dlopen'ed ipa-kdb plugin. Martin I haven't look closely so far, but I see the following compiler warning: ipa_kdb_common.c: In function 'ipadb_simple_search': ipa_kdb_common.c:175:9: warning: passing argument 1 of 'ldap_msgfree' from incompatible pointer type [enabled by default] In file included from ipa_kdb.h:35:0, from ipa_kdb_common.c:23: /usr/include/ldap.h:1848:1: note: expected 'struct LDAPMessage *' but argument is of type 'struct LDAPMessage **' bye, Sumit ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] More types of replicas in FreeIPA
On Mon, 2013-02-11 at 20:30 -0500, Dmitri Pal wrote: On 02/11/2013 03:21 PM, Simo Sorce wrote: On Mon, 2013-02-11 at 21:03 +0100, Ondrej Hamada wrote: Dne 3.2.2013 02:51, Dmitri Pal napsal(a): On 01/31/2013 06:09 PM, Ondrej Hamada wrote: Hello, I'm starting to work on my thesis about 'More types of replicas in FreeIPA' again. One of the main problems is the way how should the read-only replicas deal with KDC because they're not supposed to posses the Kerberos (krb) master key. The task was to investigate how is this solved in Active Directory and its Read Only Domain Controllers. I found out that the basic of RODC behaviour is described on technet page (http://technet.microsoft.com/en-us/library/cc754218%28v=ws.10%29.aspx). Login situation: RODC by default forwards the KRB requests to the DC. RODC then forwards the response back to the client and also requests the password to be replicated to RODC. Both the user and his host must be members of 'Allowed RODC Password Replication' group in order to let user's passwords being replicated to RODCs. Request services that the RODC doesn't have credentials for: Client sends TGS-REQ to RODC. RODC can read the TGT in the request, but doesn't have credentials for the service. So the request is forwarded to the DC. DC can decrypt the TGT that was created by RODC and sends back the TGS-RES that is forwarded to the client. (but it does not trust the RODC so it recalculates the privilege attribute certificate). RODC does not cache the credentials for the service. During my experiments the credentials got replicated to the RODC on the first log on of the user. The user's KRB requests were first forwarded to the DC. When the user got krbtgt and TGS for host, ldap and cifs, his TGT was revoked by RODC. He run through the auth. process again, but this time the requests were served by RODC only - no forwarding - and not TGS for host was requested. Unfortunately I can not still recognize how the keys are processed. There's barely any RPC communication - only one DCERPC packet exchange between RODC and DC that takes place when the user sends his first TGS request (this exchange happens also for the clients with disabled replication). It looks to me like the DC knows all the RODC keys. According to Technet, the MS implementation of Kerberos is able to recognize the key owner from the Key Version Number value. I think I can't get more info from the network traffic examination. Do you have any ideas or hints on further investigation of the problem? The page you listed shows the diagrams of the user login and TGT and then TGS acquisition. http://technet.microsoft.com/en-us/library/cc754218%28v=WS.10%29#BKMK_AuthRODC Also the following thread sheds some good light on how the authentication and caching happens in case of the RODC. http://social.technet.microsoft.com/forums/en-US/winserverDS/thread/f8d1017e-1f0e-4a9d-a241-b03b508dfe17 So they have policy driven replication of passwords. If password is allowed to be replicated by policy it is replicated if not it RODC has to always proxy to RWDC for this account. The password changes always happen on RWDC and replicated back. They can be replicated by RSO operation that allows replicating a single object. It seems that they assume that all the services are always remote thus connectivity to the central RWDC is a must. They do not seem to keep any service keys locally in the RODC. With SSSD in play on the client I am not sure we should worry about caching at least in the first implementation. So in our case it might make sense to have a proxy KDC and a RO replica with the subset of data replicated to it. I wonder if you can accomplish that by taking 389 RO replica + IAKERB I suggest you look at IAKERB and see if it can be used as a proxy for user authentication, password change and TGS requests. If not may be we can at least use it as an inspiration. The attached diagram shows what I mean. Later we can add some logic for the following: a) RODC requesting replication of a specific object b) RWDC replicating a specific object c) Policy to define for which accounts the passwords and keys can replicated d) RODC getting policy and performing local authentication for the accounts for which the keys were replicated. However with SSSD on the client it might be easier for KDC proxy just to go offline and not respond to the SSSD if it loses connection to the central server. That would force SSSD to go offline and use local password caching. I suspect that SSSD password caching would be enabled in many cases anyways so not caching all passwords in one central locating in RODC might actually be a good thing. Thanks for hints. I was looking at IAKERB and according to RFC it should support both AS and TGS requests/replies so it might be doable. I'll try to
Re: [Freeipa-devel] [PATCH] 370 ipa-kdb: remove memory leaks
On Tue, 2013-02-12 at 12:24 +0100, Martin Kosek wrote: Comments inline. --- a/daemons/ipa-kdb/ipa_kdb_common.c +++ b/daemons/ipa-kdb/ipa_kdb_common.c @@ -172,7 +172,7 @@ krb5_error_code ipadb_simple_search(struct ipadb_context *ipactx, /* first test if we need to retry to connect */ if (ret != 0 ipadb_need_retry(ipactx, ret)) { - +ldap_msgfree(res); Why do we need this ? We get in this branch only if the previous search failed, so res should always be empty. What am I missing ? ret = ldap_search_ext_s(ipactx-lcontext, basedn, scope, filter, attrs, 0, NULL, NULL, std_timeout, LDAP_NO_LIMIT, @@ -283,6 +283,7 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, int times; int ret; int c, i; +bool retry; for (c = 0; deref_attr_names[c]; c++) { /* count */ ; @@ -315,7 +316,8 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, /* retry once if connection errors (tot. max. 2 tries) */ times = 2; ret = LDAP_SUCCESS; -while (!ipadb_need_retry(ipactx, ret) times 0) { +retry = true; +while (retry) { times--; ret = ldap_search_ext_s(ipactx-lcontext, base_dn, scope, filter, @@ -323,11 +325,18 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, ctrl, NULL, std_timeout, LDAP_NO_LIMIT, res); +retry = ipadb_need_retry(ipactx, ret) times 0; + +if (retry) { +/* Free result before next try */ +ldap_msgfree(*res); +} } This snippet seem to change the logic. Before the condition was !ipadb_need_retry() and no you change it to ipadb_need_retry() so it looks reversed, was this on purpose ? kerr = ipadb_simple_ldap_to_kerr(ret); done: +ldap_control_free(ctrl[0]); ldap_memfree(derefval.bv_val); free(ds); return kerr; diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 0780e81cb5507ed590cc9b0646ba5919c0084523..6abd51ec019ef45dd52d317b85dcb9584b49f06f 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -944,6 +944,7 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, goto done; } +ldap_msgfree(results); kerr = ipadb_deref_search(ipactx, basedn, LDAP_SCOPE_ONE, filter, entry_attrs, deref_search_attrs, memberof_pac_attrs, results); @@ -1636,14 +1637,24 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, /* put in signed data */ ad.magic = KV5M_AUTHDATA; ad.ad_type = KRB5_AUTHDATA_WIN2K_PAC; -ad.contents = (krb5_octet *)pac_data.data; +ad.contents = malloc(pac_data.length); +if (ad.contents == NULL) { +krb5_free_data_contents(context, pac_data); +ad.length = 0; +kerr = ENOMEM; +goto done; +} +memcpy(ad.contents, pac_data.data, pac_data.length); ad.length = pac_data.length; +krb5_free_data_contents(context, pac_data); + Why all this copying around ? It should be sufficient to call krb5_free_data_contents() at the end at most. authdata[0] = ad; kerr = krb5_encode_authdata_container(context, KRB5_AUTHDATA_IF_RELEVANT, authdata, signed_auth_data); +free(ad.contents); if (kerr != 0) { goto done; } The rest looks good. 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] [PATCH] 361 ipa-adtrust-install should ask for SID generation
On 02/12/2013 04:48 PM, Alexander Bokovoy wrote: On Fri, 01 Feb 2013, Martin Kosek wrote: On 01/31/2013 07:06 PM, Alexander Bokovoy wrote: On Thu, 31 Jan 2013, Martin Kosek wrote: On 01/31/2013 04:29 PM, Alexander Bokovoy wrote: On Thu, 31 Jan 2013, Martin Kosek wrote: When ipa-adtrust-install is run, check if there are any objects that need to have SID generated. If yes, interactively ask the user if the sidgen task should be run. https://fedorahosted.org/freeipa/ticket/3195 ... I would still run this check in options.unattended mode and reported warning, for accounting purposes. Could you please make so? Sure! Updated patch attached. Thanks! I have only small addition: +object_count = len(entries) +if object_count 0: +print +print WARNING: %d existing users or groups do not have a SID identifier assigned. \ +% len(entries) +print Installer can run a task to have ipa-sidgen Directory Server plugin generate +print the SID identifier for all these users. Please note, the in case of a high +print number of users and groups, the operation might lead to high replication +print traffic and performance degradation. Refer to ipa-adtrust-install(1) man page +print for details. +print +if not options.unattended: +if ipautil.user_input(Do you want to run the ipa-sidgen task?, default=False, +allow_empty=False): +options.add_sids = True ... to make the text of warning consistent it would be good to add + else: + print Unattended mode was selected, installer will *not* run ipa-sidgen task! And here is the updated patch. ACK. I actually tested it already with other patches just forgot to reply to this email. Pushed to master, ipa-3-1. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 02/12/2013 05:14 PM, Endi Sukma Dewata wrote: On 2/8/2013 7:27 AM, Petr Vobornik wrote: Checkbox for NONE option was added. https://fedorahosted.org/freeipa/ticket/3404 Patches for master and 3.1 branch attached. ACK. We were discussing to NACK this approach. The implementation should be improved because of the mutually exclusive nature of NONE option with [MS-PAC, PAD] options. I think we should add spec definition (to Web UI only, or into server plugin as well) of mutex options. Something like: mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of groups*/]]; basically an array of group arrays where group array would contain arrays of mutually exclusive option. Is it over-complicated? Would one array or a pair of groups be enough? That way we can disable and uncheck(should not happen, but to keep data consistency) checkboxes of options from other groups when user selects a value of some group. As a separate issue, we might want to fetch the options from metadata, when we want to show the values and don't care about creating different labels. -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0030] Add option to specify SID using domain name to idrange-add/mod
On Fri, 08 Feb 2013, Tomas Babej wrote: On 02/08/2013 03:25 PM, Alexander Bokovoy wrote: On Mon, 04 Feb 2013, Tomas Babej wrote: Hi, When adding/modifying an ID range for a trusted domain, the newly added option --dom-name can be used. This looks up SID of the trusted domain in LDAP and therefore the user is not required to write it down in CLI. If the lookup fails, error message asking the user to specify the SID manually is shown. https://fedorahosted.org/freeipa/ticket/3133 Tomas From 72f8802953edaaf5b9f7c34a38601fbccd681c8e Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Mon, 4 Feb 2013 08:33:53 -0500 Subject: [PATCH] Add option to specify SID using domain name to idrange-add/mod When adding/modifying an ID range for a trusted domain, the newly added option --dom-name can be used. This looks up SID of the trusted domain in LDAP and therefore the user is not required to write it down in CLI. If the lookup fails, error message asking the user to specify the SID manually is shown. https://fedorahosted.org/freeipa/ticket/3133 --- ipalib/plugins/idrange.py | 78 +-- ipaserver/dcerpc.py | 10 ++ 2 files changed, 78 insertions(+), 10 deletions(-) diff --git a/ipalib/plugins/idrange.py b/ipalib/plugins/idrange.py index 84e1057ac6b59b8ad99882a54e3288897338c978..77a75e4cabc18ca873be7cadcf870427d5b36ea0 100644 --- a/ipalib/plugins/idrange.py +++ b/ipalib/plugins/idrange.py @@ -197,6 +197,11 @@ class idrange(LDAPObject): cli_name='dom_sid', label=_('Domain SID of the trusted domain'), ), + Str('ipanttrusteddomainname?', + cli_name='dom_name', + flags=('no_search', 'virtual_attribute'), + label=_('Name of the trusted domain'), + ), New options is added but API.txt wasn't changed. As result, 'make rpms' does not work. Could you please fix the patch and re-send it? Sorry about that. Updated patch attached. I have one small question regarding use of dom_sid/dom_name. If both dom_sid and dom_name were specified, failing to resolve dom_name would force command to raise exception. I'm not sure this is right behavior. Probably we should detect that both dom_sid and dom_name were specified and bail out earlier so that only one of them is accepted. That would be clearer to users, wouldn't it? -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 255 Added Web UI support for service PAC type option: NONE
On 2/12/2013 10:56 AM, Petr Vobornik wrote: We were discussing to NACK this approach. The implementation should be improved because of the mutually exclusive nature of NONE option with [MS-PAC, PAD] options. I think we should add spec definition (to Web UI only, or into server plugin as well) of mutex options. Something like: mutex_groups: [[['NONE'],['MS-PAC', 'PAD']], [/*another array of groups*/]]; basically an array of group arrays where group array would contain arrays of mutually exclusive option. Is it over-complicated? Would one array or a pair of groups be enough? That way we can disable and uncheck(should not happen, but to keep data consistency) checkboxes of options from other groups when user selects a value of some group. If they are mutually exclusive, they probably should be separated using radio buttons like this: PAC: ( ) None (o) Type: [x] MS-PAC [ ] PAD It might be better to use a composite widget of radio buttons and checkboxes so we can reuse the code. Probably the definition will look something like this: { name: 'ipakrbauthzdata', type: 'radio', label: ..., options: [ { label: ..., value: 'NONE' }, { label: ..., type: 'checkboxes', options: [ { label: ..., value: 'MS-PAC' }, { label: ..., value: 'PAD' } ] } ] } The composite widget will handle setting the appropriate widgets when the value of the field on load. It will handle enabling/disabling the checkboxes when the radio button is selected. It will also compute the final value of the field from selected radio button/checkboxes on save. Or just dim (no disable) and uncheck. That way there would still be visual distinction and one don't have to uncheck all the options from one group when he wants to select options of mutex group. That is also possible, but it changes the normal behavior of checkboxes, so probably users would have to play around with it to understand the grouping. They could also get confused, e.g. is dimmed checkbox disabled? As a separate issue, we might want to fetch the options from metadata, when we want to show the values and don't care about creating different labels. If we use radio buttons like above, new labels are necessary to describe the different groups of checkboxes. For all radio buttons checkboxes in general, if we don't want to use labels we probably could simplify the definition such that we can specify the string values directly (without nested object): { name: 'ipakrbauthzdata', type: 'radio', options: [ 'NONE', { type: 'checkboxes', options: ['MS-PAC', 'PAD'] } ] } -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] More types of replicas in FreeIPA
On 02/12/2013 08:20 AM, Simo Sorce wrote: On Mon, 2013-02-11 at 20:30 -0500, Dmitri Pal wrote: On 02/11/2013 03:21 PM, Simo Sorce wrote: On Mon, 2013-02-11 at 21:03 +0100, Ondrej Hamada wrote: Dne 3.2.2013 02:51, Dmitri Pal napsal(a): On 01/31/2013 06:09 PM, Ondrej Hamada wrote: Hello, I'm starting to work on my thesis about 'More types of replicas in FreeIPA' again. One of the main problems is the way how should the read-only replicas deal with KDC because they're not supposed to posses the Kerberos (krb) master key. The task was to investigate how is this solved in Active Directory and its Read Only Domain Controllers. I found out that the basic of RODC behaviour is described on technet page (http://technet.microsoft.com/en-us/library/cc754218%28v=ws.10%29.aspx). Login situation: RODC by default forwards the KRB requests to the DC. RODC then forwards the response back to the client and also requests the password to be replicated to RODC. Both the user and his host must be members of 'Allowed RODC Password Replication' group in order to let user's passwords being replicated to RODCs. Request services that the RODC doesn't have credentials for: Client sends TGS-REQ to RODC. RODC can read the TGT in the request, but doesn't have credentials for the service. So the request is forwarded to the DC. DC can decrypt the TGT that was created by RODC and sends back the TGS-RES that is forwarded to the client. (but it does not trust the RODC so it recalculates the privilege attribute certificate). RODC does not cache the credentials for the service. During my experiments the credentials got replicated to the RODC on the first log on of the user. The user's KRB requests were first forwarded to the DC. When the user got krbtgt and TGS for host, ldap and cifs, his TGT was revoked by RODC. He run through the auth. process again, but this time the requests were served by RODC only - no forwarding - and not TGS for host was requested. Unfortunately I can not still recognize how the keys are processed. There's barely any RPC communication - only one DCERPC packet exchange between RODC and DC that takes place when the user sends his first TGS request (this exchange happens also for the clients with disabled replication). It looks to me like the DC knows all the RODC keys. According to Technet, the MS implementation of Kerberos is able to recognize the key owner from the Key Version Number value. I think I can't get more info from the network traffic examination. Do you have any ideas or hints on further investigation of the problem? The page you listed shows the diagrams of the user login and TGT and then TGS acquisition. http://technet.microsoft.com/en-us/library/cc754218%28v=WS.10%29#BKMK_AuthRODC Also the following thread sheds some good light on how the authentication and caching happens in case of the RODC. http://social.technet.microsoft.com/forums/en-US/winserverDS/thread/f8d1017e-1f0e-4a9d-a241-b03b508dfe17 So they have policy driven replication of passwords. If password is allowed to be replicated by policy it is replicated if not it RODC has to always proxy to RWDC for this account. The password changes always happen on RWDC and replicated back. They can be replicated by RSO operation that allows replicating a single object. It seems that they assume that all the services are always remote thus connectivity to the central RWDC is a must. They do not seem to keep any service keys locally in the RODC. With SSSD in play on the client I am not sure we should worry about caching at least in the first implementation. So in our case it might make sense to have a proxy KDC and a RO replica with the subset of data replicated to it. I wonder if you can accomplish that by taking 389 RO replica + IAKERB I suggest you look at IAKERB and see if it can be used as a proxy for user authentication, password change and TGS requests. If not may be we can at least use it as an inspiration. The attached diagram shows what I mean. Later we can add some logic for the following: a) RODC requesting replication of a specific object b) RWDC replicating a specific object c) Policy to define for which accounts the passwords and keys can replicated d) RODC getting policy and performing local authentication for the accounts for which the keys were replicated. However with SSSD on the client it might be easier for KDC proxy just to go offline and not respond to the SSSD if it loses connection to the central server. That would force SSSD to go offline and use local password caching. I suspect that SSSD password caching would be enabled in many cases anyways so not caching all passwords in one central locating in RODC might actually be a good thing. Thanks for hints. I was looking at IAKERB and according to RFC it should support both AS and TGS requests/replies so it might be doable. I'll try to prepare a prototype that will be using 389 RO replica