Re: [Freeipa-devel] [PATCH] 363-368 Configurable SID blacklists

2013-02-08 Thread Martin Kosek
On 02/08/2013 10:47 AM, Martin Kosek wrote:
 Sending patches according to RFE:
 http://www.freeipa.org/page/V3/Configurable_SID_Blacklists
 
 How this works:
 
 1) Trust is added, SID blacklist is filled with default list (by ipa-sam
 plugin). When SID blacklist attribute is missing (e.g. for current trusts),
 ipa-kdb will use the hardcoded list.
 
 # echo password | ipa trust-add MKAD2012.TEST --admin=Administrator 
 --password
 --
 Re-established trust to domain MKAD2012.TEST
 --
   Realm name: MKAD2012.TEST
   Domain NetBIOS name: MKAD2012
   Domain Security Identifier: S-1-5-21-xx-xx-xx
   SID blacklist incoming: S-1-0, S-1-1, S-1-2, S-1-3, S-1-5-1, S-1-5-2,
 S-1-5-3, S-1-5-4, S-1-5-5,
   S-1-5-6, S-1-5-7, S-1-5-8, S-1-5-9, S-1-5-10,
 S-1-5-11, S-1-5-12, S-1-5-13,
   S-1-5-14, S-1-5-15, S-1-5-16, S-1-5-17, S-1-5-18,
 S-1-5-19, S-1-5-20
   SID blacklist outgoing: S-1-0, S-1-1, S-1-2, S-1-3, S-1-5-1, S-1-5-2,
 S-1-5-3, S-1-5-4, S-1-5-5,
   S-1-5-6, S-1-5-7, S-1-5-8, S-1-5-9, S-1-5-10,
 S-1-5-11, S-1-5-12, S-1-5-13,
   S-1-5-14, S-1-5-15, S-1-5-16, S-1-5-17, S-1-5-18,
 S-1-5-19, S-1-5-20
   Trust direction: Two-way trust
   Trust type: Active Directory domain
   Trust status: Established and verified
 
 2) Incoming SID blacklist is updated (I added S-1-18-1 to the list as it is
 included in MS-PAC when I log from AD 2012):
 
 # ipa trust-mod MKAD2012.TEST --sid-blacklist-incoming
 S-1-0,S-1-1,S-1-2,S-1-3,S-1-5-1,S-1-5-2,S-1-5-3,S-1-5-4,S-1-5-5,S-1-5-6,S-1-5-7,S-1-5-8,S-1-5-9,S-1-5-10,S-1-5-11,S-1-5-12,S-1-5-13,S-1-5-14,S-1-5-15,S-1-5-16,S-1-5-17,S-1-5-18,S-1-5-19,S-1-5-20,S-1-18-1
 
 3) When I now login from AD2012 to my IPA machine, I get error message in
 krb5kdc.log about the filtered SID I configured in LDAP:
 
 ...
 Feb 08 04:11:33 ipa.linux.mkad2012.test krb5kdc[6493](Error): PAC filtering
 issue: SID [S-1-18-1] is not allowed from a trusted source and will be 
 excluded.
 ...
 
 NOTE:
 When coding and testing this feature I fixed several related bugs I found in
 ipa-kdb, see description of patches 363-365.
 
 Martin
 

I forgot to update ACI allowing Trust Admins to modify the blacklist. I also
added a validator for SIDs to help catching invalid SIDs.

Updated patches attached.

Martin
From b8e5b74aba5412c4eec254bdcf9b3b11ad555a09 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 7 Feb 2013 12:14:41 +0100
Subject: [PATCH 1/6] ipa-kdb: add sentinel for LDAPDerefSpec allocation

Without sentinel in place, ldap_create_deref_control_value executed
an invalid read in unallocated memory.
---
 daemons/ipa-kdb/ipa_kdb_common.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_common.c b/daemons/ipa-kdb/ipa_kdb_common.c
index 71df9634c4e25378494b165db9a9381f2b8fc206..247a38301295f159aced2ad7baa1534555d5fe0d 100644
--- a/daemons/ipa-kdb/ipa_kdb_common.c
+++ b/daemons/ipa-kdb/ipa_kdb_common.c
@@ -282,21 +282,22 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx,
 krb5_error_code kerr;
 int times;
 int ret;
-int c;
+int c, i;
 
 for (c = 0; deref_attr_names[c]; c++) {
 /* count */ ;
 }
 
-ds = calloc(c, sizeof(LDAPDerefSpec));
+ds = calloc(c+1, sizeof(LDAPDerefSpec));
 if (!ds) {
 return ENOMEM;
 }
 
-for (c = 0; deref_attr_names[c]; c++) {
-ds[c].derefAttr = deref_attr_names[c];
-ds[c].attributes = deref_attrs;
+for (i = 0; deref_attr_names[i]; i++) {
+ds[i].derefAttr = deref_attr_names[i];
+ds[i].attributes = deref_attrs;
 }
+ds[c].derefAttr = NULL;
 
 ret = ldap_create_deref_control_value(ipactx-lcontext, ds, derefval);
 if (ret != LDAP_SUCCESS) {
-- 
1.8.1

From f1f9add4ccf7a99fcabc98d898e600df73e911f0 Mon Sep 17 00:00:00 2001
From: Martin Kosek mko...@redhat.com
Date: Thu, 7 Feb 2013 13:17:28 +0100
Subject: [PATCH 2/6] ipa-kdb: avoid ENOMEM when all SIDs are filtered out

When all SIDs in info3.sids structure were filtered out, we tried
to talloc_realloc to zero memory size. talloc_realloc then returned
NULL pointer and filter_login_info returned with ENOMEM.

The code now rather frees the SID array and set info3.sidcount to
correct value.
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index ee1c6124f8d04cb10d091f11883834620c5c35ea..7307071a0d2562ca9ff5e4a5511ccdd6248ced4a 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -1288,11 +1288,21 @@ static krb5_error_code filter_logon_info(krb5_context context,
 } while (i  count);
 
 if (j != 0) {
-info-info-info3.sids = talloc_realloc(memctx, 

Re: [Freeipa-devel] [PATCH] 1085 cert-find command

2013-02-08 Thread Rob Crittenden

Jan Cholasta wrote:

Hi,

On 6.2.2013 00:44, Rob Crittenden wrote:

This adds a cert-find command for the dogtag backend.

Searches can be done by serial number, by subject, revocation reason,
issue date, notbefore, notafter and revocation dates.

I added some basic tests for this. I made it a separate test file
because the cert plugin tests do not use the declarative format and rely
on the selfsign backend by default.

rob



I have one design question: why do you emulate object interface with
Command plugins? Wouldn't it be better to add an actual Object plugin
and Method plugins? That way you would not have to duplicate the Object
bits for certs and as a result, the code would be cleaner and consistent
with the rest of our plugins.

Honza



I forget the details of the reasoning but IIRC it is because these 
commands aren't backed by LDAP. So the normal things we get out of 
Object don't apply.


rob

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


Re: [Freeipa-devel] [RFE] List of IPA realm domains

2013-02-08 Thread Ana Krivokapic
I have modified the design page as per suggestions posted on the list:

* There will 2 commands (not 3): realmdomains-show and realmdomains-mod
* Updates and Upgrades section updated to reflect the addition of new
LDAP container

http://www.freeipa.org/page/V3/Realm_Domains


On 02/06/2013 06:27 PM, Ana Krivokapic wrote:
 Hello,

 Below is a design page for ticket:
 https://fedorahosted.org/freeipa/ticket/2945.

 There are a couple of questions in the text.

 Thoughts, comments welcome!

 http://www.freeipa.org/page/V3/Realm_Domains



-- 
Regards,

Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.

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


Re: [Freeipa-devel] [PATCH] 89 Raise ValidationError on invalid CSV values

2013-02-08 Thread Martin Kosek
On 01/28/2013 03:30 PM, Petr Viktorin wrote:
 On 01/28/2013 10:29 AM, Jan Cholasta wrote:
 On 14.1.2013 12:56, Petr Viktorin wrote:
 On 01/09/2013 06:11 PM, Jan Cholasta wrote:
 Hi,

 this patch fixes https://fedorahosted.org/freeipa/ticket/3323.

 Honza


 The patch works well, but could you also add a test to ensure we don't
 regress in the future?



 Test added.

 
 ACK
 

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 0030] Add option to specify SID using domain name to idrange-add/mod

2013-02-08 Thread Alexander Bokovoy

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?

--
/ Alexander Bokovoy

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


[Freeipa-devel] [PATCH] 1086 handle no entries migrated

2013-02-08 Thread Rob Crittenden
The migration performance patch added a crash bug if no entries were 
successfully migrated. This should fix it.


rob
From 3724f1ee1fb4e231586d253c922c7dcf3e26f459 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Fri, 8 Feb 2013 09:42:34 -0500
Subject: [PATCH] Prevent a crash when no entries are successfully migrated.

It would fail in _update_default_group() because migrate_cnt wasn't
defined in context.

https://fedorahosted.org/freeipa/ticket/3386
---
 ipalib/plugins/migration.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index ccb484e4ae46826a29ffecae472a20198cadeb07..1ba21ece0cad7db35168b5ec088a267fcb337ef2 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -750,6 +750,7 @@ can use their Kerberos accounts.''')
 valid_gids = []
 invalid_gids = []
 migrate_cnt = 0
+context['migrate_cnt'] = 0
 for (dn, entry_attrs) in entries:
 context['migrate_cnt'] = migrate_cnt
 s = datetime.datetime.now()
-- 
1.8.1

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

Re: [Freeipa-devel] [PATCH] 356 Add trusconfig-show and trustconfig-mod commands

2013-02-08 Thread Alexander Bokovoy

On Fri, 01 Feb 2013, Martin Kosek wrote:

On 02/01/2013 03:55 PM, Alexander Bokovoy wrote:

On Tue, 29 Jan 2013, Martin Kosek wrote:

trust_output_params = (
@@ -482,3 +499,158 @@ api.register(trust_mod)
api.register(trust_del)
api.register(trust_find)
api.register(trust_show)
+
+
+_trust_type_option = (
+StrEnum('trust_type',
+cli_name='type',
+label=_('Trust type (ad for Active Directory, default)'),
+values=(u'ad',),
+default=u'ad',
+autofill=True,
+),
+)

We already have various trust type definitions in the same file. Maybe
it makes sense to unify those somehow?


Right, I unified those 2 separate trust_type option definitions.




+def get_dn(self, *keys, **kwargs):
+trust_type = kwargs.get('trust_type')
+if trust_type is None:
+raise errors.RequirementError(name='trust_type')
+if kwargs['trust_type'] == u'ad':

Perhaps better to define constants for the trust type values...


I changed it a bit and now it uses a dict instead. I think its now more general
and extensible.




+except ValueError:
+# The search is performed for groups with posixgroup objectclass
+# and not ipausergroup so that it can also match groups like
+# Default SMG Group which does not have this objectclass.

'Default SM_B_ Group'


Fixed.



Thanks for the unit tests too!



You are welcome! I also generated API.txt which I forgot to do last time.
Updated patch attached.

ACK for the code but please add more documentation (below).

Works like sharm. I tried also changing default fallback group to
some IPA group, then back to Default SMB Group and it worked well. Also
specifying non-existing group was noted and rejected.

Please make sure to mention in the design page magic value 'Default SMB
Group' and also that you can use any group with 'posixgroup'
objectclass, and that 'Default SMB Group' is not visible through normal
IPA tools.

We need to write better documentation (online help) for trustconfig-mod.
Basically, right now it helps no one to understand what is supposed to
be done here.

Once help is added, ACK.
--
/ Alexander Bokovoy

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