Re: [Freeipa-devel] DNS SOA serial managed by 389 DS plugin: design

2013-02-12 Thread Petr Spacek

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

2013-02-12 Thread Martin Kosek
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

2013-02-12 Thread Petr Spacek

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

2013-02-12 Thread Petr Spacek

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

2013-02-12 Thread Sumit Bose
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

2013-02-12 Thread Simo Sorce
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

2013-02-12 Thread Simo Sorce
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

2013-02-12 Thread Martin Kosek
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

2013-02-12 Thread Petr Vobornik

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

2013-02-12 Thread Alexander Bokovoy

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

2013-02-12 Thread Endi Sukma Dewata

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

2013-02-12 Thread Dmitri Pal
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