Re: [Freeipa-devel] [PATCH] 8 Add DNS service records for Windows

2011-11-28 Thread Alexander Bokovoy 
On Fri, 25 Nov 2011, Sumit Bose wrote:
 On Wed, Nov 23, 2011 at 05:33:42PM -0500, Rob Crittenden wrote:
  Alexander Bokovoy wrote:
  Hi Sumit,
  
  On Fri, 14 Oct 2011, Sumit Bose wrote:
  It would make more clear what is the default and that it is really
  optional setting -- I'm thinking from the perspective of maintenance
  of the code in future.
  
  Thank you for your comments, new version attached.
  Finally got to test it. ACK.
  
  
  pushed to master.
 
 Sorry, I think you pushed the first version and not -3- which was ACKed
 by Alexander.
Hm. Sumit, could you please rebase -3- on top of current master 
HEAD+(1)? 

I tried that briefly myself and failed. 

(1) Right now I'm also getting make-lint failing due to more strict 
PyLint in F16/Rawhide and those seems to be corect and also affect 
adtrustinstance.

I'm sending the patch shortly, so please rebase on top of it.

-- 
/ Alexander Bokovoy

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

[Freeipa-devel] [PATCH] 0037 make-lint fails on Fedora 16/Rawhide

2011-11-28 Thread Alexander Bokovoy 
Hi,

Attached are fixes for ldap.LDAPObject.add_s(self, dn, modlist) uses
which now don't pass 'make-lint' on Fedora 16/Rawhide.

-- 
/ Alexander Bokovoy
From dd866262c98be779a094a617975145e2fb1e0dd1 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Mon, 28 Nov 2011 14:21:17 +0200
Subject: [PATCH] Be more explicit when passing Entry class to
 ldap.LDAPObject.add_s()

ldap.LDAPObject.add_s(self, dn, modlist) requires two positional arguments.
We used to pass our Entry class which implements dictionary access that gives
proper way to pass the positional arguments, but PyLint in Fedora 16/Rawhide
became more strict about that and can't infer dictionary interface through
static checking.

Thus, we need to explicitly annotate dictionary passing with **entry syntax.
This has additional benefit to remind that we deal with multiple arguments here.
---
 ipaserver/install/adtrustinstance.py |6 +++---
 ipaserver/install/krbinstance.py |4 ++--
 ipaserver/install/replication.py |   14 +++---
 ipaserver/install/service.py |2 +-
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py 
b/ipaserver/install/adtrustinstance.py
index 
96f99dc9b76370c07b727e8e94ccabfe7afca074..2f6d75bb2921eca455e49cc970ddc68e21e1ce55
 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -99,7 +99,7 @@ class ADTRUSTInstance(service.Service):
 entry.setValues(objectclass, [account, simplesecurityobject])
 entry.setValues(uid, samba)
 entry.setValues(userPassword, self.smb_dn_pwd)
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(**entry)
 
 # And finally grant it permission to read NT passwords, we do not want
 # to support LM passwords so there is no need to allow access to them.
@@ -188,7 +188,7 @@ class ADTRUSTInstance(service.Service):
 entry = ipaldap.Entry(self.trust_dn)
 entry.setValues(objectclass, [nsContainer])
 entry.setValues(cn, trusts)
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(**entry)
 
 entry = ipaldap.Entry(self.smb_dom_dn)
 entry.setValues(objectclass, [sambaDomain, nsContainer])
@@ -196,7 +196,7 @@ class ADTRUSTInstance(service.Service):
 entry.setValues(sambaDomainName, self.netbios_name)
 entry.setValues(sambaSID, self.__gen_sid_string())
 #TODO: which MAY attributes do we want to set ?
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(**entry)
 
 def __write_smb_conf(self):
 self.fstore.backup_file(self.smb_conf)
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 
6ed38516223a56e723bcc9cf643ac2b362200da4..72c22d7971a024f19e06c2eae8d4129f6f60cf1e
 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -259,7 +259,7 @@ class KrbInstance(service.Service):
 entry.setValues(nsSaslMapFilterTemplate, 
'(krbPrincipalName=\\1@\\2)')
 
 try:
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(**entry)
 except ldap.ALREADY_EXISTS:
 root_logger.critical(failed to add Full Principal Sasl mapping)
 raise e
@@ -272,7 +272,7 @@ class KrbInstance(service.Service):
 entry.setValues(nsSaslMapFilterTemplate, '(krbPrincipalName=@%s)' % 
self.realm)
 
 try:
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(**entry)
 except ldap.ALREADY_EXISTS:
 root_logger.critical(failed to add Name Only Sasl mapping)
 raise e
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 
a139fd0fbe7168193dcfa6ba5f4d19f20d395c52..7aaf959a3904916101b0454b6df502e2996f2771
 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -227,7 +227,7 @@ class ReplicationManager(object):
 ent.setValues(sn, replication manager pseudo user)
 
 try:
-conn.add_s(ent)
+conn.add_s(**ent)
 except ldap.ALREADY_EXISTS:
 conn.modify_s(dn, [(ldap.MOD_REPLACE, userpassword, pw)])
 pass
@@ -277,7 +277,7 @@ class ReplicationManager(object):
 entry.setValues('nsds5replicabinddn', [replica_binddn])
 entry.setValues('nsds5replicalegacyconsumer', off)
 
-conn.add_s(entry)
+conn.add_s(**entry)
 
 def setup_changelog(self, conn):
 dn = cn=changelog5, cn=config
@@ -287,7 +287,7 @@ class ReplicationManager(object):
 entry.setValues('cn', changelog5)
 entry.setValues('nsslapd-changelogdir', dirpath)
 try:
-conn.add_s(entry)
+conn.add_s(**entry)
 except ldap.ALREADY_EXISTS:
 return
 
@@ -310,7 +310,7 @@ class ReplicationManager(object):
 entry.setValues('nsmultiplexorbinddn',

Re: [Freeipa-devel] [PATCH] 0037 make-lint fails on Fedora 16/Rawhide

2011-11-28 Thread Alexander Bokovoy 
On Mon, 28 Nov 2011, Alexander Bokovoy wrote:

 Hi,
 
 Attached are fixes for ldap.LDAPObject.add_s(self, dn, modlist) uses
 which now don't pass 'make-lint' on Fedora 16/Rawhide.
 
 -- 
 / Alexander Bokovoy

 From dd866262c98be779a094a617975145e2fb1e0dd1 Mon Sep 17 00:00:00 2001
 From: Alexander Bokovoy aboko...@redhat.com
 Date: Mon, 28 Nov 2011 14:21:17 +0200
 Subject: [PATCH] Be more explicit when passing Entry class to
  ldap.LDAPObject.add_s()
 
 ldap.LDAPObject.add_s(self, dn, modlist) requires two positional arguments.
 We used to pass our Entry class which implements dictionary access that gives
 proper way to pass the positional arguments, but PyLint in Fedora 16/Rawhide
 became more strict about that and can't infer dictionary interface through
 static checking.
 
 Thus, we need to explicitly annotate dictionary passing with **entry syntax.
 This has additional benefit to remind that we deal with multiple arguments 
 here.
Self-NACK, doesn't really work this way. I'm still looking at better 
approach but intermediate solution is to use pylint hints.

Conservative patch is attached.

-- 
/ Alexander Bokovoy
From 4eb6c88392f43d57943426968a98fca1a91f302c Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy aboko...@redhat.com
Date: Mon, 28 Nov 2011 17:02:11 +0200
Subject: [PATCH] Disable pylint false positive when passing Entry class to
 ldap.LDAPObject.add_s()

ldap.LDAPObject.add_s(self, dn, modlist) requires two positional arguments.
We used to pass our Entry class which implements dictionary access that gives
proper way to pass the positional arguments, but PyLint in Fedora 16/Rawhide
became more strict about that and can't infer dictionary interface through
static checking.

Thus, we need to explicitly mark this error as false positive as PyLint is 
unable to
guess that.

This has additional benefit to remind that we deal with multiple arguments here.
---
 ipaserver/install/adtrustinstance.py |6 +++---
 ipaserver/install/krbinstance.py |4 ++--
 ipaserver/install/service.py |2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/ipaserver/install/adtrustinstance.py 
b/ipaserver/install/adtrustinstance.py
index 
96f99dc9b76370c07b727e8e94ccabfe7afca074..8a9e3d9375f00fef9eb7aafb0a22e88cd09d7270
 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -99,7 +99,7 @@ class ADTRUSTInstance(service.Service):
 entry.setValues(objectclass, [account, simplesecurityobject])
 entry.setValues(uid, samba)
 entry.setValues(userPassword, self.smb_dn_pwd)
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(entry) #pylint: disable=E1120
 
 # And finally grant it permission to read NT passwords, we do not want
 # to support LM passwords so there is no need to allow access to them.
@@ -188,7 +188,7 @@ class ADTRUSTInstance(service.Service):
 entry = ipaldap.Entry(self.trust_dn)
 entry.setValues(objectclass, [nsContainer])
 entry.setValues(cn, trusts)
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(entry) #pylint: disable=E1120
 
 entry = ipaldap.Entry(self.smb_dom_dn)
 entry.setValues(objectclass, [sambaDomain, nsContainer])
@@ -196,7 +196,7 @@ class ADTRUSTInstance(service.Service):
 entry.setValues(sambaDomainName, self.netbios_name)
 entry.setValues(sambaSID, self.__gen_sid_string())
 #TODO: which MAY attributes do we want to set ?
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(entry) #pylint: disable=E1120
 
 def __write_smb_conf(self):
 self.fstore.backup_file(self.smb_conf)
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 
6ed38516223a56e723bcc9cf643ac2b362200da4..193dacd2b406332c83ed8b3b86cd7b3cff20471a
 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -259,7 +259,7 @@ class KrbInstance(service.Service):
 entry.setValues(nsSaslMapFilterTemplate, 
'(krbPrincipalName=\\1@\\2)')
 
 try:
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(entry) #pylint: disable=E1120
 except ldap.ALREADY_EXISTS:
 root_logger.critical(failed to add Full Principal Sasl mapping)
 raise e
@@ -272,7 +272,7 @@ class KrbInstance(service.Service):
 entry.setValues(nsSaslMapFilterTemplate, '(krbPrincipalName=@%s)' % 
self.realm)
 
 try:
-self.admin_conn.add_s(entry)
+self.admin_conn.add_s(entry) #pylint: disable=E1120
 except ldap.ALREADY_EXISTS:
 root_logger.critical(failed to add Name Only Sasl mapping)
 raise e
diff --git a/ipaserver/install/service.py b/ipaserver/install/service.py
index 
249727b15b6f3c879336c36d43410fc7234d9d9d..6bc70fa2e661a01b78a0a74490c56dabfce5
 100644
--- a/ipaserver/install/service.py
+++ b/ipaserver/install/service.py

Re: [Freeipa-devel] [PATCH] 906 Add SELinux user mapping framework.

2011-11-28 Thread Alexander Bokovoy 
On Thu, 24 Nov 2011, Alexander Bokovoy wrote:
 On Wed, 23 Nov 2011, Rob Crittenden wrote:
  This will allow one to define what SELinux context a given user gets
  on a given machine. A rule can contain a set of users and hosts or it
  can point to an existing HBAC rule that defines them.
  
  https://fedorahosted.org/freeipa/ticket/755
 I read through the patch, will need to test it later this week. I 
 basically have two minor points:
 
 1. Split charachter in the SE Linux user map order. 
  +
  + Define SELinux user map order:
  +   ipa config-mod 
  --ipaselinuxusermaporder='guest_u:s0$xguest_u:s0$user_u:s0-s0:c0.c1023$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023'
   )
 $ can be considered 'active' character in all shells in a sense it 
 changes treatment of following characters from the shell perspective 
 and therefore will always require shielding from the shell's 
 influence. This increases likelyhood of error from a user side.
 
 Maybe / would be more neutral character? 
 
 As you said on IRC, people might have religious feeling about 
 separators but tricking users into always thinking about 
 escaping/single quoting is equally bad.
 
 2. We have two possible ways to address named properties in MagicDict 
 and NameSpace objects -- through explicit attribute use and through 
 the dictionary key. I guess for the cases when we know the attribute 
 name in advance, it would perhaps be preferrable to use the former 
 style:
 
  +def pre_callback(self, ldap, dn, *keys, **options):
  +kw = dict(seealso=dn)
  +_entries = api.Command['selinuxusermap_find'](None, **kw)
 this would be 
_entries = api.Command.selinuxusermap_find(None, **kw)
 
 Other than those two minor points, the patch looks very good. I'm 
 going to give it a run on Friday.
I tested the patch and it works for me on a new install. On upgrade of 
existing installation I've got few errors during run of 
ipa-ldap-updater for  SELinux schema changes. Unfortunately, didn't 
save the log as it was 2.1 - 2.99 upgrade as well.

-- 
/ Alexander Bokovoy

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

[Freeipa-devel] [PATCH] Make pwd-extop aware of new ipaNTHash attribute

2011-11-28 Thread Sumit Bose 
Hi,

in IPAv3 we introduce a new attribute 'ipaNTHash' to store the NT hash.
Currently the plugin handling the change password extended operation
only sets and updates 'sambaNTPassword'. This patch add support for the
new attribute without removing the support for the old one.

bye,
Sumit
From 68d66eba4e31a314242322471dbfe698f4493737 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Thu, 24 Nov 2011 18:38:38 +0100
Subject: [PATCH] Make pwd-extop aware of new ipaNTHash attribute

---
 .../ipa-pwd-extop/ipa_pwd_extop.c  |4 +-
 daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h   |7 ++-
 .../ipa-pwd-extop/ipapwd_common.c  |   38 +---
 .../ipa-pwd-extop/ipapwd_encoding.c|   22 -
 .../ipa-pwd-extop/ipapwd_prepost.c |   47 +--
 5 files changed, 89 insertions(+), 29 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c 
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
index 
65c5834595f89aee8502347311f247be058c3416..82acc49dd0a48bea9b560b882966e996ae5c4775
 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipa_pwd_extop.c
@@ -156,7 +156,7 @@ static int ipapwd_chpwop(Slapi_PBlock *pb, struct 
ipapwd_krbcfg *krbcfg)
Slapi_Value *objectclass=NULL;
char *attrlist[] = {*, passwordHistory, NULL };
struct ipapwd_data pwdata;
-   int is_krb, is_smb;
+   int is_krb, is_smb, is_ipant;
 char *principal = NULL;
 
/* Get the ber value of the extended operation */
@@ -365,7 +365,7 @@ parse_req_done:
 }
 
 rc = ipapwd_entry_checks(pb, targetEntry,
-   is_root, is_krb, is_smb,
+   is_root, is_krb, is_smb, is_ipant,
SLAPI_USERPWD_ATTR, SLAPI_ACL_WRITE);
 if (rc) {
goto free_and_return;
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h 
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
index 
787ed500a080674d4a8e1002468006b020eb1578..0edd2dcad580b25d108a762bd78271b3d8244bc5
 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd.h
@@ -113,7 +113,7 @@ struct ipapwd_krbcfg {
 };
 
 int ipapwd_entry_checks(Slapi_PBlock *pb, struct slapi_entry *e,
-int *is_root, int *is_krb, int *is_smb,
+int *is_root, int *is_krb, int *is_smb, int *is_ipant,
 char *attr, int access);
 int ipapwd_gen_checks(Slapi_PBlock *pb, char **errMesg,
   struct ipapwd_krbcfg **config, int check_flags);
@@ -144,8 +144,9 @@ void ipapwd_keyset_free(struct ipapwd_keyset **pkset);
 
 int ipapwd_gen_hashes(struct ipapwd_krbcfg *krbcfg,
   struct ipapwd_data *data, char *userpw,
-  int is_krb, int is_smb, Slapi_Value ***svals,
-  char **nthash, char **lmhash, char **errMesg);
+  int is_krb, int is_smb, int is_ipant,
+  Slapi_Value ***svals, char **nthash, char **lmhash,
+  Slapi_Value ***ntvals, char **errMesg);
 
 /* from ipapwd_prepost.c */
 int ipapwd_ext_init(void);
diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_common.c 
b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_common.c
index 
9e203be2763b13328e2d392c76e8545ba7ab549a..c36189987f785de8e8e97737554b854539b83ea2
 100644
--- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/ipapwd_common.c
@@ -498,7 +498,7 @@ done:
 /*==Common-public-functions=*/
 
 int ipapwd_entry_checks(Slapi_PBlock *pb, struct slapi_entry *e,
-int *is_root, int *is_krb, int *is_smb,
+int *is_root, int *is_krb, int *is_smb, int *is_ipant,
 char *attr, int acc)
 {
 Slapi_Value *sval;
@@ -535,6 +535,15 @@ int ipapwd_entry_checks(Slapi_PBlock *pb, struct 
slapi_entry *e,
 *is_smb = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS, 
sval);
 slapi_value_free(sval);
 
+sval = slapi_value_new_string(ipaNTUserAttrs);
+if (!sval) {
+rc = LDAP_OPERATIONS_ERROR;
+goto done;
+}
+*is_ipant = slapi_entry_attr_has_syntax_value(e, SLAPI_ATTR_OBJECTCLASS,
+  sval);
+slapi_value_free(sval);
+
 rc = LDAP_SUCCESS;
 
 done:
@@ -765,14 +774,17 @@ int ipapwd_SetPassword(struct ipapwd_krbcfg *krbcfg,
 int ret = 0;
 Slapi_Mods *smods = NULL;
 Slapi_Value **svals = NULL;
+Slapi_Value **ntvals = NULL;
 Slapi_Value **pwvals = NULL;
 struct tm utctime;
 char timestr[GENERALIZED_TIME_LENGTH+1];
 char *lm = NULL;
 char *nt = NULL;
 int is_smb = 0;
+int is_ipant = 0;
 int

Re: [Freeipa-devel] [PATCH] 908 make some fields required

2011-11-28 Thread Endi Sukma Dewata 

On 11/28/2011 12:09 PM, Rob Crittenden wrote:

Some attributes in the framework were not marked as required even though
they are in the schema. These are typically computed values and I think
the intention was to not prompt for them. This has the side-effect of
them showing as not required in the UI even though they are.

Since they all have default values set they won't be prompted for on the
CLI so there won't be any practical changes.


This patch fixes the problem with required attributes in DNS Zones and 
cn, uidNumber, and gidNumber in Users. The UI now shows the required 
indicators for these attributes. So this patch is ACKed.


Some problems mentioned in ticket #2015 are still present:

1. Removing the homeDirectory from a user fails because it's required by 
posixAccount.


2. Removing the gidNumber from a group fails because it's required by 
posixGroup.


3. Removing config attributes listed in the ticket generates internal 
error. I think at least the server should return a proper error message. 
The required indicator can be hard-coded in the UI if necessary.


--
Endi S. Dewata

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

Re: [Freeipa-devel] [PATCH] 152 Enable automember for upgraded servers

2011-11-28 Thread Rob Crittenden 

Nathan Kinder wrote:

On 11/04/2011 02:35 PM, Nathan Kinder wrote:

On 11/04/2011 02:26 PM, Martin Kosek wrote:

On Fri, 2011-11-04 at 14:04 -0700, Nathan Kinder wrote:

On 11/04/2011 02:02 PM, Rob Crittenden wrote:

Martin Kosek wrote:

automember functionality is depends on predefined data is in LDAP.
Since we add it for fresh installs only, automember cannot be used
for upgraded servers. Make sure that automember LDAP data is added
during upgrade too.

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

I think you need that automember schema as well. Can you check with
the 389-ds team to see if their upgrade script automatically adds new
schema or if we have to handle that ourselves?

The new automember schema should be added by 'setup-ds.pl -u', so I
don't expect you need to do anything around schema in FreeIPA.

Nathan, when is the setup-ds.pl -u executed? When the dirsrv rpm is
updated, just like FreeIPA runs ipa-ldap-updater in rpm update %post? Or
does it have to be run manually?

It is run in the the %posttrans stage for 389-ds-base.

I am asking because the schema problem seems like the root cause that
one user has here (the last post):

https://bugzilla.redhat.com/show_bug.cgi?id=746589

There should be a
'/etc/dirsrv/slapd-instance/schema/10automember-plugin.ldif' file if
the proper version
of 389-ds-base is being used and if 'setup-ds.pl -u' successfully
updated the schema. There should also be
a '/etc/dirsrv/schema/10automember-plugin.ldif' file present
regardless of 'setup-ds.pl -u' having run
successfully.

I just tested running 'setup-ds.pl -u' manually with a master build of
389-ds-base, and there is a bug that is preventing the updates from
being applied. I logged the following bug for this:

https://bugzilla.redhat.com/show_bug.cgi?id=751495

The fix is a one-liner, and I believe Rich is working on getting a fixed
build out ASAP.


ACK, works for me.

rob

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

Re: [Freeipa-devel] [PATCH] #2122 Fix PAC re-signing

2011-11-28 Thread Simo Sorce 
On Thu, 2011-11-24 at 13:54 +0100, Sumit Bose wrote:
 I think I found two issues which should be fixed by the following
 patch:
  - krb5_pac_add_buffer() expects krb5_pac and not krb5_pac * as a
 second
argument

good catch

  - your patch copies all buffers, including the checksums, which you
wanted to remove from the new pac

also good catch

 With this patch applied I do not see any errors in the krb5kdc.log and
 ssh from AD to IPA server works.

I still haven't solved my ssh issue from an AD client to IPA, but I get
a ticket on the client now, so it must be unrelated stuff.

I have prepared a patch which have a slightly different version of your
fixes.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
From eaf06f520acbe34972b711cb6e42ae8f8b22bdd4 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Tue, 22 Nov 2011 18:03:10 -0500
Subject: [PATCH] ipa-kdb: Support re-signing PAC with different checksum

Fixes: https://fedorahosted.org/freeipa/ticket/2122
---
 daemons/ipa-kdb/ipa_kdb_mspac.c |   54 +-
 1 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 3d4975e73e2402bda3065f6f90bf8bf7c2e4f9c5..cce1ca9060f0e03d525bb87d843bdd5811e9d20b 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -50,6 +50,12 @@ krb5int_find_authdata(krb5_context context,
 #define krb5_find_authdata krb5int_find_authdata
 #endif
 
+#ifndef KRB5_PAC_SERVER_CHECKSUM
+#define KRB5_PAC_SERVER_CHECKSUM 6
+#endif
+#ifndef KRB5_PAC_PRIVSVR_CHECKSUM
+#define KRB5_PAC_PRIVSVR_CHECKSUM 6
+#endif
 
 static char *user_pac_attrs[] = {
 objectClass,
@@ -552,6 +558,12 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
 {
 krb5_authdata **authdata = NULL;
 krb5_error_code kerr;
+krb5_ui_4 *buffer_types = NULL;
+size_t num_buffers;
+krb5_pac old_pac = NULL;
+krb5_pac new_pac = NULL;
+krb5_data data;
+size_t i;
 
 /* find the existing PAC, if present */
 kerr = krb5_find_authdata(context, tgt_auth_data, NULL,
@@ -573,16 +585,54 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
 kerr = krb5_pac_parse(context,
   authdata[0]-contents,
   authdata[0]-length,
-  pac);
+  old_pac);
 if (kerr) {
 goto done;
 }
 
-kerr = krb5_pac_verify(context, *pac, authtime,
+kerr = krb5_pac_verify(context, old_pac, authtime,
 client_princ, krbtgt_key, NULL);
+if (kerr) {
+goto done;
+}
+
+/* extract buffers and rebuilt pac from scratch so that when re-signing
+ * with a different cksum type does not cause issues due to mismatching
+ * signature buffer lengths */
+kerr = krb5_pac_init(context, new_pac);
+if (kerr) {
+goto done;
+}
+
+kerr = krb5_pac_get_types(context, old_pac, num_buffers, buffer_types);
+if (kerr) {
+goto done;
+}
+
+for (i = 0; i  num_buffers; i++) {
+if (buffer_types[i] == KRB5_PAC_SERVER_CHECKSUM ||
+buffer_types[i] == KRB5_PAC_PRIVSVR_CHECKSUM) {
+continue;
+}
+kerr = krb5_pac_get_buffer(context, old_pac,
+buffer_types[i], data);
+if (kerr == 0) {
+kerr = krb5_pac_add_buffer(context, new_pac,
+buffer_types[i], data);
+}
+krb5_free_data_contents(context, data);
+if (kerr) {
+krb5_pac_free(context, new_pac);
+goto done;
+}
+}
+
+*pac = new_pac;
 
 done:
 krb5_free_authdata(context, authdata);
+krb5_pac_free(context, old_pac);
+free(buffer_types);
 return kerr;
 }
 
-- 
1.7.7.1

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

Re: [Freeipa-devel] [PATCH] 157 Add --delattr option to complement --setattr/--addattr

2011-11-28 Thread Rob Crittenden 

Martin Kosek wrote:

On Sat, 2011-11-05 at 13:43 +0100, Martin Kosek wrote:

On Fri, 2011-11-04 at 09:23 -0400, Rob Crittenden wrote:

Martin Kosek wrote:

Add a --delattr option to round out multi-valued attribute manipulation.
The new option is be available for all LDAPUpdate based commands.

--delattr is evaluated last, it can remove any value present either
in --addattr/--setattr options or stored in LDAP.

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


Should --delattr raise an error if the value doesn't exist?

I think it probably should.

rob


You are right, it would be more expected behavior. I fixed that. In the
process of implementing the change I found that current --*attr
processing is not good, so I refactored it completely to one function
available for all baseldap.py commands.

In the process I found out that we don't have any common class for all
baseldap.py commands and the result is BaseLDAPCommand which can now
handle attribute processing and provide it to other LDAP commands.

And I also fixed one group unit test. Now all unit tests were OK.

Martin


I rebased the patch (API.txt format was changed) and tested that it
still works ok.

Martin


ACK but some of the comments need to be cleaned up before pushing. It 
will also require a minor rebase in frontend.py.


process_attr_options() should probably read:

Process all --setattr, --addattr, and --delattr options and add the 
resulting value to the list of attributes. --setattr is processed first,

then --addattr and finally --delattr.

When --setattr is not used then the original LDAP object is looked up 
(of course, not when dn is None) and the changes are applied to old 
object values.


...
AttrValueNotFound exception may be raised when an attribute value was 
not found either by --setattr and --addattr nor in existing LDAP object.


rob

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

Re: [Freeipa-devel] [PATCH] 161 Make ipa-server-install clean after itself

2011-11-28 Thread Rob Crittenden 

Martin Kosek wrote:

How to test:

1) ipa-server-install -p secret123 -a secret123 --hostname
ipa.example.com
2) Continue in interactive wizard until IP address is requested (as
ipa.example.com cannot be resolved)
3) When it is entered and ipa-server-install gives this output:

# ipa-server-install -p kokos123 -a kokos123 --hostname ipa.example.com
--setup-dns
...
Please confirm the domain name [example.com]:

Unable to resolve IP address for host name
Please provide the IP address to be used for this host name: 10.16.78.93
Adding [10.16.78.93 ipa.example.com] to your /etc/hosts file
...

hit CTRL+C to quit from the installation.
4) Try running ipa-server-install again. It will fail as /etc/hosts has
been changed - this patch fixes it.


ACK. Needs a rebase.

rob

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