Re: [Freeipa-devel] [PATCH 0069] Adds 389DS plugin to enforce UUID token IDs

2014-09-23 Thread thierry bordaz

On 09/22/2014 09:28 PM, Simo Sorce wrote:

On Mon, 22 Sep 2014 21:21:04 +0200
Martin Kosek mko...@redhat.com wrote:


On 09/22/2014 04:55 PM, Simo Sorce wrote:

On Mon, 22 Sep 2014 10:02:01 -0400
Nathaniel McCallum npmccal...@redhat.com wrote:


On Mon, 2014-09-22 at 09:50 -0400, Simo Sorce wrote:

On Mon, 22 Sep 2014 10:34:54 +0200
Martin Kosek mko...@redhat.com wrote:


On 09/22/2014 09:33 AM, thierry bordaz wrote:

Hello Nathaniel,

 Just a remark, in is_token if the entry is
objectclass=ipaToken it returns without freeing the
'objectclass' char array.

 thanks
 thierry

On 09/21/2014 09:07 PM, Nathaniel McCallum wrote:

Users that can rename the token (such as admins) can also
create non-UUID token names.

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

NOTE: this patch is an alternate approach to my patch 0065.
This version has two main advantages compared to 0065:
1. Permissions are more flexible (not tied to the admin group).
2. Enforcement occurs at the DS-level

It should also be noted that this patch does not enforce UUID
randomness, only syntax. Users can still specify a token ID so
long as it is in UUID format.

Nathaniel

I am still thinking we may be overcomplicating it. Why cannot we
use the similar UUID generation mechanism as we do for SUDO
commands for example:

# ipa sudocmd-add barbar --all --raw
---
Added Sudo Command barbar
---
dn:
ipaUniqueID=3a96de14-4232-11e4-9d66-001a4a104ec9,cn=sudocmds,cn=sudo,dc=mkosek-fedora20,dc=test
sudocmd: barbar
ipaUniqueID: 3a96de14-4232-11e4-9d66-001a4a104ec9
objectClass: ipasudocmd
objectClass: ipaobject

It lets DS generaterename the object's DN when it finds out that
the ipaUniqueID is set to autogenerate (in baseldap.py). We
could let DS generate the UUID and only add the autogenerate
keyword in otptoken-add command.

For authorization, we can simply allow users to only add tokens
with autogenerate ID, see my example here:

http://www.redhat.com/archives/freeipa-devel/2014-September/msg00438.html

Admin's or special privilege-owners would have more generous ACI
allowing other values than just autogenerate.

IMO, then the whole ipatoken-add mechanism would be a lot simpler
and we would not need a special DS plugin (unless we want regular
users to generate their own UUIDs instead of letting IPA DS to
generate it
- which I do not think is the case).

Good point Martin.

This is the avenue I first pursued. The problem is that the client
has no way to look up the DN after the entry is added. In the case
of sudocmd-add, the lookup is performed using the sudocmd
attribute (see sudocmd.get_dn()). We have no similar attribute in
this case and the lookup cannot be performed.

Well in theory we could search with creatorName and createTimestamp
set to the user's own DN and a range a few seconds in the past ...
It is not robust if you add multiple tokens at the same time, but
would this be a concern for user created tokens ?

Simo.

Ugh, no offense, but this approach sounds really ugly :-) I will wait
for Thierry or Ludwig's assessment, but I still think there is either
existing control for the modify operation to return an updated entry
or we could create one as you suggest down the thread - as this may
be useful for other use cases too.

See following mails, the right approach is to use the POST READ control.

Simo.


Hello,

Yes that is correct. Mark implemented it with 
https://fedorahosted.org/389/ticket/589 and is present since 1.3.2.18 (F20)


[root@vm-046 ~]# ldapsearch -LLL -h localhost -p 389 -x -b  -s base 
supportedcontrol | grep 1.3.6.1.1.13.2

supportedcontrol: 1.3.6.1.1.13.2
[root@vm-046 ~]# uname -a
Linux vm-046.idm.lab.bos.redhat.com 3.13.10-200.fc20.x86_64 #1 SMP Mon 
Apr 14 20:34:16 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux


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

Re: [Freeipa-devel] [PATCHES 247-259] ID views - management part

2014-09-23 Thread Petr Viktorin

On 09/16/2014 01:21 PM, Tomas Babej wrote:

Petr,

thanks for the review, your input is, as always, much to the point.

My responses are inline. Also, I'm attaching a new patchset, rebased on
master, please, have a look at that.

Most of the patches have at least minor changes, since I rebased another
~15 patches into this patchset, and there are additional 9 patches on
top. Patch 254 was deprecated, as we decided to go with more explicit
way of having two separate commands for user and group ID overrides.

Also, the test suite (around 80 tests) for ID views is attached - patch
270. It depends on patch 269, sent in separate thread.

Tomas

On 09/10/2014 03:10 PM, Petr Viktorin wrote:

On 08/01/2014 12:30 PM, Tomas Babej wrote:

Hi,

the following set of patches implements the ID view creation and
management of views and ID overrides in IPA.

Pending questions:
1.) The patch 253 implements basic managed permissions for ID views and
ID overrides. Do we want to have a separate permission for assigning ID
views?
2.) Performance: idview-apply command takes ~100 seconds for hostgroups
with 1000 member hosts. I am able to lower that by 20-30% using raw ldap
calls, is bypassing the framework worth the performance gain? We'll lose
the possiblity to hook on exception callbacks.

The commands also need more helpful documentation, I am working on that.



Not tested yet, but here are my comments on the patches:



0247 looks good

In 0248 you deleted a newline at EOF in 71-idviews.update

0249 looks good



0250:
With so many names imported from one module, I find it more readable
to `from ipalib.plugins import baseldap`, and use qualified names
later. Same in the 2 other patches.
This is personal preference/tip, feel free to disagree :)


I was just trying to be consistent here, rest of the IPA plugins seem to
use non-qualified names for plugins. Even tough the main reason is
probably that thay use evil star imports :) Let's keep it this way for now.


Yes, the rest of the plugins are using star imports; no consistency to 
be kept here.
I think that after these patches nobody will touch it again, so for 
now is basically forever.


0521 looks good


0252:



The pattern_errmsg in idoverride's uid should be expanded to fully
describe the pattern.


If we want to expand it, then it should be corrected in user.py plugin
too (I have taken it from there). Still, it's a tradeoff between
conciseness and and exactness, do we really want to specify that user
login may be at most 253 letters long or the $ character may only be
used as terminal one? Or the fact that - can't be used at the beggining?
This is probably the way it was since forever in user plugin.


I think the message should at be free of false positives. Using e.e. 
-test-name- and getting back may only include letters, numbers, _, -, 
. and $ would be extremely frustrating.


Should I file a ticket?

0253 looks good
0255-0256 looks good


0258:
idview_apply: typo in hostgroup doc, should be hosts to apply the ID
view to and after running the idview-apply command.
Typos in output params, should be e.g. this ID view


Fixed the second and third complaint ( IMHO this is clear from context,
but I added this to be explicit), I'm not so sure about the first one
though. Can we get a native speaker input on that?


Actually both just need an article, doesn't matter much if it's the or 
this.




By subclassing idview_unapply from idview_apply you're violating
Liskov's substitution principle. Make a common base class for them.


Done.


In the wrong patch, it seems :)



0259:
show_id_overrides: cn is a MUST attribute in ipa*Override; use
view.single_value['cn'] instead of get(). In enumerate_hosts, is None
okay if cn is missing?


This was actually an error, fixed in updated version of patches. New
code uses 'ipaanchoruuid' (which is a MUST) and converts it (with your
objections addressed.)


If it's MUST, use simply `single_value['ipaanchoruuid']` instead of get().


0260:
The get() method on dict-like objects returns None (or a given default) 
if the item is missing.

If sAMAccountName is MUST, use regular dict item access.
If sAMAccountName is MAY, then you're not handling the None you might get.

Also look at the other uses of get() to see if you're doing the right thing.

I've looked at help(pysss_nss_idmap.getnamebysid). Shouldn't you use 
pysss_nss_idmap.NAME_KEY and TYPE_KEY?



0261:
Again you're using get() on MUST attributes.

0262:
ipalib/constants.py - No newline at end of file

Is IPA_ANCHOR_PREFIX and SID_ANCHOR_PREFIX to be used outside 
idviews.py? Do we want it in constants?



+if anchor.startswith(IPA_ANCHOR_PREFIX):
+uuid = anchor.split(IPA_ANCHOR_PREFIX)[1].strip()


This is a dangerous idiom to use for removing a prefix, since it will 
silently give wrong results if the string contains the prefix. Prefer:

anchor[len(IPA_ANCHOR_PREFIX):]
Same with SID_ANCHOR_PREFIX below.

In get_dn, why do you resolve only the last key?


Re: [Freeipa-devel] [PATCH 0298-0302] Implement handling of inactive master zones

2014-09-23 Thread Petr Spacek

On 22.9.2014 15:10, Tomas Hozza wrote:

On 09/19/2014 03:46 PM, Petr Spacek wrote:

Hello,

This patch set fixes
https://fedorahosted.org/bind-dyndb-ldap/ticket/127
https://bugzilla.redhat.com/show_bug.cgi?id=1138317

Please review it ASAP, it targets IPA 4.1/Fedora 21.

Tomas and Martin, please communicate who is going to review what:-)

Thank you for your time!


The code seems to be fine.

ACK.


Thank you for review!

I'm branching v5 at this point (before the push). In other words v5 branch 
will not contain this patch set and master branch will contain the patch set.


Pushed to master (to-be-v6):
23f2ddb317ab46419ea83725f458caae1b432fb5
27a911cae9de911938d90c94c61cecd494136fc1
3dd95594e664b7b59a9f8c1d15cb1280eebeb3e7
169f5e5f2a551253bc489edf109eab71a8331c3f
5ef943a39cfdfbadeb2a41cc3efd707caeca36bd

--
Petr^2 Spacek

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


[Freeipa-devel] [PATCH 0303-0305] Preparation for 6.0 release

2014-09-23 Thread Petr Spacek

Hello,

Pushed to master:

Bump NVR to 6.0.
aeba2eacd5bc85cd4b2872e87a6db5f6d680c266

Update NEWS for upcoming 6.0 release.
92164653b0b8a44d19dea547ddb4917069398e82

Update idnsZoneActive description in README.
8863b85b509984259052d47cbaadf2e7b84a881b

--
Petr^2 Spacek
From 8863b85b509984259052d47cbaadf2e7b84a881b Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 23 Sep 2014 10:34:59 +0200
Subject: [PATCH] Update idnsZoneActive description in README.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 README | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/README b/README
index fe452029965357f15915afcf87af4ea41c45cfd1..1b8f2a713ad901d3aab5ed799cbb9c5e9c746c44 100644
--- a/README
+++ b/README
@@ -156,9 +156,20 @@ Attributes:
 	transfers from multiple masters to single slave.
 
 * idnsZoneActive
-Boolean which speicifies if particular DNS zone should be loaded
-or not. This option is not supported in versions = 4.0.
-https://fedorahosted.org/bind-dyndb-ldap/ticket/127
+Boolean which speicifies if particular DNS zone should be visible
+to clients or not. This attribute can be changed at run-time.
+
+Inactive zones are loaded into memory in the same way as active zones.
+The only difference is that inactive zones are not added to DNS view
+used by bind-dyndb-ldap.
+
+Zone will be re-added to DNS view if idnsActiveZone attribute is
+changed to TRUE so the change should be almost immediate.
+
+Usual zone maintenance (serial number maintenance, DNSSEC in-line
+signing etc.) is done for all zones, no matter if the zone
+is active or not. This allows us to maintain zone journal so IXFR
+works correctly even after zone re-activation.
 
 * nSEC3PARAMRecord
 	NSEC3PARAM resource record definition according to RFC5155.
-- 
1.9.3

From 92164653b0b8a44d19dea547ddb4917069398e82 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 23 Sep 2014 10:36:34 +0200
Subject: [PATCH] Update NEWS for upcoming 6.0 release.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 NEWS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/NEWS b/NEWS
index f9869ba447ff34bed0d42d514846f8a47e5be496..f88d8552d34a774f684081f14340208cdf00a43b 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,9 @@
+6.0
+
+[1] idnsZoneActive attribute is supported again and zones can be activated
+and deactivated at run-time.
+https://fedorahosted.org/bind-dyndb-ldap/ticket/127
+
 5.3
 
 [1] Internal locking was reworked to prevent crashes and deadlocks.
-- 
1.9.3

From aeba2eacd5bc85cd4b2872e87a6db5f6d680c266 Mon Sep 17 00:00:00 2001
From: Petr Spacek pspa...@redhat.com
Date: Tue, 23 Sep 2014 10:38:38 +0200
Subject: [PATCH] Bump NVR to 6.0.

Signed-off-by: Petr Spacek pspa...@redhat.com
---
 configure.ac | 2 +-
 contrib/bind-dyndb-ldap.spec | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index adb3b549e13f243d3f18adc91ddf766d036c4686..d471038ada54c07dcfc211c8a2572850e3b28205 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1,5 +1,5 @@
 AC_PREREQ([2.59])
-AC_INIT([bind-dyndb-ldap], [5.3], [freeipa-devel@redhat.com])
+AC_INIT([bind-dyndb-ldap], [6.0], [freeipa-devel@redhat.com])
 
 AM_INIT_AUTOMAKE([-Wall foreign dist-bzip2])
 
diff --git a/contrib/bind-dyndb-ldap.spec b/contrib/bind-dyndb-ldap.spec
index 70796e62e349a94fb97047d00e4f2b145576412d..572076597a7597c8495ac7a977f26578e840603e 100644
--- a/contrib/bind-dyndb-ldap.spec
+++ b/contrib/bind-dyndb-ldap.spec
@@ -1,7 +1,7 @@
 %define VERSION %{version}
 
 Name:   bind-dyndb-ldap
-Version:5.3
+Version:6.0
 Release:0%{?dist}
 Summary:LDAP back-end plug-in for BIND
 
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH] 314 Allow specifying key algorithm of the IPA CA cert in ipa-server-install

2014-09-23 Thread Jan Cholasta

Dne 6.8.2014 v 18:17 Jan Cholasta napsal(a):

Dne 6.8.2014 v 14:43 Rob Crittenden napsal(a):

Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4447.




+cert_group.add_option(--ca-key-algorithm, dest=ca_key_algorithm,
+  help=Key algorithm of the IPA CA certificate
(default SHA256withRSA))

Why not set the default here rather than later?


CA-related defaults should be internalized in CA-related code IMHO.



Should the list of options be added to the man page as well?


Sure, why not.



Do we want to support the MD*-based signing algorithms? I'd think not.


Since the reason this patch exists is to support old and/or broken
external CAs, I would think yes, but I don't have a strong opinion on this.


Turns out Dogtag does not like them, so I removed them.





Seeing the context makes me wonder if we should eventually add options
for CA key size and signing alg as well.

rob






Updated patch attached.

--
Jan Cholasta
From 528587b8024055a8cd783cc9ebd493684b6dcc62 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Wed, 6 Aug 2014 09:43:19 +0200
Subject: [PATCH] Allow specifying key algorithm of the IPA CA cert in
 ipa-server-install.

This is especially useful for external CA install, as the algorithm is also
used for the CSR signature.

https://fedorahosted.org/freeipa/ticket/4447
---
 install/tools/ipa-server-install   | 13 ++---
 install/tools/man/ipa-server-install.1 |  3 +++
 ipaserver/install/cainstance.py| 12 ++--
 3 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 207dabd..e0cc165 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -226,6 +226,10 @@ def parse_options():
 cert_group.add_option(--subject, action=callback, callback=subject_callback,
   type=string,
   help=The certificate subject base (default O=realm-name))
+cert_group.add_option(--ca-key-algorithm, dest=ca_key_algorithm,
+  type=choice,
+  choices=('SHA1withRSA', 'SHA256withRSA', 'SHA512withRSA'),
+  help=Key algorithm of the IPA CA certificate)
 parser.add_option_group(cert_group)
 
 dns_group = OptionGroup(parser, DNS options)
@@ -1082,7 +1086,8 @@ def main():
 dogtag_constants=dogtag.install_constants)
 if external == 0:
 ca.configure_instance(host_name, domain_name, dm_password,
-  dm_password, subject_base=options.subject)
+  dm_password, subject_base=options.subject,
+  ca_key_algorithm=options.ca_key_algorithm)
 elif external == 1:
 # stage 1 of external CA installation
 options.realm_name = realm_name
@@ -1097,14 +1102,16 @@ def main():
 write_cache(vars(options))
 ca.configure_instance(host_name, domain_name, dm_password,
   dm_password, csr_file=paths.ROOT_IPA_CSR,
-  subject_base=options.subject)
+  subject_base=options.subject,
+  ca_key_algorithm=options.ca_key_algorithm)
 else:
 # stage 2 of external CA installation
 ca.configure_instance(host_name, domain_name, dm_password,
   dm_password,
   cert_file=options.external_cert_file,
   cert_chain_file=options.external_ca_file,
-  subject_base=options.subject)
+  subject_base=options.subject,
+  ca_key_algorithm=options.ca_key_algorithm)
 
 # Now put the CA cert where other instances exepct it
 ca.publish_ca_cert(CACERT)
diff --git a/install/tools/man/ipa-server-install.1 b/install/tools/man/ipa-server-install.1
index 8cc2ffa..957bd6d 100644
--- a/install/tools/man/ipa-server-install.1
+++ b/install/tools/man/ipa-server-install.1
@@ -123,6 +123,9 @@ PEM file containing the CA certificate of the CA which issued the Directory Serv
 .TP
 \fB\-\-subject\fR=\fISUBJECT\fR
 The certificate subject base (default O=REALM.NAME)
+.TP
+\fB\-\-ca\-key\-algorithm\fR=\fIALGORITHM\fR
+Key algorithm of the IPA CA certificate. Possible values are SHA1withRSA, SHA256withRSA, SHA512withRSA. Default value is SHA256withRSA.
 
 .SS DNS OPTIONS
 .TP
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 8c1b139..6534010 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -410,7 +410,7 @@ class CAInstance(service.Service):
pkcs12_info=None, master_host=None, csr_file=None,

Re: [Freeipa-devel] [PATCHES] 0264-0267 backup, restore: Don't overwrite /etc/{passwd, group}

2014-09-23 Thread Tomas Babej

On 08/26/2014 01:16 PM, Petr Viktorin wrote:
 On 07/30/2014 04:26 PM, Petr Viktorin wrote:
 On 07/29/2014 06:03 PM, Petr Viktorin wrote:
 On 07/29/2014 05:02 PM, Petr Viktorin wrote:
 Hello,

 The first patch here consolidates our system user creation code a bit.

 The second patch fixes an oversight in the restore script.

 The third changes the backup script to not include
 /etc/{passwd,group},
 and the restore script to create the PKI user if a CA is being
 restored.
 Note that the DS user is already created early in the restore process.
 (In the future we may want a nice generic framework for restoring
 users,
 but I'd like to extrapolate from more than one data point when
 designing
 it.)

 Another note: tar uses owner user/group names by default, so no
 additional chowning is required even if the IDs change between backup 
 restore.


 The fourth patch adds a log entry I find very useful in testing
 backup/restore.


 Rebased onto current master.

 Rebased again.




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

I like the approach, solves the issue quite neatly. I didn't find any
issues in the changes themselves, or during the testing, so ACK from me
for the whole patcheset (which means patches 624-627, despite the subject).

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

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

Re: [Freeipa-devel] [PATCHES] 0631-0632 Integration tests for backup restore

2014-09-23 Thread Tomas Babej

On 08/06/2014 04:52 PM, Petr Viktorin wrote:
 On 08/06/2014 04:36 PM, Petr Viktorin wrote:
 Hello,
 These patches add integration tests for backup  restore.

 They depend on my earlier backup/restore patches, 0624-0627.

 I'm also attaching a patch for the job definitions at
 https://github.com/encukou/freeipa-ci


 I hit Send too soon, sorry for that



Thank you for these patches, the tests themselves seem all right to me.

My only objection is that I don't think that the coverage for the basic
backup-restore test is exhaustive enough. Right now we only check the
presence of the admin entry in the LDAP via raw ldap calls and CLI, and
the output of the cert-find command.

In the future, I'd like this to be extended with basic functionality
tests for each of the services, i.e. does DNS work after the restore?
Are all the services up and running?

However, this does not block the patches, I think they can be pushed now
(this means a ACK from me) in their current form and extended later. If
you agree, I can file a ticket.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

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


Re: [Freeipa-devel] [PATCHES] 0264-0267 backup, restore: Don't overwrite /etc/{passwd, group}

2014-09-23 Thread Petr Viktorin

On 09/23/2014 12:10 PM, Tomas Babej wrote:


On 08/26/2014 01:16 PM, Petr Viktorin wrote:

On 07/30/2014 04:26 PM, Petr Viktorin wrote:

On 07/29/2014 06:03 PM, Petr Viktorin wrote:

On 07/29/2014 05:02 PM, Petr Viktorin wrote:

Hello,

The first patch here consolidates our system user creation code a bit.

The second patch fixes an oversight in the restore script.

The third changes the backup script to not include
/etc/{passwd,group},
and the restore script to create the PKI user if a CA is being
restored.
Note that the DS user is already created early in the restore process.




I like the approach, solves the issue quite neatly. I didn't find any
issues in the changes themselves, or during the testing, so ACK from me
for the whole patcheset (which means patches 624-627, despite the subject).


Thanks! Pushed to:
master: abba25c8269f4928d659cfcaf8363ad2491bd736
ipa-4-1: 127e7a1dcc2ed0624f65597292ac58535ccc0602

--
Petr³

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


Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

2014-09-23 Thread David Kupka

On 09/18/2014 06:34 PM, Martin Basti wrote:

...
1)
+if options.unattended:
+for ip in ip_addresses:
+if search_reverse_zones and find_reverse_zone(str(ip)):
+# reverse zone is already in LDAP
+continue
+for rz in ret_reverse_zones:
+if verify_reverse_zone(rz, ip):
+# reverse zone was entered by user
+break
+else:
+rz = get_reverse_zone_default(str(ip))
+ret_reverse_zones.append(rz)
+elif options.reverse_zones or create_reverse():
+for ip in ip_addresses:
+if search_reverse_zones and find_reverse_zone(str(ip)):
+# reverse zone is already in LDAP
+continue
+for rz in ret_reverse_zones:
+if verify_reverse_zone(rz, ip):
+# reverse zone was entered by user
+break
+else:
+rz = get_reverse_zone_default(str(ip))
+rz = read_reverse_zone(rz, str(ip))
+ret_reverse_zones.append(rz)
+else:
+options.no_reverse = True
+ret_reverse_zones = []

You can make it shorter without duplications:

# this ifs can be in one line
if not options.unatended:
 if not options.reverse_zones
 if not create_reverse():
 options.no_reverse=True
 return []

for ip in ip_addresses:
 if search_reverse_zones and find_reverse_zone(str(ip)):
 # reverse zone is already in LDAP
 continue
 for rz in ret_reverse_zones:
 if verify_reverse_zone(rz, ip):
 # reverse zone was entered by user
 break
 else:
 rz = get_reverse_zone_default(str(ip))
 if not options.unattended:
 rz = read_reverse_zone(rz, str(ip))
 ret_reverse_zones.append(rz)



Thanks, I modified it bit different way to alse address recommendation 3).



2)
Typo? There is no IP address matching reverze_zone %s.
-^^



Thanks, fixed.



3)
Would be nice to ask user to create new zones only if new zones are
required. (If all required zones exist in LDAP, you ask user anyway)



I added one more variable and ask only once.


4)
Ask framework gurus, if installutils module is better place for function
above




Petr^3 said that it's ok to have it in bindinstance.py.





--
David Kupka
From cea7c3e8eb41798d7f2bba916a95a78c034ee052 Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Wed, 27 Aug 2014 13:50:21 +0200
Subject: [PATCH] Detect and configure all usable IP addresses.

Find, verify and configure all IP addresses that can be used to reach the server
FreeIPA is being installed on. Ignore some IP address only if user specifies
subset of detected addresses using --ip-address option.
This change simplyfies FreeIPA installation on multihomed and dual-stacked servers.

https://fedorahosted.org/freeipa/ticket/3575
---
 install/tools/ipa-replica-install|  48 ++---
 install/tools/ipa-server-install |  50 ++
 ipaserver/install/bindinstance.py| 114 ++-
 ipaserver/install/installutils.py|  96 +-
 ipaserver/install/ipa_replica_prepare.py |  65 ++
 5 files changed, 207 insertions(+), 166 deletions(-)

diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index e3b65b0963d92e4d102a1166aa001fe8cb164562..bdc1a86fd339718c91a67b6c32daa69905025e90 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -67,8 +67,8 @@ def parse_options():
   default=False, help=configure a dogtag CA)
 basic_group.add_option(--setup-kra, dest=setup_kra, action=store_true,
   default=False, help=configure a dogtag KRA)
-basic_group.add_option(--ip-address, dest=ip_address,
-  type=ip, ip_local=True,
+basic_group.add_option(--ip-address, dest=ip_addresses,
+  type=ip, ip_local=True, action=append, default=[],
   help=Replica server IP Address)
 basic_group.add_option(-p, --password, dest=password, sensitive=True,
   help=Directory Manager (existing master) password)
@@ -112,7 +112,8 @@ def parse_options():
   type=ip, help=Add a DNS forwarder)
 dns_group.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
-dns_group.add_option(--reverse-zone, dest=reverse_zone, help=The reverse DNS zone to use)
+dns_group.add_option(--reverse-zone, dest=reverse_zones, default=[],
+ 

[Freeipa-devel] Announcing bind-dyndb-ldap version 6.0

2014-09-23 Thread Petr Spacek

The FreeIPA team is proud to announce bind-dyndb-ldap version 6.0.

It can be downloaded from https://fedorahosted.org/released/bind-dyndb-ldap/

The new version has also been built for Fedora 21+ and and is on its way to 
updates-testing:

https://admin.fedoraproject.org/updates/bind-dyndb-ldap-6.0-1.fc21

This version is also available from FreeIPA COPR repo:
http://copr.fedoraproject.org/coprs/mkosek/freeipa/


6.0

[1] idnsZoneActive attribute is supported again and zones can be activated
and deactivated at run-time.
https://fedorahosted.org/bind-dyndb-ldap/ticket/127


== Upgrading ==
A server can be upgraded by installing updated RPM. BIND has to be restarted 
manually after the RPM installation.


Downgrading back to any 5.x version is supported if idnsZoneActive is always 
set to TRUE.



== Feedback ==
Please provide comments, report bugs and send any other feedback via the 
freeipa-users mailing list:

http://www.redhat.com/mailman/listinfo/freeipa-users

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH] 323 Fix certmonger code causing the ca_renewal_master update plugin to fail

2014-09-23 Thread David Kupka

On 09/17/2014 03:57 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4547.

Honza



Works for me, thanks for patch.

ACK.

--
David Kupka

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


Re: [Freeipa-devel] [PATCH] 0105 FIX: LDAP_updater

2014-09-23 Thread Martin Basti

On 22/09/14 14:04, Petr Viktorin wrote:

On 09/01/2014 04:31 PM, Martin Basti wrote:

On 24/07/14 09:06, Martin Basti wrote:

On 23/07/14 15:17, Martin Basti wrote:

This patch fixes ordering problem of schema updates

Martin should it be in IPA 4.0.x ? It requires rebased ldap_python
(will be in Fedora 21)

Patch attached



I found a bug there, but before I send updated version, I need to
decide how print modlist:

1. Print modlist before each LDAP update (if changes exist), show
modlist per incremental update

2. Print modlist only at once with all updates

3. Use [1] for live_run, [2] for test


Print modlis before each LDAP update
Updated patch attached.


Thanks! This works nicely, just some comments:

The required python-ldap is quite new, not even built in Koji yet. We 
need to get it to the IPA COPR repo before this is merged.



The _get_oid_dependency_order function is not indented well.

It would be nice if it was a bit more obvious that the loop in 
_get_oid_dependency_order is guaranteed to terminate. Could you assert 
that len(unordered_oids) decreases in each iteration?



+SCHEMA_ELEMENT_CLASSES_KEYS = (x[0] for x in SCHEMA_ELEMENT_CLASSES)


You're creating a generator here, which is only good for one use. Use 
a list instead.
Also, this is used just once so it doesn't need to be a module-level 
constant.


+# FIXME: We should have a better way to display 
the modlist,
+# for now display raw output of our internal 
routine


Not entirely related to your change, but you can drop this comment. 
Since 61887ac we don't use an internal routine.




Thank you!
updated patch attached.

--
Martin Basti

From 996992efff178b8bd400dcbfe6ab6203b14beefc Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 23 Jul 2014 14:42:33 +0200
Subject: [PATCH] FIX: ldap schmema updater needs correct ordering of the
 updates

Required bugfix in python-ldap 2.4.15

Updates must respect SUP objectclasses/attributes and update
dependencies first
---
 freeipa.spec.in   |   2 +-
 ipaserver/install/schemaupdate.py | 125 ++
 2 files changed, 87 insertions(+), 40 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index c92c245776c83bb86c78575e0f88bc62b849a831..d692e8c3d054be144298696a9f1a383a1f5d60d0 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -99,7 +99,7 @@ Requires: httpd = 2.4.6-6
 Requires: mod_wsgi
 Requires: mod_auth_kerb = 5.4-16
 Requires: mod_nss = 1.0.8-26
-Requires: python-ldap
+Requires: python-ldap = 2.4.15
 Requires: python-krbV
 Requires: acl
 Requires: python-pyasn1
diff --git a/ipaserver/install/schemaupdate.py b/ipaserver/install/schemaupdate.py
index bb2f0f161499c0358452d97f27f5c39b9b64a5ad..eaa45d5e947803bb339b9462df666f58f06e0a2d 100644
--- a/ipaserver/install/schemaupdate.py
+++ b/ipaserver/install/schemaupdate.py
@@ -29,17 +29,60 @@ from ipaserver.install.ldapupdate import connect
 from ipaserver.install import installutils
 
 
-SCHEMA_ELEMENT_CLASSES = {
+SCHEMA_ELEMENT_CLASSES = (
 # All schema model classes this tool can modify
-'objectclasses': ldap.schema.models.ObjectClass,
-'attributetypes': ldap.schema.models.AttributeType,
-}
+# Depends on order, attributes first, then objectclasses
+('attributetypes', ldap.schema.models.AttributeType),
+('objectclasses', ldap.schema.models.ObjectClass),
+)
 
 ORIGIN = 'IPA v%s' % ipapython.version.VERSION
 
 log = log_mgr.get_logger(__name__)
 
 
+def _get_oid_dependency_order(schema, cls):
+
+Returns a ordered list of OIDs sets, in order which respects inheritance in LDAP
+OIDs in second set, depend on first set, etc.
+
+:return [set(1st-tree-level), set(2nd-tree-level), ...]
+
+top_node = '_'
+ordered_oid_groups = []
+
+tree = schema.tree(cls)  # tree structure of schema
+
+# remove top_node from tree, it breaks ordering
+# we don't need this, tree from file is not consistent
+del tree[top_node]
+unordered_oids = tree.keys()
+
+# split into two groups, parents and child nodes, and iterate until
+# child nodes are not empty
+while unordered_oids:
+parent_nodes = set()
+child_nodes = set()
+
+for node in unordered_oids:
+if node not in child_nodes:
+# if node was child once, must remain as child
+parent_nodes.add(node)
+
+for child_oid in tree[node]:
+# iterate over all child nodes stored in tree[node] per node
+# child node must be removed from parents
+parent_nodes.discard(child_oid)
+child_nodes.add(child_oid)
+
+ordered_oid_groups.append(parent_nodes)  # parents nodes are not dependent
+
+assert len(child_nodes)  len(unordered_oids)  # while iteration must be finite
+unordered_oids = child_nodes  # extract new parent nodes in next iteration
+
+return ordered_oid_groups

[Freeipa-devel] [PATCH] JSON client: Log pretty-printed request and response with -vvv or above

2014-09-23 Thread Petr Viktorin

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


$ ipa -vvv user-show admin
ipa: INFO: trying https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json
ipa: INFO: Forwarding 'user_show' to json server 
'https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json'

ipa: INFO: Request: {
id: 0,
method: user_show,
params: [
[
admin
],
{
all: false,
no_members: false,
raw: false,
rights: false,
version: 2.102
}
]
}
[...bunch of output omitted...]
ipa: INFO: Response: {
error: null,
id: 0,
principal: ad...@idm.lab.eng.brq.redhat.com,
result: {
result: {
dn: 
uid=admin,cn=users,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com, 


gidnumber: [
36480
],
has_keytab: true,
has_password: true,
homedirectory: [
/home/admin
],
loginshell: [
/bin/bash
],
memberof_group: [
admins,
trust admins
],
nsaccountlock: false,
sn: [
Administrator
],
uid: [
admin
],
uidnumber: [
36480
]
},
summary: null,
value: admin
},
version: 4.0.0GIT88bea65
}
  User login: admin
  Last name: Administrator
  Home directory: /home/admin
  Login shell: /bin/bash
  UID: 36480
  GID: 36480
  Account disabled: False
  Password: True
  Member of groups: admins, trust admins
  Kerberos keys available: True

--
Petr³
From 88bea6539b0f23c9a92427ae3036178b67bcbcdb Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 23 Sep 2014 12:10:56 +0200
Subject: [PATCH] JSON client: Log pretty-printed request and response with
 -vvv or above

Changes `verbose` in the connection to be the level from api.env,
rather than a boolean value.

https://fedorahosted.org/freeipa/ticket/4233
---
 ipalib/backend.py |  2 +-
 ipalib/rpc.py | 11 +--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/ipalib/backend.py b/ipalib/backend.py
index b94264236795b65905ba425ef15e452e7a66625b..210058981a6ecc5884040648e9d7929abf473af7 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -113,7 +113,7 @@ def create_context(self, ccache=None, client_ip=None):
 if self.env.in_server:
 self.Backend.ldap2.connect(ccache=ccache)
 else:
-self.Backend.rpcclient.connect(verbose=(self.env.verbose = 2),
+self.Backend.rpcclient.connect(verbose=self.env.verbose,
 fallback=self.env.fallback, delegate=self.env.delegate)
 if client_ip is not None:
 setattr(context, client_ip, client_ip)
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index 1bfc4c3d3f8ac36c9dbb135561707eaa9f6ac7f3..88bed16807b3fb3255225f2aa1942933062b0f48 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -761,7 +761,7 @@ def apply_session_cookie(self, url):
 
 return session_url
 
-def create_connection(self, ccache=None, verbose=False, fallback=True,
+def create_connection(self, ccache=None, verbose=0, fallback=True,
   delegate=False, nss_dir=None):
 try:
 rpc_uri = self.env[self.env_rpc_uri_key]
@@ -965,11 +965,15 @@ def __request(self, name, args):
 payload = {'method': unicode(name), 'params': args, 'id': 0}
 version = args[1].get('version', VERSION_WITHOUT_CAPABILITIES)
 
+if self.__verbose = 3:
+root_logger.info('Request: %s',
+ json.dumps(payload, sort_keys=True, indent=4))
+
 response = self.__transport.request(
 self.__host,
 self.__handler,
 json.dumps(json_encode_binary(payload, version)),
-verbose=self.__verbose,
+verbose=self.__verbose = 2,
 )
 
 try:
@@ -977,6 +981,9 @@ def __request(self, name, args):
 except ValueError, e:
 raise JSONError(str(e))
 
+if self.__verbose = 3:
+root_logger.info('Response: %s',
+ json.dumps(response, sort_keys=True, indent=4))
 error = response.get('error')
 if error:
 try:
-- 
1.9.3

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

Re: [Freeipa-devel] [PATCH] JSON client: Log pretty-printed request and response with -vv or above

2014-09-23 Thread Petr Viktorin

On 09/23/2014 03:13 PM, Petr Viktorin wrote:

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


After talking to Rob, I've changed what the -v means a bit more:

A single -v just turns on INFO logging, as before:

$ ipa -v ping
ipa: INFO: trying https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json
ipa: INFO: Forwarding 'ping' to json server 
'https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json'

-
IPA server version 4.0.0GIT8543d4c. API version 2.102
-

Two -v's pretty-print the request  response:

$ ipa -vv ping
ipa: INFO: trying https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json
ipa: INFO: Forwarding 'ping' to json server 
'https://vm-073.idm.lab.eng.brq.redhat.com/ipa/json'

ipa: INFO: Request: {
id: 0,
method: ping,
params: [
[],
{
version: 2.102
}
]
}
ipa: INFO: Response: {
error: null,
id: 0,
principal: ad...@idm.lab.eng.brq.redhat.com,
result: {
summary: IPA server version 4.0.0GIT8543d4c. API version 2.102
},
version: 4.0.0GIT8543d4c
}
-
IPA server version 4.0.0GIT8543d4c. API version 2.102
-

And three -v's print everything -- the pretty-printed JSON and all of 
the HTTP communication.


Also, when using XML-RPC, a single -v will now also print all the HTTP 
stuff. It could respond to two -v's as before I don't think it's worth 
complicating the code (keep in mind this is client only, XML-RPC is not 
used unless requested in the env).


This patch also updates the man page.

--
Petr³

From 721fdbc50ffd951eba2e5427233c909d0245c788 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Tue, 23 Sep 2014 12:10:56 +0200
Subject: [PATCH] JSON client: Log pretty-printed request and response with -vv
 or above

The whole HTTP request is now printed with -vvv or above.

Changes `verbose` in the connection to be the level from api.env,
rather than a boolean value.

For XML-RPC, the whole request will be shown already with -v.

https://fedorahosted.org/freeipa/ticket/4233
---
 ipa.1 |  3 ++-
 ipalib/backend.py |  2 +-
 ipalib/rpc.py | 11 +--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/ipa.1 b/ipa.1
index fc39fceaae5aa4c614ccaaa7e608f2326d926755..cadd9aa98d7d976f5a683b5fe83ab30ee357dd99 100644
--- a/ipa.1
+++ b/ipa.1
@@ -56,7 +56,8 @@ Prompt for all parameters of \fICOMMAND\fR, even if they are optional.
 Don't fall back to other IPA servers if the default doesn't work.
 .TP
 \fB\-v\fR, \fB\-\-verbose\fR
-Produce verbose output. A second \-v displays the XML\-RPC request.
+Produce verbose output. A second -v pretty-prints the JSON request and response. A third \-v displays the HTTP request and response.
+.TP
 \fB\-\-version\fR
 Display the IPA version and API version.
 .SH COMMANDS
diff --git a/ipalib/backend.py b/ipalib/backend.py
index b94264236795b65905ba425ef15e452e7a66625b..210058981a6ecc5884040648e9d7929abf473af7 100644
--- a/ipalib/backend.py
+++ b/ipalib/backend.py
@@ -113,7 +113,7 @@ def create_context(self, ccache=None, client_ip=None):
 if self.env.in_server:
 self.Backend.ldap2.connect(ccache=ccache)
 else:
-self.Backend.rpcclient.connect(verbose=(self.env.verbose = 2),
+self.Backend.rpcclient.connect(verbose=self.env.verbose,
 fallback=self.env.fallback, delegate=self.env.delegate)
 if client_ip is not None:
 setattr(context, client_ip, client_ip)
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index 1bfc4c3d3f8ac36c9dbb135561707eaa9f6ac7f3..e7e60f414b9a519fc98efdb0cee2205ed60b331c 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -761,7 +761,7 @@ def apply_session_cookie(self, url):
 
 return session_url
 
-def create_connection(self, ccache=None, verbose=False, fallback=True,
+def create_connection(self, ccache=None, verbose=0, fallback=True,
   delegate=False, nss_dir=None):
 try:
 rpc_uri = self.env[self.env_rpc_uri_key]
@@ -965,11 +965,15 @@ def __request(self, name, args):
 payload = {'method': unicode(name), 'params': args, 'id': 0}
 version = args[1].get('version', VERSION_WITHOUT_CAPABILITIES)
 
+if self.__verbose = 2:
+root_logger.info('Request: %s',
+ json.dumps(payload, sort_keys=True, indent=4))
+
 response = self.__transport.request(
 self.__host,
 self.__handler,
 json.dumps(json_encode_binary(payload, version)),
-verbose=self.__verbose,
+verbose=self.__verbose = 3,
 )
 
 try:
@@ -977,6 +981,9 @@ def __request(self, name, args):
 except ValueError, e:
 raise JSONError(str(e))
 
+if self.__verbose = 2:
+

Re: [Freeipa-devel] [PATCH] 323 Fix certmonger code causing the ca_renewal_master update plugin to fail

2014-09-23 Thread Petr Viktorin

On 09/23/2014 02:34 PM, David Kupka wrote:

On 09/17/2014 03:57 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4547.

Honza



Works for me, thanks for patch.

ACK.



Pushed to:
master: f680a63158d172042c91537a1cb7f6f53766e2ad
ipa-4-1: 1a327cf42929919219c2f0bfa9b48eb2d0b039f4
ipa-4-0: 26188d7610170ff2fb89b12cd63a0c698a2381cb

--
Petr³

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


[Freeipa-devel] [PATCH 130] extdom: add support for new version

2014-09-23 Thread Sumit Bose
Hi,

this patch should fix https://fedorahosted.org/freeipa/ticket/4031 and
with the corresponding SSSD part it would be possible to get the full
list of group memberships with the id command even for user who didn't
log in before.

bye,
Sumit
From 23ff38cdea85995b211e73f474bcb4b0d7fb8039 Mon Sep 17 00:00:00 2001
From: Sumit Bose sb...@redhat.com
Date: Tue, 23 Sep 2014 15:55:43 +0200
Subject: [PATCH] extdom: add support for new version

Currently the extdom plugin is basically used to translate SIDs of AD
users and groups to names and POSIX IDs.

With this patch a new version is added which will return the full member
list for groups and the full list of group memberships for a user.
Additionally the gecos field, the home directory and the login shell of a
user are returned and an optional list of key-value pairs which
currently will contain the SID of the requested object if available.

https://fedorahosted.org/freeipa/ticket/4031
---
 .../ipa-extdom-extop/ipa_extdom.h  |  29 +-
 .../ipa-extdom-extop/ipa_extdom_common.c   | 850 +++--
 .../ipa-extdom-extop/ipa_extdom_extop.c|  28 +-
 3 files changed, 640 insertions(+), 267 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
index 
5f834a047a579104cd2589ce417c580c1c5388d3..548ee74f561c474854c049726c4c3e71da5cbbea
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom.h
@@ -64,6 +64,7 @@
 #include sss_nss_idmap.h
 
 #define EXOP_EXTDOM_OID 2.16.840.1.113730.3.8.10.4
+#define EXOP_EXTDOM_V2_OID 2.16.840.1.113730.3.8.10.4.1
 
 #define IPA_EXTDOM_PLUGIN_NAME   ipa-extdom-extop
 #define IPA_EXTDOM_FEATURE_DESC  IPA trusted domain ID mapper
@@ -71,6 +72,11 @@
 
 #define IPA_PLUGIN_NAME IPA_EXTDOM_PLUGIN_NAME
 
+enum extdom_version {
+EXTDOM_V1 = 1,
+EXTDOM_V2
+};
+
 enum input_types {
 INP_SID = 1,
 INP_NAME,
@@ -80,14 +86,17 @@ enum input_types {
 
 enum request_types {
 REQ_SIMPLE = 1,
-REQ_FULL
+REQ_FULL,
+REQ_FULL_WITH_GROUPS
 };
 
 enum response_types {
 RESP_SID = 1,
 RESP_NAME,
 RESP_USER,
-RESP_GROUP
+RESP_GROUP,
+RESP_USER_GROUPLIST,
+RESP_GROUP_MEMBERS
 };
 
 struct extdom_req {
@@ -123,11 +132,18 @@ struct extdom_res {
 char *user_name;
 uid_t uid;
 gid_t gid;
+char *gecos;
+char *home;
+char *shell;
+size_t ngroups;
+char **groups;
 } user;
 struct {
 char *domain_name;
 char *group_name;
 gid_t gid;
+size_t nmembers;
+char **members;
 } group;
 } data;
 };
@@ -150,15 +166,14 @@ struct pwd_grp {
 struct passwd pwd;
 struct group grp;
 } data;
+int ngroups;
+gid_t *groups;
 };
 
 int parse_request_data(struct berval *req_val, struct extdom_req **_req);
 void free_req_data(struct extdom_req *req);
+int check_request(struct extdom_req *req, enum extdom_version version);
 int handle_request(struct ipa_extdom_ctx *ctx, struct extdom_req *req,
-   struct extdom_res **res);
-int create_response(struct extdom_req *req, struct pwd_grp *pg_data,
-const char *sid_str, enum sss_id_type id_type,
-const char *domain_name, struct extdom_res **_res);
-void free_resp_data(struct extdom_res *res);
+   struct berval **berval);
 int pack_response(struct extdom_res *res, struct berval **ret_val);
 #endif /* _IPA_EXTDOM_H_ */
diff --git a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c 
b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
index 
025d37dc5eda05c8db43d4e8176fd7898ed32fe7..5c1ae79c818676c3660d5cd5b8ca5515a4f0f18d
 100644
--- a/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
+++ b/daemons/ipa-slapi-plugins/ipa-extdom-extop/ipa_extdom_common.c
@@ -70,6 +70,7 @@ int parse_request_data(struct berval *req_val, struct 
extdom_req **_req)
  *requestType ENUMERATED {
  *simple (1),
  *full (2)
+ *full_with_groups (3)
  *},
  *data InputData
  * }
@@ -179,23 +180,23 @@ void free_req_data(struct extdom_req *req)
 free(req);
 }
 
-int handle_request(struct ipa_extdom_ctx *ctx, struct extdom_req *req,
-   struct extdom_res **res)
+int check_request(struct extdom_req *req, enum extdom_version version)
+{
+if (version == EXTDOM_V1) {
+if (req-request_type == REQ_FULL_WITH_GROUPS) {
+return LDAP_PROTOCOL_ERROR;
+}
+}
+
+return LDAP_SUCCESS;
+}
+
+static int get_buffer(size_t *_buf_len, char **_buf)
 {
-int ret;
-char *domain_name = NULL;
-char *sid_str = NULL;
-size_t buf_len;
-char *buf = NULL;
 long pw_max;
 long gr_max;
-struct pwd_grp pg_data;
-

Re: [Freeipa-devel] Krb service delegation rules in CLI

2014-09-23 Thread Martin Kosek
On 09/22/2014 09:48 PM, Alexander Bokovoy wrote:
 On Mon, 22 Sep 2014, Martin Basti wrote:
 Hello,

 Related ticket: https://fedorahosted.org/freeipa/ticket/3644


 1) API

 The ipaKrb5DelegationACL objectclass requires targets which are stored in
 extra objectclass.

 A) we allow users to create groups of principals and then associate them as
 targets -- user can use same group for multiple delegation ACL

 B) users specify only list of target principals (no groups)

 B seems better to me.
 No.
 
 You have three clearly separate object types here:
 
 1. Principals -- any object in the tree that has krbPrincipalAux
 supplemental auxiliary object class. These objects can be anywhere in
 the tree, we can decide on limiting subtrees to search.
 
 2. Delegation ACLs -- any object that has ipaKrb5DelegationACL
 supplemental structural object class. These objects can be anywhere
 under cn=s4u2proxy,$SUFFIX
 
 3. Groups of principals  -- any object that has groupOfPrincipals _and_
 doesn't have ipaKrb5DelegationACL object classes. These objects can be
 anywhere in the tree, we can decide on limiting subtrees to search and
 can decide on a specific subtree to put new groups of principals.
 
 For (1) you just need to be able to reference principals, this only
 requires a search filter to define DN of the principal based on
 krbPrincipalName or krbCanonicalName, with support for 'shortcuts'
 (specifying principal without realm which is our realm).
 
 (1) is only used to resolve principals for (2) and (3).
 
 For (2) you need to implement a proper object handling, like any other
 IPA object that supports:
  -- multiple values for ipaAllowToImpersonate
  -- multiple values for ipaAllowedTarget
 CLI would look something like this:
  ipa krbdelegation-add
   -show
   -find
   -mod
   -add-source
   -add-target
   -del-source
   -del-target
   -del
 
 For (3) you need to implement a proper object handling where creating
 the object would happen in the predefined subtree but search or modify
 would be possible for any object matched by a proper filter.
 
 CLI would look something like this:
  ipa krbprincipalgroup-add
   -show
   -find
   -mod
   -add-member
   -del-member
 
 
 The way we use right now groupOfPrincipals, the objects referenced are
 placed in different subtrees, according to their primary role. That's
 right, as krbPrincipalAux is an auxiliary class, and is added on top of
 other object classes to turn other objects into principals. We also use
 groupOfPrincipals not only for Kerberos delegation ACLs, that's why
 support for multiple subtrees is needed.
 
 Creating new groupOfPrincipals objects can be allowed in a single
 designated place, cn=s4u2proxy,$SUFFIX, because this is primary use of
 this API.

Alexander's proposal makes sense to me. I would just personally name the
commands differently to share the same prefix. I.e.

ipa krbdelegation-*
and
ipa krbdelegationgroup-*

as we are not creating general purpose principal groups but only groups for
S4U2Proxy usage.

I would also reconsider your request to search for the objects in the whole
s4u2proxy tree. Across FreeIPA framework, we handle objects in static and
defined location of our tree, not in the entire subtree.

I think we should although indeed consider adding group of principals to own
nsContainer (like cn=targets) as otherwise we will have a clash between
krbdelegation rule names and krbdelegationgroup names.

Martin

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


Re: [Freeipa-devel] [PATCHES 0114-0115] DNS: allow to add root zone '.'

2014-09-23 Thread Petr Vobornik

On 25.8.2014 14:52, Martin Basti wrote:

Patches attached.

Ticket: https://fedorahosted.org/freeipa/ticket/4149

There is a bug in bind-dyndb-ldap (or worse in dirsrv), which cause the
named service is stopped after deleting zone.
Bug ticket: https://fedorahosted.org/bind-dyndb-ldap/ticket/138




Review of:
http://www.redhat.com/archives/freeipa-devel/2014-September/msg00484.html

1. Please follow pep8 for the new code.
 # git diff HEAD~7 -U0 | pep8 --diff --ignore=E501
Produces 25 erros.

Only E124 and E128 could be ignored if they are part of old code.

Patch 114:

2. Also should mention reverse zones:
-# except for our own domain
+# except for our own domain, and root zone

Patch 115: looks ok (except #1)

Patch 120:

3. This patch uses term 'deprecated' in a different meaning than a 
DeprecatedParam. It creates inconsistency - future confusion. IMHO this 
usage is correct since the usual understanding of deprecation is that 
the param is still usable but user should be prepared that it will be 
removed in a future.  IMHO DeprecatedParam is badly designed but that's 
not an issue of this patch.


I think we can leave this as is and create a ticket to rename 
DeprecatedParam e.g. to RemovedParam. What do you think?


That said, I don't see a reason for keeping 'doc', 'label', 
_validate_ipaddr in 'ip_address' param definition since the param is not 
used anywhere, and is not present in CLI nor Web UI.


4. Convention for defining optional param is to use '?' at the end of 
name not: `required=False,` (idnssoamname)


5. You've removed 'idnssoamname' and 'force' from Web UI but dnszone-add 
precallback still uses these params. What is the intended purpose?


Patch 121:

6. Could this be evaluated as 'None' string? 
idnssoamname=unicode(ns_hostname),


the method doesn't guarantee that 'hostname  is not None' and 
ipa_replica_prepare calls it without args: `add_zone(domain)`



7. This line is too long:
+self.step(adding NS record to the zones, self.__add_self_ns) 
 # all zones must be created before this step


8. Forgotten debugging lines:
print no ldap2 connection  #TODO remove

and maybe:
print entry, repr(entry)


9. Is the LDAP connection check in remove_server_ns_records required? 
Other RR removal methods are called before this one and they don't have 
it. IMO if it has to be done, it should be done by a caller.



Note: I have not tested replica installation and removal in this round 
of review process.



Patch 123:

10. In `normalize_zonemgr(zonemgr)`, if zonemgr contains '@' shouldn't 
it be normalized to contain '.' at the end? Or is it handled by 
bind-dyndb-ldap?


11. You can remove `validate_zonemgr` import from dns.py

Patch 124:

12. 'idnssoamname' field in dnszone details page should be hidden or 
marked as required since the LDAP attribute is a MUST


13. You can completely remove `IPA.dnszone_adder_dialog` class since it 
existed only to contain the logic for idnssamname and ip_address fields 
(depends on #5).


Then remove `$factory: IPA.dnszone_adder_dialog,` from the dialog spec.

Patch 125:
14. Got:

  File 
/home/pvoborni/dev/freeipa/ipatests/test_xmlrpc/test_dns_plugin.py, 
line 332, in module

class test_dns(Declarative):
  File 
/home/pvoborni/dev/freeipa/ipatests/test_xmlrpc/test_dns_plugin.py, 
line 422, in test_dns

'nsrecord': nameservers,
NameError: name 'nameservers' is not defined

Did not investigate much.. but this line is weird:
  `e = No DNS servers found in LDAP` shouldn't 'e' be 
'get_nameservers_error'?



Unrelated to this patch set:

a. One is able to run:
  # ipa dnszone-remove-permission $zone
multiple times and it always returns success

Is it intentional?

b. Web UI doesn't have means to call dnszone-mod with --force option

--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 0645 ipa-replica-prepare: Wait for the DNS entry to be resolvable

2014-09-23 Thread Petr Spacek

On 22.9.2014 14:09, Petr Viktorin wrote:

On 09/22/2014 01:48 PM, Petr Spacek wrote:

On 22.9.2014 10:38, Martin Kosek wrote:

On 09/22/2014 10:31 AM, Petr Spacek wrote:

On 22.9.2014 10:14, Martin Kosek wrote:

On 09/19/2014 07:29 PM, Petr Viktorin wrote:

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

See ticket  commit message for details.


Shouldn't we add a 1 sec sleep between tries? Wouldn't current
version just
hammer DNS server with as many DNS queries as it can send?


Oh yes, please add some time.sleep() call :-)


Wow, no idea how that slipped out. Thanks for the catch.


Also I would like to see more detailed message:
+self.log.info('Waiting for hostname %s to be resolvable',
+  self.replica_fqdn)

= 'Waiting for hostname %s to be resolvable to A or  record'


bikeshed

Really? Shouldn't term resolvable already have that covered? A good
software
should work on all network types, whether it is IPv4, IPv6 or IPv8. So I
personally do not think we need to be that specific and can stick to
original
proposal.


I will agree with you if you post magic code which will work with DNS
records for IPv8 :-) The code is not going to work with IPv8 just
because we didn't mention 'A/' in the error message, A and 
RRtypes are hardcoded in the code.


+1; we're checking A and  so that's what we should say we're doing.

Is this wording OK?

Little NACK. (However, the wording is fine.)

Tcpdump revealed this:

IP vm-117.test.34067  vm-133.test.domain: 38467+ A? vm-092.test. (51)
IP vm-133.test.domain  vm-117.test.34067: 38467 NXDomain* 0/1/0 (116)
IP vm-117.test.36006  vm-133.test.domain: 20194+ A? vm-092.test.ipa.example. 
(63)
IP vm-133.test.domain  vm-117.test.36006: 20194 NXDomain* 0/1/0 (143)
IP vm-117.test.51333  vm-133.test.domain: 34027+ ? vm-092.test. (51)
IP vm-133.test.domain  vm-117.test.51333: 34027 NXDomain* 0/1/0 (116)
IP vm-117.test.60373  vm-133.test.domain: 45679+ ? 
vm-092.test.ipa.example. (63)


You can see that the query for each A/ type is repeated twice, the second 
time with 'ipa.example.' suffix.


This is caused by search list processing (search directive in 
/etc/resolv.conf) and is highly undesirable. (Read this [1] e-mail if you want 
to hear it from Paul Vixie.)


The fix is simple: You have to be sure that self.replica_fqdn is actually 
absolute FQDN - with the trailing period.


Naive solution would be to use
dns_answer = resolver.query(self.replica_fqdn + '.', 'A', 'IN')
but I don't know if self.replica_fqdn variable can contain trailing period or 
not.

Mbasti can show you more advanced code snippets using 'dns.name'.

[1] 
https://lists.dns-oarc.net/pipermail/dns-operations/2014-September/012157.html

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0126 - 0127] DNS: remove --class option

2014-09-23 Thread Petr Spacek

On 22.9.2014 19:21, Martin Basti wrote:

On 22/09/14 13:17, Petr Vobornik wrote:

On 19.9.2014 16:15, Martin Basti wrote:

Ticket: https://fedorahosted.org/freeipa/ticket/3414
Patch attached.



Patch 126:

1. I think that just
  DeprecatedParam('dnsclass?'),

should be enough.

Also

2. You forgot to update API.txt and VERSION

Patch 127:
 ACK


Updated patchset attached


ACK, it works for me.

--
Petr^2 Spacek

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


Re: [Freeipa-devel] [PATCH 0126 - 0127] DNS: remove --class option

2014-09-23 Thread Martin Basti

On 23/09/14 18:35, Petr Spacek wrote:

On 22.9.2014 19:21, Martin Basti wrote:

On 22/09/14 13:17, Petr Vobornik wrote:

On 19.9.2014 16:15, Martin Basti wrote:

Ticket: https://fedorahosted.org/freeipa/ticket/3414
Patch attached.



Patch 126:

1. I think that just
  DeprecatedParam('dnsclass?'),

should be enough.

Also

2. You forgot to update API.txt and VERSION

Patch 127:
 ACK


Updated patchset attached


ACK, it works for me.


Please don't push, we discuss this and we will nit use the DeprecatedParam.

--
Martin Basti

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


Re: [Freeipa-devel] [PATCH] 0009 Detect and configure all usable IP addresses.

2014-09-23 Thread Martin Basti

On 23/09/14 13:23, David Kupka wrote:

On 09/18/2014 06:34 PM, Martin Basti wrote:

...
1)
+if options.unattended:
+for ip in ip_addresses:
+if search_reverse_zones and find_reverse_zone(str(ip)):
+# reverse zone is already in LDAP
+continue
+for rz in ret_reverse_zones:
+if verify_reverse_zone(rz, ip):
+# reverse zone was entered by user
+break
+else:
+rz = get_reverse_zone_default(str(ip))
+ret_reverse_zones.append(rz)
+elif options.reverse_zones or create_reverse():
+for ip in ip_addresses:
+if search_reverse_zones and find_reverse_zone(str(ip)):
+# reverse zone is already in LDAP
+continue
+for rz in ret_reverse_zones:
+if verify_reverse_zone(rz, ip):
+# reverse zone was entered by user
+break
+else:
+rz = get_reverse_zone_default(str(ip))
+rz = read_reverse_zone(rz, str(ip))
+ret_reverse_zones.append(rz)
+else:
+options.no_reverse = True
+ret_reverse_zones = []

You can make it shorter without duplications:

# this ifs can be in one line
if not options.unatended:
 if not options.reverse_zones
 if not create_reverse():
 options.no_reverse=True
 return []

for ip in ip_addresses:
 if search_reverse_zones and find_reverse_zone(str(ip)):
 # reverse zone is already in LDAP
 continue
 for rz in ret_reverse_zones:
 if verify_reverse_zone(rz, ip):
 # reverse zone was entered by user
 break
 else:
 rz = get_reverse_zone_default(str(ip))
 if not options.unattended:
 rz = read_reverse_zone(rz, str(ip))
 ret_reverse_zones.append(rz)



Thanks, I modified it bit different way to alse address recommendation 
3).




2)
Typo? There is no IP address matching reverze_zone %s.
-^^



Thanks, fixed.



3)
Would be nice to ask user to create new zones only if new zones are
required. (If all required zones exist in LDAP, you ask user anyway)



I added one more variable and ask only once.


4)
Ask framework gurus, if installutils module is better place for function
above




Petr^3 said that it's ok to have it in bindinstance.py.






NACK, most important point is 7

1)
I'm not sure if this is bug, but interactively is allowed to add only 
one ip address


Unable to resolve IP address for host name
Please provide the IP address to be used for this host name: 2001:db8::2
The kerberos protocol requires a Realm name to be defined.

2)
I'm getting error message

Invalid reverse zone 10.in-addr.arpa. for IP address 2001:db8::dead:beef
Invalid reverse zone 10.in-addr.arpa. for IP address 
fed0:babe:baab:0:21a:4aff:fe10:4e18


 - or -

Do you want to configure the reverse zone? [yes]:
Please specify the reverse zone name 
[0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.]:
Invalid reverse zone 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa. for IP 
address fed0:babe:baab:0:21a:4aff:fe10:4e18
Please specify the reverse zone name 
[0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.]:
Using reverse zone(s) 0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa., 
0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.


This shouldn't be there

Could be better to ask user to specific zone for ip address a.b.c.d.

4) just nitpick
The IPA Master Server will be configured with:
...
IP address(es): 2001:db8::dead:beef, fed0:babe:baab:0:21a:4aff:fe10:4e18
...
Reverse zone:  0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa., 
0.0.0.0.b.a.a.b.e.b.a.b.0.d.e.f.ip6.arpa.


You have label IP address(es), so you should use label Reverse zone(s)

5)
ipa-server-install --ip-address=10.16.78.105 
--reverse-zone=10.in-addr.arpa. --reverse-zone=16.10.in-addr.arpa. 
--setup-dns


Creates both reverse zones, but 10.in-addr.arpa. is empty. I'm not sure 
if this is wrong, but we prevents user to add zone without address in 
it, so we should prevents, to add empty zone.


6)
ipa-replica-prepare --ip-address 10.16.78.105 --ip-address 
2001:db8::dead:beef --reverse-zone 1.0.0.2.ip6.arpa. vm-105.example.com

Directory Manager (existing master) password:

Invalid reverse zone 1.0.0.2.ip6.arpa. for IP address 10.16.78.105
Invalid reverse zone 1.0.0.2.ip6.arpa.

IMO This should work, right?

+sys.exit(There is no IP address matching reverse zone 
%s. % rz)

I expected at least this error to be shown.

7)
ipa-replica-prepare --ip-address 10.16.78.105 --ip-address 
2001:db8::dead:beef vm-105.example.com

Directory Manager (existing master) password:

...
Adding DNS records for vm-105.example.com
Values instance has no 

Re: [Freeipa-devel] [PATCHES] 319-321 Build and packaging fixes

2014-09-23 Thread Petr Vobornik

On 17.9.2014 13:58, Jan Cholasta wrote:

Dne 17.9.2014 v 13:07 Alexander Bokovoy napsal(a):

On Wed, 17 Sep 2014, Martin Kosek wrote:

On 09/17/2014 12:31 PM, Jan Cholasta wrote:


+Conflicts: %{alt_name}-server-trust-ad
+Obsoletes: %{alt_name}-server-trust-ad  %{version}-%{release}


Just one question - should we check also for %{release}? Generally,
release
number does not have much value, we could rebuild our version of the
package 10
times, but it does not mean it is newer.

I.e. freeipa-server-4.0.0-10 should probably not obsolete
ipa-server-4.0.0-1
but only ipa-server-3.3.3-10, right?

I agree with the %{release} idea. We definitely don't need to be so
picky with it.


OK, fixed.

I used wrong numbers for the patches, they should be 320-322. Fixed as
well.

Updated patches attached.



Patch 320: ACK
Patch 321: ACK
Patch 322: ACK

All pushed:

master:
* 0e2dc70d8ec4d96914190a38dac9a240605cab1d Allow RPM upgrade from ipa-* 
packages
* 9fa8cff6da6b6b85aa5b03028386f159fc816124 Include ipaplatform in 
client-only build
* 449d10b85cf2fe6a5b41bf167419f2432e752ad1 Include the ipa command in 
client-only build

ipa-4-1:
* 9486f3dc8d549b83f5a64b3dc8e57bf4896588d8 Allow RPM upgrade from ipa-* 
packages
* 72a82b855b77e04a0e672eaffc59e33256a66721 Include ipaplatform in 
client-only build
* fbc6345153391213d59e1c7c267e6425fdf7b77e Include the ipa command in 
client-only build

ipa-4-0:
* faba3f060e7513790d584bde3a9901793031fcf1 Allow RPM upgrade from ipa-* 
packages
* df187a4b4a9a2eeac8b11152de0cdc250e55cfb1 Include ipaplatform in 
client-only build
* 22ce015913c482308d499572bd24389bca98a0e5 Include the ipa command in 
client-only build


--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCHES] 319, 324-335 CA management and renewal fixes

2014-09-23 Thread Rob Crittenden
Jan Cholasta wrote:
 Hi,
 
 the attached patches fix various bugs and shortcomings in the CA
 management and renewal code. Related tickets:
 https://fedorahosted.org/freeipa/ticket/4416,
 https://fedorahosted.org/freeipa/ticket/4460.
 
 (Patch 319 was originally posted at
 http://www.redhat.com/archives/freeipa-devel/2014-September/msg00132.html.)

Only two of the patches includes what ticket(s) they address. Most have
the tersest of commit messages. If got more and more difficult to see
why the changes were needed at all, as you'll see.

As a side note, it was rather difficult to review parts of this as
different patches modify exactly the same code in different ways.

General:

These patches don't replace /etc/pki/nssdb, they just add a new
location. Do we need to keep using the shared db because of p11-kit?

319:

The post-install script tries to get a cert with nickname 'IPA CA'. It
should also look for '$REALM IPA CA'. I'd use
ipapythhon.certdb.py:CA_NICKNAME_FMT. Having a similar option for
External CA is probably not a bad idea.

You don't need the password file to import a cert into an NSS database.

It may be too complex in a spec file but would it be better to try to
fetch things out of LDAP? This nearly screams for a separate script to
do the work.

Are these operations important enough to be logged to
/var/log/ipaupgrade.log? I suspect it's the kind of thing that might
fail and never be noticed.

At some point we'll upgrade to sql NSS databases. Is this going to
complicate things?

There are some places you do str(e[2]) that aren't necessary.

Might be worthwhile to clean up the comments regarding XML-RPC while
you're at it and just refer to RPC.

I'd rename ca_certs_nss to ca_certs_trust for clarity.

Is the separate helper create_ipa_nssdb() necessary? Should create_db()
be extended to do this extra work?

324:

ACK

325:

The exception from get_cert() is very rough so wouldn't cover the case
of file permissions, missing database, etc, but since the overall
behavior isn't changing, ACK. Might be worth a comment though indicating
a potential source of oddness for the uninitiated.

326:

ACK

327:

@@ -133,6 +116,7 @@ class CertUpdate(admintool.AdminTool):
 criteria = {
 'cert-database': dogtag_constants.ALIAS_DIR,
 'cert-nickname': nickname,
+'ca-name': 'dogtag-ipa-ca-renew-agent',
 }
 request_id = certmonger.get_request_id(criteria)
 if request_id is not None:

Doesn't seem related to this change.

328:

ACK

329:

ACK

330:

I don't understand the reference to ipa.p11-kit (an unowned file, btw).

Are you removing any cert that may be left over from some previous install?

If the file were to exist it could cause update-ca-trust to be executed
twice. Is that needed?

I think more clarity in the commit message will clear things up.

331:

ACK

332:

What is the reason for this refactoring?

333:

Seems reasonable but why would a CA profile change in the middle of a
request?

334:

Same boat, why?

335:

I kinda get the picture, along with previous patches, but need more info.

Some tips on testing would be helpful.

rob

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


[Freeipa-devel] Continuous Integration dependency tree testing

2014-09-23 Thread Petr Spacek
Hello,

The recent problem with Tomcat forced me to think how we can detect that some 
other package broke IPA before it hit users.

After all, it seems pretty easy.

0) Get a VM for testing purposes (preferably with minimal set of IPA 
dependencies). The VM has to be reverted to snapshot after every run. (This is 
pretty simple with RHEV-M/libvirt.)

1) Detect all changes in the minimal set of packages installed on the test VM.

It is easy because we can simply hash output of YUM/DNF upgrade command:

# echo n | dnf upgrade | md5sum  changeset.id

changeset.id will change every time when a package which is present on test 
system changes but stays the same if there were no updates from last run. 
(There could be false positives but it doesn't matter - the test will be simply 
run twice.)

2) if (changeset.id != previous_changeset.id)
run_tests()

3) This machinery could be triggered by cron or even by a message on fedmsg 
about a new package pushed to updates-testing...

Tests could use released versions of IPA (present in fedora-updates) to get 
better signal/noise ratio.

Okay, it is time to stop dreaming and go to bed. Good night from Europe :-)

-- 
Petr^2 Spacek

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


Re: [Freeipa-devel] Krb service delegation rules in CLI

2014-09-23 Thread Alexander Bokovoy

On Tue, 23 Sep 2014, Martin Kosek wrote:

On 09/22/2014 09:48 PM, Alexander Bokovoy wrote:

On Mon, 22 Sep 2014, Martin Basti wrote:

Hello,

Related ticket: https://fedorahosted.org/freeipa/ticket/3644


1) API

The ipaKrb5DelegationACL objectclass requires targets which are stored in
extra objectclass.

A) we allow users to create groups of principals and then associate them as
targets -- user can use same group for multiple delegation ACL

B) users specify only list of target principals (no groups)

B seems better to me.

No.

You have three clearly separate object types here:

1. Principals -- any object in the tree that has krbPrincipalAux
supplemental auxiliary object class. These objects can be anywhere in
the tree, we can decide on limiting subtrees to search.

2. Delegation ACLs -- any object that has ipaKrb5DelegationACL
supplemental structural object class. These objects can be anywhere
under cn=s4u2proxy,$SUFFIX

3. Groups of principals  -- any object that has groupOfPrincipals _and_
doesn't have ipaKrb5DelegationACL object classes. These objects can be
anywhere in the tree, we can decide on limiting subtrees to search and
can decide on a specific subtree to put new groups of principals.

For (1) you just need to be able to reference principals, this only
requires a search filter to define DN of the principal based on
krbPrincipalName or krbCanonicalName, with support for 'shortcuts'
(specifying principal without realm which is our realm).

(1) is only used to resolve principals for (2) and (3).

For (2) you need to implement a proper object handling, like any other
IPA object that supports:
 -- multiple values for ipaAllowToImpersonate
 -- multiple values for ipaAllowedTarget
CLI would look something like this:
 ipa krbdelegation-add
  -show
  -find
  -mod
  -add-source
  -add-target
  -del-source
  -del-target
  -del

For (3) you need to implement a proper object handling where creating
the object would happen in the predefined subtree but search or modify
would be possible for any object matched by a proper filter.

CLI would look something like this:
 ipa krbprincipalgroup-add
  -show
  -find
  -mod
  -add-member
  -del-member


The way we use right now groupOfPrincipals, the objects referenced are
placed in different subtrees, according to their primary role. That's
right, as krbPrincipalAux is an auxiliary class, and is added on top of
other object classes to turn other objects into principals. We also use
groupOfPrincipals not only for Kerberos delegation ACLs, that's why
support for multiple subtrees is needed.

Creating new groupOfPrincipals objects can be allowed in a single
designated place, cn=s4u2proxy,$SUFFIX, because this is primary use of
this API.


Alexander's proposal makes sense to me. I would just personally name the
commands differently to share the same prefix. I.e.

ipa krbdelegation-*
and
ipa krbdelegationgroup-*

as we are not creating general purpose principal groups but only groups for
S4U2Proxy usage.

I would also reconsider your request to search for the objects in the whole
s4u2proxy tree. Across FreeIPA framework, we handle objects in static and
defined location of our tree, not in the entire subtree.

You have to deal with the fact that we already have established uses and
need to support them. New entities can be created where they are but
existing ones cannot be moved for the sake of moving because DNs of some
objects are used in external config files.

--
/ Alexander Bokovoy

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


Re: [Freeipa-devel] Krb service delegation rules in CLI

2014-09-23 Thread Simo Sorce
On Tue, 23 Sep 2014 17:18:38 +0200
Martin Kosek mko...@redhat.com wrote:

 On 09/22/2014 09:48 PM, Alexander Bokovoy wrote:
  On Mon, 22 Sep 2014, Martin Basti wrote:
  Hello,
 
  Related ticket: https://fedorahosted.org/freeipa/ticket/3644
 
 
  1) API
 
  The ipaKrb5DelegationACL objectclass requires targets which are
  stored in extra objectclass.
 
  A) we allow users to create groups of principals and then
  associate them as targets -- user can use same group for multiple
  delegation ACL
 
  B) users specify only list of target principals (no groups)
 
  B seems better to me.
  No.
  
  You have three clearly separate object types here:
  
  1. Principals -- any object in the tree that has krbPrincipalAux
  supplemental auxiliary object class. These objects can be anywhere
  in the tree, we can decide on limiting subtrees to search.
  
  2. Delegation ACLs -- any object that has ipaKrb5DelegationACL
  supplemental structural object class. These objects can be anywhere
  under cn=s4u2proxy,$SUFFIX
  
  3. Groups of principals  -- any object that has groupOfPrincipals
  _and_ doesn't have ipaKrb5DelegationACL object classes. These
  objects can be anywhere in the tree, we can decide on limiting
  subtrees to search and can decide on a specific subtree to put new
  groups of principals.
  
  For (1) you just need to be able to reference principals, this only
  requires a search filter to define DN of the principal based on
  krbPrincipalName or krbCanonicalName, with support for 'shortcuts'
  (specifying principal without realm which is our realm).
  
  (1) is only used to resolve principals for (2) and (3).
  
  For (2) you need to implement a proper object handling, like any
  other IPA object that supports:
   -- multiple values for ipaAllowToImpersonate
   -- multiple values for ipaAllowedTarget
  CLI would look something like this:
   ipa krbdelegation-add
-show
-find
-mod
-add-source
-add-target
-del-source
-del-target
-del
  
  For (3) you need to implement a proper object handling where
  creating the object would happen in the predefined subtree but
  search or modify would be possible for any object matched by a
  proper filter.
  
  CLI would look something like this:
   ipa krbprincipalgroup-add
-show
-find
-mod
-add-member
-del-member
  
  
  The way we use right now groupOfPrincipals, the objects referenced
  are placed in different subtrees, according to their primary role.
  That's right, as krbPrincipalAux is an auxiliary class, and is
  added on top of other object classes to turn other objects into
  principals. We also use groupOfPrincipals not only for Kerberos
  delegation ACLs, that's why support for multiple subtrees is needed.
  
  Creating new groupOfPrincipals objects can be allowed in a single
  designated place, cn=s4u2proxy,$SUFFIX, because this is primary use
  of this API.
 
 Alexander's proposal makes sense to me. I would just personally name
 the commands differently to share the same prefix. I.e.
 
 ipa krbdelegation-*
 and
 ipa krbdelegationgroup-*
 
 as we are not creating general purpose principal groups but only
 groups for S4U2Proxy usage.
 
 I would also reconsider your request to search for the objects in the
 whole s4u2proxy tree. Across FreeIPA framework, we handle objects in
 static and defined location of our tree, not in the entire subtree.
 
 I think we should although indeed consider adding group of principals
 to own nsContainer (like cn=targets) as otherwise we will have a
 clash between krbdelegation rule names and krbdelegationgroup names.

Keep in mind that we already have a few rules in there and the groups
are in the same container, the names are different as the groups are
named rule-name-targets, so we'll have to keep using a subtree
search, but that is fine cn=s4u2proxy has no subtree atm.

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] 755 webui-ci: case-insensitive record check

2014-09-23 Thread Endi Sukma Dewata

On 9/22/2014 9:49 AM, Petr Vobornik wrote:

[PATCH] webui-ci: case-insensitive record check

Indirect association are no longer lower cased, which caused a issue in CI.


Is the use of |= operator intentional? I don't see the has variable 
defined anywhere else in this method.


  has |= self.has_record(pkey.lower(), parent, table_name)

If this has been tested to work, then ACK.

--
Endi S. Dewata

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


Re: [Freeipa-devel] [PATCH] 756 webui: fix regression in association facet preop

2014-09-23 Thread Endi Sukma Dewata

On 9/22/2014 9:50 AM, Petr Vobornik wrote:

Association facet specs use 'add_method' instead of 'add_command'

origin: https://fedorahosted.org/freeipa/ticket/4507


ACK.

--
Endi S. Dewata

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