Re: [Freeipa-devel] What about desktop policies?

2013-03-04 Thread Petr Spacek

On 27.2.2013 14:16, Loris Santamaria wrote:

El mar, 26-02-2013 a las 15:11 -0500, Dmitri Pal escribió:

On 02/25/2013 02:15 PM, Loris Santamaria wrote:

Hi all,

some customers of ours are interested in managing desktop policies for
their linux workstations, really nothing fancy, corporate background and
proxy settings are the most common requests.

In the past I created Gnome desktop profiles using Sabayon, distributed
them using puppet and associated them to user accounts with a Sabayon
specific LDAP attribute, a process a bit convoluted, and no longer
possible since sabayon is no longer developed. Also it was really buggy,
and very gnome specific.

I was thinking in how integrate desktop policies in freeIPA in a general
manner and I wanted to share my ideas with you. Hopefully some of this
may be incorporated in IPA at some point in the future.

Properties of a policy:

   * is a collection of settings
   * can be associated with users or groups (desktop policy) or with
 hosts or hostgroups (system policy)
   * is associated with a consumer, the client software that
 interprets and applies the policy. This way one could define
 policies for dconf, policies for kde, policies for WBEM.

Properties of a setting
   * is a key-value pair
   * must conform to a schema
   * may be mandatory

The schema:
   * indicates which attributes a policy may consist of
   * indicates which kind of value may take an attribute. Bool,
 string, etc.
   * There may be more than one schema for a given consumer. For
 example for dconf you may have an evolution schema, a
 gnome-games schema, etc.

Sample policy creation process:
  1. The admin creates a new schema in IPA, with a command like ipa
 schema-add --consumer=dconf gnomeSettingsSchema
  2. The admin adds some definition to the schema: ipa
 schema-add-setting gnomeSettingsSchema
 --name=/schemas/desktop/gnome/background/picture_filename
 --type=string --description='File to use for the background
 image.'
  3. He creates a new policy: ipa policy-add corporateBackground
 --type=desktop --consumer=dconf
  4. He adds a setting to the policy: ipa policy-add-setting
 corporateBackground
 --name=/schemas/desktop/gnome/background/picture_filename
 --value=file:///san/wp/wallpaper.jpg --mandatory. Ipa would
 check that the key is defined in one of the dconf related
 schemas and the value is acceptable for that key.
  5. He associates the policy with users: ipa-policy-add-user
 corporateBackground --groups=ipausers

How should the policy be applied? On the workstation, on startup, an ipa
related utility should check if there are any policies related to the
workstation, if there are any it should call a helper capable of
applying a specific type of policy. Then on user logon another ipa
related utility should check if there are any policies associated with
the user and call the appropriate helper, if available.

For the policy created in the above example, on logon the ipa policy
utility would find that there is a policy of type dconf associated with
the user. It would check if there is a dconf policy helper installed and
if positive it would call the helper passing it the parameters defined
in the policy.

Hope this is useful at least as a starting point in defining desktop
policies in IPA.


This is great!
Thank you for sharing some ideas.
We sort of stayed away from centralized policy management for quite
some time.
Originally we thought that IPA will do policy management and did a lot
of design around it.
However at some point we realized that there is an overlap with the
system management and content management for which things like puppet
are more suitable. We said then that IdM would focus on managing
identity related policies that are traditionally served via LDAP.
The things that you are talking about resemble to some extent MSFT GPO
and we felt that IdM might not be the right place for it. May be it is
time to reassess it.
I would however not go that route at least yet.


Hey puppet is great and we use it a lot to install packages and to
distribute configuration files, yet it is not so great to set these
key=value kind of settings of which more and more modern software
consists of. When you take into consideration gconf settings for gnome
2.x, dconf settings for gnome 3.x, mozilla settings, thunderbird
settings it quickly becomes a PITA to manage them distributing around
files with puppet.


Did you look at http://augeas.net/ ? Puppet + Augeas could be very very strong 
combination.


Petr^2 Spacek


Having those key=value pairs in an ldap would allow a helper on the
client to pull only the keys it understands and to merge them in the
configuration database in the appropriate way.

Of course i took inspiration from AD GPOs, yet I think that IPA should
manage these 

[Freeipa-devel] [PATCH 0117] Fix crash caused by invalid query/transfer policy

2013-03-04 Thread Petr Spacek

Hello,

Fix crash caused by invalid query/transfer policy.

Please double-check correctness. The ISC parser is really complex beast!

Thank you.

--
Petr^2 Spacek
From 41061726684211924e453f74d1db3bec6c2e32d6 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Mon, 4 Mar 2013 14:20:56 +0100
Subject: [PATCH] Fix crash caused by invalid query/transfer policy.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 src/acl.c | 45 +++--
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/src/acl.c b/src/acl.c
index f95cf431b6363d82085e9cfec7e6c1d6ddd45d7a..076a50375ae1fd132c143aa96379f7c80cc78cb8 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -71,6 +71,19 @@ static cfg_type_t *allow_query;
 static cfg_type_t *allow_transfer;
 static cfg_type_t *forwarders;
 
+/* Following definitions are necessary for context (map configuration object)
+ * required during ACL parsing. */
+static cfg_clausedef_t * empty_map_clausesets[] = {
+	NULL
+};
+
+static cfg_type_t cfg_type_empty_map = {
+	empty_map, cfg_parse_map, cfg_print_map, cfg_doc_map, cfg_rep_map,
+	empty_map_clausesets
+};
+
+static cfg_type_t *empty_map_p = cfg_type_empty_map;
+
 static cfg_type_t *
 get_type_from_tuplefield(const cfg_type_t *cfg_type, const char *name)
 {
@@ -469,44 +482,56 @@ acl_from_ldap(isc_mem_t *mctx, const char *aclstr, acl_type_t type,
 	cfg_parser_t *parser = NULL;
 	cfg_obj_t *aclobj = NULL;
 	cfg_aclconfctx_t *aclctx = NULL;
+	/* ACL parser requires configuration context. The parser looks for
+	 * undefined names in this context. We create empty context (map type),
+	 * i.e. only built-in named lists any, none etc. are supported. */
+	cfg_obj_t *cctx = NULL;
+	cfg_parser_t *parser_empty = NULL;
 
 	REQUIRE(aclp != NULL  *aclp == NULL);
 
 	CHECK(bracket_str(mctx, aclstr, new_aclstr));
 
 	CHECK(cfg_parser_create(mctx, dns_lctx, parser));
+	CHECK(cfg_parser_create(mctx, dns_lctx, parser_empty));
+	CHECK(parse(parser_empty, {}, empty_map_p, cctx));
+
 	switch (type) {
 	case acl_type_query:
-		result = parse(parser, str_buf(new_aclstr), allow_query,
-			   aclobj);
+		CHECK(parse(parser, str_buf(new_aclstr), allow_query,
+			aclobj));
 		break;
 	case acl_type_transfer:
-		result = parse(parser, str_buf(new_aclstr), allow_transfer,
-			   aclobj);
+		CHECK(parse(parser, str_buf(new_aclstr), allow_transfer,
+			aclobj));
 		break;
 	default:
 		/* This is a bug */
 		REQUIRE(Unhandled ACL type in acl_from_ldap == NULL);
 	}
 
-	if (result != ISC_R_SUCCESS) {
-		log_error(Failed to parse ACL (%s), aclstr);
-		goto cleanup;
-	}
-
 	CHECK(cfg_aclconfctx_create(mctx, aclctx));
-	CHECK(cfg_acl_fromconfig(aclobj, NULL, dns_lctx, aclctx, mctx, 0, acl));
+	CHECK(cfg_acl_fromconfig(aclobj, cctx, dns_lctx, aclctx, mctx, 0, acl));
 
 	*aclp = acl;
 	result = ISC_R_SUCCESS;
 
 cleanup:
+	if (result != ISC_R_SUCCESS)
+		log_error_r(%s ACL parsing failed: '%s',
+			type == acl_type_query ? query : transfer,
+			aclstr);
+
 	if (aclctx != NULL)
 		cfg_aclconfctx_detach(aclctx);
 	if (aclobj != NULL)
 		cfg_obj_destroy(parser, aclobj);
 	if (parser != NULL)
 		cfg_parser_destroy(parser);
+	if (cctx != NULL)
+		cfg_obj_destroy(parser_empty, cctx);
+	if (parser_empty != NULL)
+		cfg_parser_destroy(parser_empty);
 	str_destroy(new_aclstr);
 
 	return result;
-- 
1.7.11.7

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

Re: [Freeipa-devel] [PATCH 0114] Log name of the zone if zone cannot be created

2013-03-04 Thread Adam Tkac
On Wed, Feb 20, 2013 at 04:57:13PM +0100, Petr Spacek wrote:
 Hello,
 
   Log name of the zone if zone cannot be created.

Ack

 From 9f97b85ada8d67f5422dedfb936bc75cb02dfa2c Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Wed, 20 Feb 2013 16:55:30 +0100
 Subject: [PATCH] Log name of the zone if zone cannot be created.
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_helper.c | 17 +++--
  1 file changed, 11 insertions(+), 6 deletions(-)
 
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 71b96ce5eef2a249f7d16c448c70e2c2068d271d..19d9399ada3358c94b7d68f36b8e5147c4ae432d
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -750,12 +750,17 @@ create_zone(ldap_instance_t *ldap_inst, dns_name_t 
 *name, dns_zone_t **zonep)
   argv[1] = ldap_inst-db_name;
  
   result = dns_view_findzone(ldap_inst-view, name, zone);
 - if (result == ISC_R_SUCCESS) {
 - result = ISC_R_EXISTS;
 - log_error_r(failed to create new zone);
 - goto cleanup;
 - } else if (result != ISC_R_NOTFOUND) {
 - log_error_r(dns_view_findzone() failed);
 + if (result != ISC_R_NOTFOUND) {
 + char zone_name[DNS_NAME_FORMATSIZE];
 + dns_name_format(name, zone_name, DNS_NAME_FORMATSIZE);
 +
 + if (result == ISC_R_SUCCESS) {
 + result = ISC_R_EXISTS;
 + log_error_r(failed to create new zone '%s', 
 zone_name);
 + } else {
 + log_error_r(dns_view_findzone() failed while 
 + searching for zone '%s', zone_name);
 + }
   goto cleanup;
   }
  
 -- 
 1.7.11.7
 


-- 
Adam Tkac, Red Hat, Inc.

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


Re: [Freeipa-devel] [PATCH 0115] Add support for DNAME substitution

2013-03-04 Thread Adam Tkac
On Thu, Feb 21, 2013 at 04:27:03PM +0100, Petr Spacek wrote:
 On 21.2.2013 16:21, Petr Spacek wrote:
 Hello,
 
  Add support for DNAME substitution.
 
  https://fedorahosted.org/bind-dyndb-ldap/ticket/63
 
 
 And now the patch :-)

Ack

 From dc1215e8a82d3993f69436b4de9ff91ea16f4369 Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Thu, 21 Feb 2013 13:34:52 +0100
 Subject: [PATCH] Add support for DNAME substitution.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/63
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_driver.c | 22 +++---
  1 file changed, 19 insertions(+), 3 deletions(-)
 
 diff --git a/src/ldap_driver.c b/src/ldap_driver.c
 index 
 cde09ee8aa3c9332f3766a031030a95b0cff3229..9cae66b3950323221d3319649fc7b86ef25a5d68
  100644
 --- a/src/ldap_driver.c
 +++ b/src/ldap_driver.c
 @@ -457,7 +457,6 @@ cleanup:
   return result;
  }
  
 -/* XXX add support for DNAME redirection */
  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,
 @@ -469,6 +468,7 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t 
 *version,
   ldapdb_node_t *node = NULL;
   dns_rdatalist_t *rdlist = NULL;
   isc_boolean_t is_cname = ISC_FALSE;
 + isc_boolean_t is_dname = ISC_FALSE;
   isc_boolean_t is_delegation = ISC_FALSE;
   ldapdb_rdatalist_t rdatalist;
   unsigned int labels, qlabels;
 @@ -515,7 +515,20 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t 
 *version,
   continue;
   }
  
 - /* TODO: We should check for DNAME records right here */
 + /* RFC 6672 section 2.3.:
 +Unlike a CNAME RR, a DNAME RR redirects DNS names
 +subordinate to its owner name; the owner name of a DNAME
 +is not redirected itself. */
 + if (qlabels  dns_name_countlabels(traversename)) {
 + rdlist = NULL;
 + result = ldapdb_rdatalist_findrdatatype(rdatalist,
 + 
 dns_rdatatype_dname,
 + rdlist);
 + if (result == ISC_R_SUCCESS) {
 + is_dname = ISC_TRUE;
 + goto skipfind;
 + }
 + }
  
   /*
* Check if there is at least one NS RR. If yes and this is not 
 NS
 @@ -527,6 +540,7 @@ find(dns_db_t *db, dns_name_t *name, dns_dbversion_t 
 *version,
   if (dns_name_countlabels(db-origin) 
   dns_name_countlabels(traversename) 
   (options  DNS_DBFIND_GLUEOK) == 0) {
 + rdlist = NULL;
   result = ldapdb_rdatalist_findrdatatype(rdatalist,
   
 dns_rdatatype_ns,
   rdlist);
 @@ -582,7 +596,7 @@ found:
  skipfind:
   CHECK(dns_name_copy(traversename, foundname, NULL));
  
 - if (rdataset != NULL  type != dns_rdatatype_any) {
 + if (rdataset != NULL  (type != dns_rdatatype_any || is_dname)) {
   /* dns_rdatalist_tordataset returns success only */
   CHECK(clone_rdatalist_to_rdataset(ldapdb-common.mctx, rdlist,
 rdataset));
 @@ -600,6 +614,8 @@ skipfind:
   return DNS_R_DELEGATION;
   else if (is_cname)
   return DNS_R_CNAME;
 + else if (is_dname)
 + return DNS_R_DNAME;
   else
   return ISC_R_SUCCESS;
  
 -- 
 1.7.11.7
 

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


-- 
Adam Tkac, Red Hat, Inc.

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


Re: [Freeipa-devel] [PATCH 0116] Fix crash caused by invalid wildcard in update policy string

2013-03-04 Thread Adam Tkac
On Mon, Feb 25, 2013 at 03:28:57PM +0100, Petr Spacek wrote:
 Hello,
 
 Fix crash caused by invalid wildcard in update policy string.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/108
 
 Question:
 What we should do if update policy string contains an error?
 Should we disable all updates?
 Or let the old policy in place?
 I vote for disallowing all updates.

+1. In my opinion disallowing all updates is correct.

Ack for the patch.

 From 9265430d94cb4997188583b8e4c2befe7b28ba4b Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Mon, 25 Feb 2013 15:24:07 +0100
 Subject: [PATCH] Fix crash caused by invalid wildcard in update policy
  string.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/108
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/acl.c | 12 
  1 file changed, 12 insertions(+)
 
 diff --git a/src/acl.c b/src/acl.c
 index 
 c62a8cb9e867b658b65ce05a07fc31377b2356c2..f95cf431b6363d82085e9cfec7e6c1d6ddd45d7a
  100644
 --- a/src/acl.c
 +++ b/src/acl.c
 @@ -420,6 +420,18 @@ acl_configure_zone_ssutable(const char *policy_str, 
 dns_zone_t *zone)
   CHECK(get_fixed_name(stmt, name, fname));
   CHECK(get_types(mctx, stmt, types, n));
  
 + if (match_type == DNS_SSUMATCHTYPE_WILDCARD 
 + !dns_name_iswildcard(dns_fixedname_name(fname))) {
 + char name[DNS_NAME_FORMATSIZE];
 + dns_name_format(dns_fixedname_name(fname), name,
 + DNS_NAME_FORMATSIZE);
 + dns_zone_log(zone, ISC_LOG_ERROR,
 +  invalid update policy: 
 +  name '%s' is expected to be a wildcard,
 +  name);
 + CLEANUP_WITH(DNS_R_BADNAME);
 + }
 +
   result = dns_ssutable_addrule(table, grant,
 dns_fixedname_name(fident),
 match_type,
 -- 
 1.7.11.7
 


-- 
Adam Tkac, Red Hat, Inc.

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


Re: [Freeipa-devel] [PATCH 0112] Make log messages related to Kerberos more verbose

2013-03-04 Thread Adam Tkac
On Wed, Feb 27, 2013 at 04:21:16PM +0100, Petr Spacek wrote:
 On 12.2.2013 13:58, Petr Spacek wrote:
 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.
 
 Added explanatory error message for case where Kerberos context
 initialization failed.

Ack

 From 467a5d405f57e2277808c0b33b22480a3167abe4 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 | 38 +++---
  1 file changed, 23 insertions(+), 15 deletions(-)
 
 diff --git a/src/krb5_helper.c b/src/krb5_helper.c
 index 
 ffa6938d08a37d3470dd9184be2d8ab5c604afdf..25de7f80ee56a6a2c6c6591266edf621914a10b9
  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;
   }
 @@ -123,42 +123,46 @@ get_krb5_tgt(isc_mem_t *mctx, const char *principal, 
 const char *keyfile)
   }
  
   krberr = krb5_init_context(context);
 - if (krberr) {
 - log_error(Failed to init kerberos context);
 - return ISC_R_FAILURE;
 - }
 + /* This will blow up with older versions of Heimdal Kerberos, but
 +  * this kind of errors are not debuggable without any error message.
 +  * http://mailman.mit.edu/pipermail/kerberos/2013-February/018720.html 
 */
 + CHECK_KRB5(NULL, krberr, Kerberos context initialization failed);
  
   /* get credentials cache */
   CHECK(str_new(mctx, ccname));
   CHECK(str_sprintf(ccname, MEMORY:_ld_krb5_cc_%s, principal));
  
   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);
 

[Freeipa-devel] [PATCH] 0190 Fix installing server with external CA

2013-03-04 Thread Petr Viktorin
I did not test the external CA case when we merged DS instances some 
time ago, so it ended up broken. Here is a fix.



Our DsInstance class could only be initialized properly by calling 
create_instance or create_replica. Fr step 2, when the DS is not being 
installed, I gathered the common setup code to init_info, and called 
that. Ideally this will one day end up in __init__, but that's for a 
bigger refactoring.



https://fedorahosted.org/freeipa/ticket/3459

--
Petr³
From 56ccdc78264e15bc2b36982d51548240afd2419b Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Mon, 25 Feb 2013 17:15:23 +0100
Subject: [PATCH] Fix installing server with external CA

Reorganize ipa-server-instal so that DS (and NTP server) installation
only happens in step one.

Change CAInstance to behave correctly in two-step install.

Add an `init_info` method to DSInstance that includes common
attribute/sub_dict initialization from create_instance and create_replica.
Use it in ipa-server-install to get a properly configured DSInstance
for later tasks.

https://fedorahosted.org/freeipa/ticket/3459
---
 install/tools/ipa-server-install | 76 ++--
 ipaserver/install/cainstance.py  | 18 +-
 ipaserver/install/dsinstance.py  | 55 ++---
 3 files changed, 79 insertions(+), 70 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 15591071b0983511394a2cba3d829e1b84fe328e..57511c2147e52c78b1da894a7d7e83e9cb974acf 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -691,6 +691,15 @@ def main():
 sys.exit(1)
 cert = certdict[certissuer]
 
+# Figure out what external CA step we're in. See cainstance.py for more
+# info on the 3 states.
+if options.external_cert_file:
+external = 2
+elif options.external_ca:
+external = 1
+else:
+external = 0
+
 print ==
 print This program will set up the FreeIPA Server.
 print 
@@ -717,8 +726,9 @@ def main():
 print To accept the default shown in brackets, press the Enter key.
 print 
 
-# Make sure the 389-ds ports are available
-check_dirsrv(options.unattended)
+if external != 2:
+# Make sure the 389-ds ports are available
+check_dirsrv(options.unattended)
 
 if options.conf_ntp:
 try:
@@ -921,36 +931,43 @@ def main():
 except ipautil.CalledProcessError, e:
 root_logger.critical(failed to add DS group: %s % e)
 
-# Configure ntpd
-if options.conf_ntp:
-ipaclient.ntpconf.force_ntpd(sstore)
-ntp = ntpinstance.NTPInstance(fstore)
-if not ntp.is_configured():
-ntp.create_instance()
-
-# Create a directory server instance
-ds = dsinstance.DsInstance(fstore=fstore)
-
 if options.dirsrv_pin:
 [pw_fd, pw_name] = tempfile.mkstemp()
 os.write(pw_fd, options.dirsrv_pin)
 os.close(pw_fd)
-
-if options.dirsrv_pkcs12:
 pkcs12_info = (options.dirsrv_pkcs12, pw_name)
-try:
+
+if external != 2:
+# Configure ntpd
+if options.conf_ntp:
+ipaclient.ntpconf.force_ntpd(sstore)
+ntp = ntpinstance.NTPInstance(fstore)
+if not ntp.is_configured():
+ntp.create_instance()
+
+# Create a directory server instance
+ds = dsinstance.DsInstance(fstore=fstore)
+
+if options.dirsrv_pkcs12:
+try:
+ds.create_instance(realm_name, host_name, domain_name,
+dm_password, pkcs12_info,
+subject_base=options.subject,
+hbac_allow=not options.hbac_allow)
+finally:
+os.remove(pw_name)
+else:
 ds.create_instance(realm_name, host_name, domain_name,
-   dm_password, pkcs12_info,
-   subject_base=options.subject,
-   hbac_allow=not options.hbac_allow)
-finally:
-os.remove(pw_name)
+dm_password, self_signed_ca=options.selfsign,
+idstart=options.idstart, idmax=options.idmax,
+subject_base=options.subject,
+hbac_allow=not options.hbac_allow)
 else:
-ds.create_instance(realm_name, host_name, domain_name,
-   dm_password, self_signed_ca=options.selfsign,
-   idstart=options.idstart, idmax=options.idmax,
-   subject_base=options.subject,
-   hbac_allow=not options.hbac_allow)
+ds = dsinstance.DsInstance(fstore=fstore)
+ds.init_info(
+realm_name, host_name, domain_name, 

Re: [Freeipa-devel] [PATCH 0111] Automatically reload invalid zone after each change in zone data

2013-03-04 Thread Adam Tkac
On Tue, Feb 12, 2013 at 12:57:44PM +0100, Petr Spacek wrote:
 Hello,
 
 Automatically reload invalid zone after each change in zone data.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/102

Ack

 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. 
 

Re: [Freeipa-devel] [PATCH 0113] Periodically reconnect if Kerberos KDC is unavailable

2013-03-04 Thread Adam Tkac
On Fri, Feb 15, 2013 at 09:53:46AM +0100, Petr Spacek wrote:
 Hello,
 
 Periodically reconnect if Kerberos KDC is unavailable.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/100
 
 At the moment, Kerberos libraries contain a memory leak which will
 be triggered by periodical reconnecting implemented in this ticket.
 
 https://bugzilla.redhat.com/show_bug.cgi?id=90
 https://bugzilla.redhat.com/show_bug.cgi?id=911147
 
 -- 
 Petr^2 Spacek

Ack

 From bed25248a8c08c2f13027f1dcffecd5ffc60f2dd Mon Sep 17 00:00:00 2001
 From: Petr Spacek pspa...@redhat.com
 Date: Fri, 15 Feb 2013 09:42:41 +0100
 Subject: [PATCH] Periodically reconnect if Kerberos KDC is unavailable.
 
 https://fedorahosted.org/bind-dyndb-ldap/ticket/100
 
 Signed-off-by: Petr Spacek pspa...@redhat.com
 ---
  src/ldap_helper.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/src/ldap_helper.c b/src/ldap_helper.c
 index 
 491817813229f988161f3cedc6c83055040ad3ae..509b5019d1c4b8e330cf21aeefc7ef686e747c19
  100644
 --- a/src/ldap_helper.c
 +++ b/src/ldap_helper.c
 @@ -2286,7 +2286,7 @@ force_reconnect:
 str_buf(ldap_inst-krb5_keytab));
   UNLOCK(ldap_inst-kinit_lock);
   if (result != ISC_R_SUCCESS)
 - return result;
 + return ISC_R_NOTCONNECTED;
   }
  
   log_debug(4, trying interactive bind using %s mechanism,
 -- 
 1.7.11.7
 


-- 
Adam Tkac, Red Hat, Inc.

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


Re: [Freeipa-devel] [PATCH] 1088 Recover DNA ranges when deleting a master

2013-03-04 Thread Petr Viktorin

On 03/01/2013 11:57 PM, Rob Crittenden wrote:

Implement the design at http://freeipa.org/page/V3/Recover_DNA_Ranges


Could you add the link to the commit message?


Note that this required some new ACIs in cn=config which is not
replicated so the range-set commands won't work against older instances.
It should be gracefully handled though.


I think noting this in the man page would be helpful.

On new installs, the ACI on cn=Posix IDs,cn=Distributed Numeric 
Assignment Plugin,cn=plugins,cn=config is added before the entry itself. 
I didn't test everything as I didn't get the access.



It also doesn't work so well if you try it using a delegated
administrator, see ticket https://fedorahosted.org/freeipa/ticket/3480

rob


It should be possible to shrink existing ranges, i.e. ones that overlap 
with themselves:

$ ipa-replica-manage dnarange-show
vm-075.idm.lab.eng.brq.redhat.com: 140105-1401100499
$ ipa-replica-manage dnarange-set vm-075.idm.lab.eng.brq.redhat.com 
140105-1401100498

New range overlaps the DNA range on vm-075.idm.lab.eng.brq.redhat.com


freeipa-rcrit-1088-dnarange.patch



From 9a654b0b3730f8d9058dfbf25a93a58e1f4939e7 Mon Sep 17 00:00:00 2001

From: Rob Crittendenrcrit...@redhat.com
Date: Fri, 1 Mar 2013 15:02:14 -0500
Subject: [PATCH] Extend ipa-replica-manage to be able to manage DNA ranges.

Attempt to automatically save DNA ranges when a master is removed.
This is done by trying to find a master that does not yet define
a DNA on-deck range. If one can be found then the range on the deleted
master is added.

If one cannot be found then it is reported as an error.

Some validation of the ranges are done to ensure that they do overlap
an IPA local range and do not overlap existing DNA ranges configured
on other masters.


The patch adds some trailing whitespace, please trim it.
I also found some nitpicks, see below.



https://fedorahosted.org/freeipa/ticket/3321
---

[...]

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 
859809bf1c301913c3eb7fc92d1ed58675609b8c..6c0b45620dd2deabfc11ef2249b18205fb23b1fd
 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage

[...]


+def showDNARanges(hostname, master, realm, dirman_passwd, nextrange=False):


Style issue: please don't use camel case for functions.

[...]

+try:
+repl = replication.ReplicationManager(realm, hostname, dirman_passwd)
+except Exception, e:
+sys.exit(Connection failed: %s % ipautil.convert_ldap_error(e))


ipaldap should convert LDAP errors to IPA ones, there's no need to call 
convert_ldap_error. Same in other places.



+dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), repl.suffix)
+try:
+entries = repl.conn.get_entries(dn, repl.conn.SCOPE_ONELEVEL)
+except:


Don't use a bare except. Same in other places.

[...]

+def setDNARange(hostname, range, realm, dirman_passwd, next_range=False):
+
+Given a DNA range try to change it on the designated master.
+
+The range must not overlap with any other ranges and must be within
+one of the IPA local ranges as defined in cn=ranges.
+
+Setting an on-deck range of 0-0 removes the range.
+
+Return True if range was saved, False if not
+
+hostname: hostname of the master to set the range on
+range: The DNA range to set
+realm: our realm, needed to create a connection
+dirman_passwd: the DM password, needed to create a connection


Please also mention next_range.

[...]

+def range_intersection(s1, s2, r1, r2):
+overlap = xrange(max(s1, r1), min(s2, r2) + 1)
+return len(overlap)  0


That looks complicated. How about:
def ranges_intersect(s1, s2, r1, r2):
 return max(s1, r1) = min(s2, r2)

[...]

+# Normalize the range
+(dna_next, dna_max) = range.split('-', 1)


Can this be done before validate_range, so id doesn't have to be 
duplicated there?


[...]

+dn = DN(('cn', 'masters'), ('cn', 'ipa'), ('cn', 'etc'), 
ipautil.realm_to_suffix(realm))


I think you should use repl.suffix instead of generating it again.

[...]

+# Verify that this is within one of the IPA domain ranges.
+dn = DN(('cn','ranges'),('cn','etc'),repl.suffix)


Style issue: no spaces after commas


+try:
+entries = repl.conn.get_entries(dn, repl.conn.SCOPE_ONELEVEL,
+(objectclass=ipaDomainIDRange))
+except errors.NotFound, e:
+sys.exit('Unable to load IPA ranges: %s' % e.message)
+
+failed = 0
+for ent in entries:



This loops more than necessary and is somewhat hard to follow. Consider 
using for-else here:


for ...:
...
if okay:
break
else:
raise error


+entry_start = int(ent.single_value('ipabaseid'))
+entry_max = entry_start + int(ent.single_value('ipaidrangesize'))
+if not range_intersection(dna_next, dna_max, entry_start, 

Re: [Freeipa-devel] [PATCHES] 94-99 Read and use per-service PAC type

2013-03-04 Thread Sumit Bose
On Fri, Mar 01, 2013 at 08:58:34AM -0500, Simo Sorce wrote:
 On Fri, 2013-03-01 at 10:08 +0100, Martin Kosek wrote:
  On 03/01/2013 09:20 AM, Sumit Bose wrote:
   On Fri, Mar 01, 2013 at 08:33:51AM +0100, Martin Kosek wrote:
   On 02/28/2013 03:28 PM, Simo Sorce wrote:
   On Thu, 2013-02-28 at 13:02 +0100, Martin Kosek wrote:
   On 02/28/2013 12:42 PM, Sumit Bose wrote:
   On Thu, Feb 28, 2013 at 08:44:35AM +0100, Martin Kosek wrote:
   On 02/27/2013 06:48 PM, Sumit Bose wrote:
  
   Hi Sumit,
  
   This looks like a good idea and would prevent the magic default PAC 
   type, yes.
   Though I would not add this service-specific setting to global IPA 
   config object.
  
   I would rather like to see that in the service tree, for example as a
   configuration option of the service root which could be controlled 
   with
   serviceconfig-* commands (we already have dnsconfig, trustconfig), 
   e.g:
  
   # ipa serviceconfig-add-pacmap --service=nfs --pac-type=NONE
   # ipa serviceconfig-add-pacmap --service=cifs --pac-type=PAD
   # ipa serviceconfig-show
 Default PAC Map: nfs:NONE, cifs:PAD
  
   Are you thinking of having this in addition to the for-all-services
   default values in cn=ipaConfig,cn=etc or shall those be dropped? I 
   don't
   like the first case because then three different objects needs to be
   consulted to find out which is the right type. This wouldn't be an 
   issue
   for the plugin, but I think it is hard for the user/admin to follow.
  
   Hm, you are right.
  
  
   If the current defaults shall be dropped I think this is a major 
   change
   because it will require changes in the current CLI and WebUI which 
   will
   be visible to the users. I'm not against this change, I'm just 
   wondering
   if it is worth the effort for the next release?
  
   Maybe an argument to keep this is in global default is that the 
   settings
   are used for the host/*.* services as well which are in a different
   sub-tree of the cn=accounts container. Additionally in future we might
   want apply those setting to the user TGTs as well?
  
   Yeah, that was actually my point. That we are mixing service-specific 
   PAC
   rules to the global setting. Which may be shared with host/*.* 
   principals and
   user principals. This automatic PAC rules may require some designing 
   so that is
   is generally usable.
  
   I think putting everything in the general config is more understandable
   and discoverable. These per-service defaults are basically exceptions to
   the general rule so it make sense to keep everything together.
  
   Simo.
  
  
   Ok, if these are really just an exceptions to the general rule (and 
   there will
   not be too many of them), I think we can leave it in config entry. But 
   if we
   expect to have exceptions for other types of entries (hosts, users), I 
   think we
   should rather use something like service:nfs:NONE do distinguish this 
   exception.
  
   Question is, do we want to implement the interface and processing for 
   that in
   current Sumit's patches or do we use that is they are?
   
   I would like to update the patches so that they can handle the
   service:TYPE style entry and replace the current update code with just
   adding nfs:NONE to the global options. I will update the design page
   accordingly, too.
  
  Ok. If the update procedure shrinks just to adding service:nfs:NONE then 
  it'd
  be great.
 
 If we need to distinguish between service principals and user principals
 I would prefer rather use a special keyword for upns
 
 service: is redundant and I do not want here to be able to say
 upn:martin:NONE because per principal options are available on the
 principal object.
 
 I actually really do not see the need for changing the default just for
 user principals. If we are worried that one day we might want to really
 have upn:NONE, then let's use nfs/:NONE, host/:NONE etc... so one day we
 might add upn:NONE and the lack of / will tell us this is not a service
 named upn/foo.bar.baz but rather it means user principal names.
 
 However I do not see us ever really needing upn:NONE
 
   I would prefer if the enhancements needed for the CLI and WebUI can be
   covered by other/new tickets, but I'm happy to add the needed
   information to the design page too.
   
   bye,
   Sumit
  
  I am OK with adding the interface for this special exception later. In that
  case, a ticket + note in the design as you mentioned would be enough.
 
 Ack.
 
 Simo.
 

Please find attached a new version of the patches. 0095 i(updating) is
renamed and much simpler now. I opened
https://fedorahosted.org/freeipa/ticket/3484 to added the needed change
for 'service:TYPE' to CLI and WebUI. For the time being I've added
patch 0108 which simply allows 'nfs:NONE' as a type to make sure that it
is not deleted accidentally when e.g. using the WebUI. If you do not
like it it can simply be dropped, everything is working fine without it.

bye,
Sumit
From 

Re: [Freeipa-devel] [PATCH] 1088 Recover DNA ranges when deleting a master

2013-03-04 Thread Rob Crittenden

Petr Viktorin wrote:

[snip]


--- a/ipaserver/ipaldap.py
+++ b/ipaserver/ipaldap.py
@@ -1775,6 +1775,8 @@ class IPAdmin(LDAPClient):
  if removes:
  if not force_replace:
  modlist.append((ldap.MOD_DELETE, key, removes))
+elif new_values == []: # delete an empty value
+modlist.append((ldap.MOD_DELETE, key, removes))


I don't understand this change. AFAIK updateEntry/generateModList is
only used in ldapupdater now, and it's going away as soon as I can find
time to remove it. If you need to change it I'd like to know why.


Things may have changed since the refactoring, I did the development 
against the old code then did some sanity checking. I'll take another look.


This code lets one delete a single-valued attribute. If you want to 
delete a single-value attribute then force_replace will set so the 
delete will be lost.


rob

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


Re: [Freeipa-devel] [PATCH] 263 Web UI: configurable SID blacklists

2013-03-04 Thread Endi Sukma Dewata

On 2/18/2013 10:37 AM, Petr Vobornik wrote:

Added blacklists section, with ipantsidblacklistincoming and
ipantsidblacklistoutgoing multivalued textbox fields, into trust details
page.

https://fedorahosted.org/freeipa/ticket/3289


ACK.

--
Endi S. Dewata

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


[Freeipa-devel] [PATCH 0037] Add support for re-enrolling hosts using keytab

2013-03-04 Thread Tomas Babej

Hi,

A host that has been previously unenrolled and does not have its
host entry disabled or removed, can be re-enrolled using
a previously backed up keytab file.

A new option --keytab has been added to ipa-client-install. This
can be used to specify path to the keytab and can be used instead
of -p or -w options.

A new option -f has been added to ipa-join. It forces client to
join even if the host entry already exits. A new certificate,
ssh keys are generated, ipaUniqueID stays the same.

https://fedorahosted.org/freeipa/ticket/3374

Attaching a comparison between host entry states
(enrolled using principal and reenrolled using keytab).

Tomas

From e576009bb7a93daec1cbc4ef94785017f80b2756 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 26 Feb 2013 13:20:13 +0100
Subject: [PATCH] Add support for re-enrolling hosts using keytab

A host that has been previously unenrolled and does not have its
host entry disabled or removed, can be re-enrolled using
a previously backed up keytab file.

A new option --keytab has been added to ipa-client-install. This
can be used to specify path to the keytab and can be used instead
of -p or -w options.

A new option -f has been added to ipa-join. It forces client to
join even if the host entry already exits. A new certificate,
ssh keys are generated, ipaUniqueID stays the same.

https://fedorahosted.org/freeipa/ticket/3374
---
 ipa-client/ipa-install/ipa-client-install | 32 +--
 ipa-client/ipa-join.c | 14 +-
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 308c3f8d0ec39e1e7f048d37a34738bf6a4853e2..e78b36a3c386184dc0cb1c83d8169890e3fa75da 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -104,6 +104,8 @@ def parse_options():
   help=principal to use to join the IPA realm),
 basic_group.add_option(-w, --password, dest=password, sensitive=True,
   help=password to join the IPA realm (assumes bulk password unless principal is also set)),
+basic_group.add_option(-k, --keytab, dest=keytab, sensitive=True,
+  help=path to backed up keytab from previous enrollment),
 basic_group.add_option(-W, dest=prompt_password, action=store_true,
   default=False,
   help=Prompt for a password to join the IPA realm),
@@ -1691,7 +1693,11 @@ def install(options, env, fstore, statestore):
 except ipaclient.ntpconf.NTPConfigurationError:
 pass
 
-if options.unattended and (options.password is None and options.principal is None and options.prompt_password is False) and not options.on_master:
+if options.unattended and (options.password is None and
+   options.principal is None and
+   options.keytab is None and
+   options.prompt_password is False)\
+   and not options.on_master:
 root_logger.error(One of password and principal are required.)
 return CLIENT_INSTALL_ERROR
 
@@ -1985,12 +1991,34 @@ def install(options, env, fstore, statestore):
 else:
 stdin = sys.stdin.readline()
 
-(stderr, stdout, returncode) = run([kinit, principal], raiseonerr=False, stdin=stdin, env=env)
+(stderr, stdout, returncode) = run([kinit, principal],
+raiseonerr=False,
+stdin=stdin,
+env=env)
 if returncode != 0:
 root_logger.error(Kerberos authentication failed)
 root_logger.info(%s, stdout)
 print_port_conf_info()
 return CLIENT_INSTALL_ERROR
+elif options.keytab:
+join_args.append(-f)
+if os.path.exists(options.keytab):
+(stderr, stdout, returncode) = run(
+['/usr/bin/kinit','-k', '-t', options.keytab,
+'host/%s@%s' % (hostname, cli_realm)],
+env=env,
+raiseonerr=False)
+
+if returncode != 0:
+root_logger.error(Kerberos authentication failed 
+  using keytab: %s % options.keytab)
+root_logger.info(%s, stdout)
+print_port_conf_info()
+return CLIENT_INSTALL_ERROR
+else:
+root_logger.error(Keytab file could not be found: %s
+  % options.keytab)
+return CLIENT_INSTALL_ERROR