Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Ludwig Krispenz


On 06/03/2015 11:37 AM, Martin Babinsky wrote:

Hi everyone,

I have been playing with the topology related patches and I have 
encountered a few issues that I would like to address in this thread:


1.) When replica install for whatever reason crashes _after_ the setup 
of replication agreements etc., it leaves the topology plugin with 
dangling segment pointing to the dysfunctional node. An attempt to 
delete it leads to:



ipa: ERROR: Server is unwilling to perform: Removal of Segment 
disconnects topology.Deletion not allowed.
if the endpoints of the segments are still in the managed master list 
and there is no other path connecting these two nodes the behaviour is 
correct.
you need to remove the master first, teh segment should be removed 
automatically.
ipa-replica-manage del should do this, it worked for me  with the latest 
patches.


can you provide a scenario where it fails ?



And you cannot reinstall the crashed replica because it complains 
about existing replication agreements. It would probably help to be 
able to force-remove the segments if one of the endpoints doesn't 
exist/respond.


2.) I was not able to figure out a way remove replica from the 
topology without explosions or tampering 
'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del 
doesn't work anymore (I have tried just for fun, it leads to SIGSEGV 
of the host's dirsrv and leaves dangling segments to offending 
replica, leading to point 1).


I managed to remove replica from the topology only by directly 
uninstalling FreeIPA on the node and then deleting its' host entry 
from 'cn=masters'. Only after this was the plugin able to 
automagically removed the segments pointing to/from removed node.


The design page suggests that it should be enough to uninstall IPA 
server on the replica. The plugin would then pick-up the dangling 
segments and remove them automatically. However, this behavior seems 
to require additional modification of the uninstall procedure (e.g. 
the uninstalling replica should remove its' entry from cn=masters).


3.) It seems that the removal of topology suffixes containing 
functioning segments is not handled well. I once tried to do this and 
it led to segmentation fault on the dirsrv instance. What is the 
expected behavior in this scenario?




--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 857 topology: ipa management commands

2015-06-03 Thread Petr Vobornik

On 06/03/2015 10:59 AM, Martin Babinsky wrote:

On 06/03/2015 10:52 AM, Martin Babinsky wrote:

On 05/26/2015 03:31 PM, Petr Vobornik wrote:

On 05/26/2015 12:19 PM, Petr Vobornik wrote:

this patch is based on top of my patch #856 and tbabej'
s 325-9.

Obsoletes Ludwig's 0006.

ipalib part of topology management

Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology

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




New version attached:
- domainlevel_show usage changed to domainlevel_get
- updated VERSION
- added more attrs to default_attributes




Hi Petr,

the commands themselves seem to work just fine. I had encountered some
quirks in the underlying topology plugin, but I will address them in a
different thread in order to keep the discussion relevant to the
reviewed patch.

I have some minor coomments below:

1.)
  IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: pvoborni - added topology management commands

Several people were touching API in the meantime so please double-check
that you have correct VERSION and regenerate API.txt


Patch rebased.



2.)

+Str(
+'nsds5replicatedattributelist?',
+cli_name='replattrs',
+label='Attributes to replicate',
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a fractional update. E.g., `(objectclass=*) '
+  '$ EXCLUDE accountlockout memberof'),
+),
+Str(
+'nsds5replicatedattributelisttotal?',
+cli_name='replattrstotal',
+label=_('Attributes for total update'),
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a total update. E.g. (objectclass=*) $
EXCLUDE '
+  'accountlockout'),

The descriptions of these two options confused me greatly, are these
attributes supposed to be replicated or not, or is there some more
complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he
can probably explain them to us and then we can decide whether we may
alter the descriptions to be less confusing.

3.)

+takes_params = (
+Str(
+'cn',
+cli_name='name',
+primary_key=True,
+label=_('Suffix name'),
+),
+Str(
+'iparepltopoconfroot',
+maxlength=255,
+cli_name='suffix',
+label=_('Suffix to be managed'),
+normalizer=lambda value: value.lower(),
+),
+)

This also confused me at first, I suggest to change the label of
'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or
'LDAP subtree to be managed'.


Changed to 'LDAP suffix to be managed'



4.)

There is currently no way to rename existing topology segments/suffixes.
In the case of hosts with funky FQDN's (pointing at you, ABC lab), the
segment cn's created during replica installs are mearly impossible to
remember and it would be nice to rename them to something more
manageable. However, this is not related to core functionality and can
be a subject of a separate patch once this gets pushed.

That's all from my side.



I also forgot to ask what is the expected policy when deleting a
non-empty topology suffix. If this is not supported and you have to
first remove all segments and then the suffix itself, the
'topologysuffix-del' command should issue an error pointing the user to
correct procedure.



Do we have a use case for creation or deletion of topology suffix?
--
Petr Vobornik
From ea383de2037b63e0ec725fff1fbd7bd69673d40d Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 22 May 2015 09:50:09 +0200
Subject: [PATCH] topology: ipa management commands

ipalib part of topology management

Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology

https://fedorahosted.org/freeipa/ticket/4302
---
 API.txt| 155 ++
 VERSION|   4 +-
 ipalib/constants.py|   1 +
 ipalib/plugins/topology.py | 383 +
 4 files changed, 541 insertions(+), 2 deletions(-)
 create mode 100644 ipalib/plugins/topology.py

diff --git a/API.txt b/API.txt
index 6520f2c428342cdd30b0db830ed4ddbc89e4302a..0e42fadc66c129e53c3860fb7eeec69c1f148147 100644
--- a/API.txt
+++ b/API.txt
@@ -4494,6 +4494,161 @@ option: Str('version?', exclude='webui')
 output: Entry('result', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
+command: topologysegment_add
+args: 2,13,3
+arg: Str('topologysuffixcn', cli_name='topologysuffix', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('cn', attribute=True, cli_name='name', maxlength=255, 

Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Ludwig Krispenz


On 06/03/2015 01:32 PM, Oleg Fayans wrote:

Hi Ludwig

On 06/03/2015 12:23 PM, Ludwig Krispenz wrote:


On 06/03/2015 11:51 AM, Oleg Fayans wrote:

I confirm every point of this.
did you test with all the latest patches applied ? In your issues you 
refer to crashes, the crashes reported should be resolved, if you 
still have crashes, pleas provide a core dump or scenario to 
reproduce the crash.

With patch0009 ipa-replica-manage del worked for me

Yep, patch 0009 is applied.
The full list of patches applied on top of the master branch (at it's 
state yesterday at 10 PM) is as follows:

freeipa-lkrispen-0007-replica-install-fails-with-domain-level-1.patch
freeipa-lkrispen-0008-plugin-uses-1-as-minimum-domain-level-to-become-acti.patch 


freeipa-lkrispen-0009-crash-when-removing-a-replica.patch
freeipa-mbasti-0262-Installers-fix-remove-temporal-ccache.patch
freeipa-pvoborni-0857-1-topology-ipa-management-commands.patch
freeipa-pvoborni-0858-1-webui-IPA.command_dialog-a-new-dialog-base-class.patch 

freeipa-pvoborni-0859-1-webui-use-command_dialog-as-a-base-class-for-passwor.patch 

freeipa-pvoborni-0860-1-webui-make-usage-of-all-in-details-facet-optional.patch 


freeipa-pvoborni-0861-2-webui-topology-plugin.patch
freeipa-pvoborni-0862-webui-configurable-refresh-command.patch

The scenario is pretty basic:
1. 3 fedora-21 vms with the latest directory server packages from 
mreynolds repo:

389-ds-base-2015_06_02-1.fc21.x86_64

2. setup master on one of them, prepare gpg files for two replicas
3. setup replicas using these gpg files.
4. Try to remove one of the replicas using command `ipa 
topologysegment-del`

this should remove a segment, not a replica and it should be rejected

5. Try to create a new user via web UI on any of the replicas





On 06/03/2015 11:37 AM, Martin Babinsky wrote:

Hi everyone,

I have been playing with the topology related patches and I have 
encountered a few issues that I would like to address in this thread:


1.) When replica install for whatever reason crashes _after_ the 
setup of replication agreements etc., it leaves the topology plugin 
with dangling segment pointing to the dysfunctional node. An 
attempt to delete it leads to:



ipa: ERROR: Server is unwilling to perform: Removal of Segment 
disconnects topology.Deletion not allowed.


Furthermore, any attempts to delete a segment (even a properly setup 
one) lead to the same very error.


And you cannot reinstall the crashed replica because it complains 
about existing replication agreements. It would probably help to be 
able to force-remove the segments if one of the endpoints doesn't 
exist/respond.


2.) I was not able to figure out a way remove replica from the 
topology without explosions or tampering 
'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage 
del doesn't work anymore (I have tried just for fun, it leads to 
SIGSEGV of the host's dirsrv and leaves dangling segments to 
offending replica, leading to point 1).


I managed to remove replica from the topology only by directly 
uninstalling FreeIPA on the node and then deleting its' host entry 
from 'cn=masters'. Only after this was the plugin able to 
automagically removed the segments pointing to/from removed node.


The design page suggests that it should be enough to uninstall IPA 
server on the replica. The plugin would then pick-up the dangling 
segments and remove them automatically. However, this behavior 
seems to require additional modification of the uninstall procedure 
(e.g. the uninstalling replica should remove its' entry from 
cn=masters).


3.) It seems that the removal of topology suffixes containing 
functioning segments is not handled well. I once tried to do this 
and it led to segmentation fault on the dirsrv instance. What is 
the expected behavior in this scenario?










--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Oleg Fayans

I confirm every point of this.

On 06/03/2015 11:37 AM, Martin Babinsky wrote:

Hi everyone,

I have been playing with the topology related patches and I have 
encountered a few issues that I would like to address in this thread:


1.) When replica install for whatever reason crashes _after_ the setup 
of replication agreements etc., it leaves the topology plugin with 
dangling segment pointing to the dysfunctional node. An attempt to 
delete it leads to:



ipa: ERROR: Server is unwilling to perform: Removal of Segment 
disconnects topology.Deletion not allowed.


Furthermore, any attempts to delete a segment (even a properly setup 
one) lead to the same very error.


And you cannot reinstall the crashed replica because it complains 
about existing replication agreements. It would probably help to be 
able to force-remove the segments if one of the endpoints doesn't 
exist/respond.


2.) I was not able to figure out a way remove replica from the 
topology without explosions or tampering 
'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del 
doesn't work anymore (I have tried just for fun, it leads to SIGSEGV 
of the host's dirsrv and leaves dangling segments to offending 
replica, leading to point 1).


I managed to remove replica from the topology only by directly 
uninstalling FreeIPA on the node and then deleting its' host entry 
from 'cn=masters'. Only after this was the plugin able to 
automagically removed the segments pointing to/from removed node.


The design page suggests that it should be enough to uninstall IPA 
server on the replica. The plugin would then pick-up the dangling 
segments and remove them automatically. However, this behavior seems 
to require additional modification of the uninstall procedure (e.g. 
the uninstalling replica should remove its' entry from cn=masters).


3.) It seems that the removal of topology suffixes containing 
functioning segments is not handled well. I once tried to do this and 
it led to segmentation fault on the dirsrv instance. What is the 
expected behavior in this scenario?




--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates

2015-06-03 Thread Martin Basti

On 02/06/15 16:03, Jan Cholasta wrote:

Dne 2.6.2015 v 12:36 Martin Basti napsal(a):

On 02/06/15 11:42, Fraser Tweedale wrote:

On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote:

On 01/06/15 06:40, Fraser Tweedale wrote:

New version of patch; ``{host,service}-show --out=FILE`` now writes
all certs to FILE.  Rebased on latest master.

Thanks,
Fraser

On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote:

Updated patch attached.  Notably restores/adds revocation behaviour
to host-mod and service-mod.

Thanks,
Fraser

On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:

On 27/05/15 15:53, Fraser Tweedale wrote:
This patch adds supports for multiple user / host 
certificates.  No

schema change is needed ('usercertificate' attribute is already
multi-value).  The revoke-previous-cert behaviour of host-mod and
user-mod has been removed but revocation behaviour of -del and
-disable is preserved.

The latest profiles/caacl patchset (0001..0013 v5) depends on this
patch for correct cert-request behaviour.

There is one design question (or maybe more, let me know): the
`--out=FILENAME' option to {host,service} show saves ONE 
certificate

to the named file.  I propose to either:

a) write all certs, suffixing suggested filename with either a
sequential numerical index, e.g. cert.pem becomes
cert.pem.1, cert.pem.2, and so on; or

b) as above, but suffix with serial number and, if there are
different issues, some issuer-identifying information.

Let me know your thoughts.

Thanks,
Fraser



Is there a possible way how to store certificates into one file?
I read about possibilities to have multiple certs in one .pem
file, but I'm
not cert guru :)

I personally vote for serial number in case there are multiple
certificates,
if ^ is no possible.


1)
+if len(certs)  0:

please use only,
if certs:

2)
You need to re-generate API/ACI.txt in this patch

3)
syntax error:
+for dercert in certs_der


4)
command
ipa user-mod ca_user --certificate=ceritifcate

removes the current certificate from the LDAP, by design.
Should be the old certificate(s) revoked? You removed that part in
the code.

only the --addattr='usercertificate=cert' appends new value there

--
Martin Basti


My objections/proposed solutions in attached patch.

* VERSION
* In the previous version normalized values was stored in LDAP, so I
added
it back.  (I dont know why there is no normalization in param
settings, but
normalization for every certificate is done in callback. I will file a
ticket for this)
* IMO only normalized certificates should be compared in the old
certificates detection


I incorporated your suggested changes in new patch (attached).

There were no proposed changes to the other patchset (0001..0013)
since rebase.

Thanks,
Fraser

Thank you,
ACK
Martin^2



Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212


Regression found.

Patch to fix the issue is attached.

--
Martin Basti

From 6b489fc6a04b60a38e768009558fe1d9b1164b45 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Wed, 3 Jun 2015 13:11:58 +0200
Subject: [PATCH] Fix: regression in host and service plugin

Test failures:
 * wrong error message
 * mod operation always delete usercertificates

https://fedorahosted.org/freeipa/ticket/4238
---
 ipalib/plugins/host.py| 10 +++---
 ipalib/plugins/service.py | 11 +++
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py
index 9ad087e26250d86b15fbe723a98cca278ef29adf..e81dca94e124b080a3d68a3b1cfd079710e30336 100644
--- a/ipalib/plugins/host.py
+++ b/ipalib/plugins/host.py
@@ -871,8 +871,11 @@ class host_mod(LDAPUpdate):
 x509.verify_cert_subject(ldap, keys[-1], cert)
 
 # revoke removed certificates
-if self.api.Command.ca_is_enabled()['result']:
-entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
+if certs and self.api.Command.ca_is_enabled()['result']:
+try:
+entry_attrs_old = ldap.get_entry(dn, ['usercertificate'])
+except errors.NotFound:
+self.obj.handle_not_found(*keys)
 old_certs = entry_attrs_old.get('usercertificate', [])
 old_certs_der = map(x509.normalize_certificate, old_certs)
 removed_certs_der = set(old_certs_der) - set(certs_der)
@@ -899,7 +902,8 @@ class host_mod(LDAPUpdate):
   nsprerr.args[1])
 else:
 raise nsprerr
-entry_attrs['usercertificate'] = certs_der
+if certs:
+entry_attrs['usercertificate'] = certs_der
 
 if options.get('random'):
 entry_attrs['userpassword'] = ipa_generate_password(characters=host_pwd_chars)
diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index c290344cf6c14155ec1b103525ff8642a7a8e2af..d8bd03523089cd8446377d6b0c85bc318e69b809 100644
--- 

Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates

2015-06-03 Thread Milan Kubik

On 06/03/2015 01:17 PM, Martin Basti wrote:

On 02/06/15 16:03, Jan Cholasta wrote:

Dne 2.6.2015 v 12:36 Martin Basti napsal(a):

On 02/06/15 11:42, Fraser Tweedale wrote:

On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote:

On 01/06/15 06:40, Fraser Tweedale wrote:

New version of patch; ``{host,service}-show --out=FILE`` now writes
all certs to FILE.  Rebased on latest master.

Thanks,
Fraser

On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote:

Updated patch attached.  Notably restores/adds revocation behaviour
to host-mod and service-mod.

Thanks,
Fraser

On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:

On 27/05/15 15:53, Fraser Tweedale wrote:
This patch adds supports for multiple user / host 
certificates.  No

schema change is needed ('usercertificate' attribute is already
multi-value).  The revoke-previous-cert behaviour of host-mod and
user-mod has been removed but revocation behaviour of -del and
-disable is preserved.

The latest profiles/caacl patchset (0001..0013 v5) depends on 
this

patch for correct cert-request behaviour.

There is one design question (or maybe more, let me know): the
`--out=FILENAME' option to {host,service} show saves ONE 
certificate

to the named file.  I propose to either:

a) write all certs, suffixing suggested filename with either a
sequential numerical index, e.g. cert.pem becomes
cert.pem.1, cert.pem.2, and so on; or

b) as above, but suffix with serial number and, if there are
different issues, some issuer-identifying information.

Let me know your thoughts.

Thanks,
Fraser



Is there a possible way how to store certificates into one file?
I read about possibilities to have multiple certs in one .pem
file, but I'm
not cert guru :)

I personally vote for serial number in case there are multiple
certificates,
if ^ is no possible.


1)
+if len(certs)  0:

please use only,
if certs:

2)
You need to re-generate API/ACI.txt in this patch

3)
syntax error:
+for dercert in certs_der


4)
command
ipa user-mod ca_user --certificate=ceritifcate

removes the current certificate from the LDAP, by design.
Should be the old certificate(s) revoked? You removed that part in
the code.

only the --addattr='usercertificate=cert' appends new value 
there


--
Martin Basti


My objections/proposed solutions in attached patch.

* VERSION
* In the previous version normalized values was stored in LDAP, so I
added
it back.  (I dont know why there is no normalization in param
settings, but
normalization for every certificate is done in callback. I will 
file a

ticket for this)
* IMO only normalized certificates should be compared in the old
certificates detection


I incorporated your suggested changes in new patch (attached).

There were no proposed changes to the other patchset (0001..0013)
since rebase.

Thanks,
Fraser

Thank you,
ACK
Martin^2



Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212


Regression found.

Patch to fix the issue is attached.


The fix works, thanks.

Milan

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Jan Cholasta

Dne 2.6.2015 v 20:40 Simo Sorce napsal(a):

On Tue, 2015-06-02 at 07:07 -0500, Endi Sukma Dewata wrote:

On 6/2/2015 1:10 AM, Martin Kosek wrote:

Hi Endi,

Quickly skimming through your patches raised couple questions on my side:

1) Will it be possible to also store plain text password via Vault? It
talks about taking in the binary data or the text file, but will it also
work with plain user secrets (passwords)? I am talking about use like this:

# ipa vault-archive name --user mkosek --data Secret123


For security the plain text password should be stored in a file first:

# vi password.txt
# ipa vault-archive name --user mkosek --in password.txt

It's also possible to specify the password as base-64 encoded data:

# echo -n Secret123 | base64
# ipa vault-archive name --user mkosek --data U2VjcmV0MTIz

But it's not recommended since the data will be stored in the command
history and someone could see and decode it. I think passing a plain
text password as command line argument would be even worse. The --data
parameter is mainly used for unit testing.

Later we might be able to add an option to read from standard input:

# cat password.txt | ipa vault-archive name --user mkosek --std-in


Yes please, a way to pass in via stdin is extremely useful, as leaving
files on the filesystem is also a big risk.


This will not work well, it should use the normal prompting mechanism:

$ ipa vault-archive name --user user
Data: type data here

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Martin Kosek
On 06/02/2015 11:22 PM, Alexander Bokovoy wrote:
 On Tue, 02 Jun 2015, Endi Sukma Dewata wrote:
 Please take a look at the new patch.

 On 6/2/2015 10:05 AM, Martin Kosek wrote:
 4) In the vault-archive forward method, you use pki module. However,
 this module will be only available on FreeIPA PKI-powered servers and
 not on FreeIPA clients - so this will not work unless freeipa-client
 gets a dependency on pki-base - which is definitely not something we
 want...

 In my opinion it should be fine to require pki-base on the client because 
 it
 contains just the client library, unless you have other concerns? Any
 objections to having pki-nss and pki-cryptography dependencies on the 
 client?

 Even if we can change the client code not to depend on pki module, since 
 in
 this framework the client and server code are written in the same plugin, 
 the
 import pki still cannot be removed since it's still needed by the server
 code, and I don't think conditional import is a good programming practice.

 I have major concerns here. Look at the different between installing
 freeipa-client and freeipa-client + pki-base on my F21:

 ~~
 $ sudo yum install freeipa-client
 ...
 Install  1 Package (+4 Dependent packages)

 Total download size: 2.6 M
 Installed size: 14 M
 ~~
 $ sudo yum install freeipa-client pki-base
 ...
 Install  2 Packages (+288 Dependent packages)

 Total download size: 160 M
 Installed size: 235 M
 ~~

 This is obviously a no-go for client. The conditional import is smaller 
 concern
 that big dependency growth on the client. We do them in trust plugin for
 example and it works fine (though I agree it is not ideal programming
 practice).

 IMO, we should limit new freeipa-client dependencies only to
 python-cryptography (or also python-nss in the worst case, in case
 python-cryptography is not enough) - there should be no pki dependencies at
 all, these should be only on the server side.

 OK. I opened a ticket to split the pki-base into separate Python and Java
 packages:
 https://fedorahosted.org/pki/ticket/1399

 For now in this patch I added conditional imports for pki.account and pki.key
 which are needed to access KRA on the server side. I removed dependency on
 pki.crypto on the client side and replaced it with direct python-nss code.

 On 6/2/2015 1:40 PM, Simo Sorce wrote:
 I have coded in python (jwcrypto)
 support for some key wrapping not yet available in python-cryptography
 and can lend you the code as needed.

 Thanks. I'll get back to you when it's time to add support for
 python-cryptography in KRA:
 https://fedorahosted.org/pki/ticket/1400

 On 6/2/2015 10:16 AM, Alexander Bokovoy wrote:
 Yes, please use conditional import here, it is perfectly valid use case
 for the client side.

 On 6/2/2015 1:40 PM, Simo Sorce wrote:
 conditional import is just fine

 The conditional imports that I've seen usually are used for importing
 different versions of the same module, which I think is acceptable because
 the dependency always exists. In the vault case we're selectively importing a
 module depending on where the code runs. I think that is bad because it adds
 complexity and it's easy to make mistakes. Any code that depends on that
 module would have to be (a) guarded:

  if pki_is_loaded:
  ... call pki ...

 or (b) used in a method that's only called if the module is loaded:

  def do_something(self): # runs only on server
  ... call pki ...

 The (a) is similar to #ifdef's which should be avoidable using OOD, and in
 (b) we may inadvertently call a wrong method indirectly. I think ideally the
 client and server code should be in separate files (so they can be deployed
 separately too), but the framework doesn't seem to allow that.
 This exactly the case we have to use here and we are using that in
 trusts case as well -- some code has to run on server only and shouldn't
 cause to install Samba related packages on the client. This is because
 IPA client is actually using the same IPA plugins that server uses, to
 have access to the API calls metadata and client-side callbacks are
 defined in the same place where server-side callbacks are. It is IPA
 framework design, so we have to use what we have.

This is planned to be changed BTW, when we start with the Thin Client concept
and have different code/plugins for FreeIPA server side and client side.

 In other words, it is not necessarily an evil under conditions we are
 dealing with.
 

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Database error on replicas

2015-06-03 Thread Martin Kosek
On 06/03/2015 10:33 AM, Oleg Fayans wrote:
 Hi,
 
 With the latest freeipa code containing Topology plugin patches, I am unable 
 to
 make any changes in replicas.
 
 I have the following topology:
 replica1 = master = replica3
 Here is the output of the ipa topologysegment-find command:
 
 Suffix name: realm
 --
 2 segments matched
 --
   Segment name: replica1.zaeba.li-to-testmaster.zaeba.li
   Left node: replica1.zaeba.li
   Right node: testmaster.zaeba.li
   Connectivity: both
 
   Segment name: replica3.zaeba.li-to-testmaster.zaeba.li
   Left node: replica3.zaeba.li
   Right node: testmaster.zaeba.li
   Connectivity: both
 
 Number of entries returned 2
 
 
 
 Any changes on master get replicated to replicas successfully. However, any
 attempts to change anything on replicas, for example, create a user, result in
 the error message about DatabaseError (attached).
 
 The corresponding part of the dirsrv log looks like this:
 
 03/Jun/2015:04:11:55 -0400] slapi_ldap_bind - Error: could not perform
 interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't
 contact LDAP server)
 [03/Jun/2015:04:15:02 -0400] slapi_ldap_bind - Error: could not send startTLS
 request: error -1 (Can't contact LDAP server) errno 0 (Success)
 [03/Jun/2015:04:16:55 -0400] slapd_ldap_sasl_interactive_bind - Error: could
 not perform interactive bind for id [] mech [GSSAPI]: LDAP error -1 (Can't
 contact LDAP server) ((null)) errno 2 (No such file or directory)
 [03/Jun/2015:04:16:55 -0400] slapi_ldap_bind - Error: could not perform
 interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't
 contact LDAP server)
 
 The full log is attached

Ludwig, could this be caused by the Topology plugin?

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 857 topology: ipa management commands

2015-06-03 Thread Martin Babinsky

On 05/26/2015 03:31 PM, Petr Vobornik wrote:

On 05/26/2015 12:19 PM, Petr Vobornik wrote:

this patch is based on top of my patch #856 and tbabej'
s 325-9.

Obsoletes Ludwig's 0006.

ipalib part of topology management

Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology

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




New version attached:
- domainlevel_show usage changed to domainlevel_get
- updated VERSION
- added more attrs to default_attributes




Hi Petr,

the commands themselves seem to work just fine. I had encountered some 
quirks in the underlying topology plugin, but I will address them in a 
different thread in order to keep the discussion relevant to the 
reviewed patch.


I have some minor coomments below:

1.)
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: pvoborni - added topology management commands

Several people were touching API in the meantime so please double-check 
that you have correct VERSION and regenerate API.txt


2.)

+Str(
+'nsds5replicatedattributelist?',
+cli_name='replattrs',
+label='Attributes to replicate',
+doc=_('Attributes that are not replicated to a consumer 
server '

+  'during a fractional update. E.g., `(objectclass=*) '
+  '$ EXCLUDE accountlockout memberof'),
+),
+Str(
+'nsds5replicatedattributelisttotal?',
+cli_name='replattrstotal',
+label=_('Attributes for total update'),
+doc=_('Attributes that are not replicated to a consumer 
server '

+  'during a total update. E.g. (objectclass=*) $ EXCLUDE '
+  'accountlockout'),

The descriptions of these two options confused me greatly, are these 
attributes supposed to be replicated or not, or is there some more 
complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he 
can probably explain them to us and then we can decide whether we may 
alter the descriptions to be less confusing.


3.)

+takes_params = (
+Str(
+'cn',
+cli_name='name',
+primary_key=True,
+label=_('Suffix name'),
+),
+Str(
+'iparepltopoconfroot',
+maxlength=255,
+cli_name='suffix',
+label=_('Suffix to be managed'),
+normalizer=lambda value: value.lower(),
+),
+)

This also confused me at first, I suggest to change the label of 
'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or 
'LDAP subtree to be managed'.


4.)

There is currently no way to rename existing topology segments/suffixes. 
In the case of hosts with funky FQDN's (pointing at you, ABC lab), the 
segment cn's created during replica installs are mearly impossible to 
remember and it would be nice to rename them to something more 
manageable. However, this is not related to core functionality and can 
be a subject of a separate patch once this gets pushed.


That's all from my side.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 857 topology: ipa management commands

2015-06-03 Thread Martin Babinsky

On 06/03/2015 10:52 AM, Martin Babinsky wrote:

On 05/26/2015 03:31 PM, Petr Vobornik wrote:

On 05/26/2015 12:19 PM, Petr Vobornik wrote:

this patch is based on top of my patch #856 and tbabej'
s 325-9.

Obsoletes Ludwig's 0006.

ipalib part of topology management

Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology

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




New version attached:
- domainlevel_show usage changed to domainlevel_get
- updated VERSION
- added more attrs to default_attributes




Hi Petr,

the commands themselves seem to work just fine. I had encountered some
quirks in the underlying topology plugin, but I will address them in a
different thread in order to keep the discussion relevant to the
reviewed patch.

I have some minor coomments below:

1.)
  IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: pvoborni - added topology management commands

Several people were touching API in the meantime so please double-check
that you have correct VERSION and regenerate API.txt

2.)

+Str(
+'nsds5replicatedattributelist?',
+cli_name='replattrs',
+label='Attributes to replicate',
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a fractional update. E.g., `(objectclass=*) '
+  '$ EXCLUDE accountlockout memberof'),
+),
+Str(
+'nsds5replicatedattributelisttotal?',
+cli_name='replattrstotal',
+label=_('Attributes for total update'),
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a total update. E.g. (objectclass=*) $ EXCLUDE '
+  'accountlockout'),

The descriptions of these two options confused me greatly, are these
attributes supposed to be replicated or not, or is there some more
complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he
can probably explain them to us and then we can decide whether we may
alter the descriptions to be less confusing.

3.)

+takes_params = (
+Str(
+'cn',
+cli_name='name',
+primary_key=True,
+label=_('Suffix name'),
+),
+Str(
+'iparepltopoconfroot',
+maxlength=255,
+cli_name='suffix',
+label=_('Suffix to be managed'),
+normalizer=lambda value: value.lower(),
+),
+)

This also confused me at first, I suggest to change the label of
'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or
'LDAP subtree to be managed'.

4.)

There is currently no way to rename existing topology segments/suffixes.
In the case of hosts with funky FQDN's (pointing at you, ABC lab), the
segment cn's created during replica installs are mearly impossible to
remember and it would be nice to rename them to something more
manageable. However, this is not related to core functionality and can
be a subject of a separate patch once this gets pushed.

That's all from my side.



I also forgot to ask what is the expected policy when deleting a 
non-empty topology suffix. If this is not supported and you have to 
first remove all segments and then the suffix itself, the 
'topologysuffix-del' command should issue an error pointing the user to 
correct procedure.


--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Martin Kosek
On 06/02/2015 08:34 PM, Simo Sorce wrote:
 On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote:
 Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a):
 On 5/28/2015 12:46 AM, Jan Cholasta wrote:
 On a related note, since KRA is optional, can we move the vaults
 container to cn=kra,cn=vaults? This is the convetion used by the other
 optional components (DNS and recently CA).

 I mean cn=vaults,cn=kra of course.

 If you are talking about the o=kra,PKI suffix, I'm not sure whether
 the IPA framework will work with it.

 If you are talking about adding a new cn=kra,IPA suffix entry on top
 of cn=vaults, what is the purpose of this entry? Is the entry going to
 be created/deleted automatically when the KRA is installed/removed? Is
 it going to be used for something else other than vaults?

 I'm talking about cn=kra,IPA suffix. It should be created only when 
 KRA is installed, although I think this can be done later after the 
 release, moving vaults to cn=kra should be good enough for now. It's 
 going to be used for everything KRA-specific.


 There are a lot of questions that need to be answered before we can make
 this change.

 This is about sticking to a convention, which everyone should do, and 
 everyone except KRA already does.

 I'm sorry I didn't realize this earlier, but the change must be done now.

 We probably should revisit this issue after the core vault
 functionality is added.


 We can't revisit it later because after release we are stuck with 
 whatever is there forever.

 See attachment for a patch which implements the change.

 
 Shouldn't we s/kra/vault/ ?
 After all the feature is called Vault, not KRA.

I thought we are naming it by the name of the optional subsystem, not the
feature itself. If for example, another feature from KRA is used, it would
still live in cn=kra, no?

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Database error on replicas

2015-06-03 Thread Ludwig Krispenz


On 06/03/2015 10:44 AM, Martin Kosek wrote:

On 06/03/2015 10:33 AM, Oleg Fayans wrote:

Hi,

With the latest freeipa code containing Topology plugin patches, I am unable to
make any changes in replicas.

I have the following topology:
replica1 = master = replica3
Here is the output of the ipa topologysegment-find command:

Suffix name: realm
--
2 segments matched
--
   Segment name: replica1.zaeba.li-to-testmaster.zaeba.li
   Left node: replica1.zaeba.li
   Right node: testmaster.zaeba.li
   Connectivity: both

   Segment name: replica3.zaeba.li-to-testmaster.zaeba.li
   Left node: replica3.zaeba.li
   Right node: testmaster.zaeba.li
   Connectivity: both

Number of entries returned 2



Any changes on master get replicated to replicas successfully. However, any
attempts to change anything on replicas, for example, create a user, result in
the error message about DatabaseError (attached).

The corresponding part of the dirsrv log looks like this:

03/Jun/2015:04:11:55 -0400] slapi_ldap_bind - Error: could not perform
interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't
contact LDAP server)
[03/Jun/2015:04:15:02 -0400] slapi_ldap_bind - Error: could not send startTLS
request: error -1 (Can't contact LDAP server) errno 0 (Success)
[03/Jun/2015:04:16:55 -0400] slapd_ldap_sasl_interactive_bind - Error: could
not perform interactive bind for id [] mech [GSSAPI]: LDAP error -1 (Can't
contact LDAP server) ((null)) errno 2 (No such file or directory)
[03/Jun/2015:04:16:55 -0400] slapi_ldap_bind - Error: could not perform
interactive bind for id [] authentication mechanism [GSSAPI]: error -1 (Can't
contact LDAP server)

The full log is attached

Ludwig, could this be caused by the Topology plugin?

maybe, don't know yet





--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Database error on replicas

2015-06-03 Thread Martin Babinsky

On 06/03/2015 10:33 AM, Oleg Fayans wrote:

Hi,

With the latest freeipa code containing Topology plugin patches, I am
unable to make any changes in replicas.

I have the following topology:
replica1 = master = replica3
Here is the output of the ipa topologysegment-find command:

Suffix name: realm
--
2 segments matched
--
   Segment name: replica1.zaeba.li-to-testmaster.zaeba.li
   Left node: replica1.zaeba.li
   Right node: testmaster.zaeba.li
   Connectivity: both

   Segment name: replica3.zaeba.li-to-testmaster.zaeba.li
   Left node: replica3.zaeba.li
   Right node: testmaster.zaeba.li
   Connectivity: both

Number of entries returned 2



Any changes on master get replicated to replicas successfully. However,
any attempts to change anything on replicas, for example, create a user,
result in the error message about DatabaseError (attached).

The corresponding part of the dirsrv log looks like this:

03/Jun/2015:04:11:55 -0400] slapi_ldap_bind - Error: could not perform
interactive bind for id [] authentication mechanism [GSSAPI]: error -1
(Can't contact LDAP server)
[03/Jun/2015:04:15:02 -0400] slapi_ldap_bind - Error: could not send
startTLS request: error -1 (Can't contact LDAP server) errno 0 (Success)
[03/Jun/2015:04:16:55 -0400] slapd_ldap_sasl_interactive_bind - Error:
could not perform interactive bind for id [] mech [GSSAPI]: LDAP error
-1 (Can't contact LDAP server) ((null)) errno 2 (No such file or directory)
[03/Jun/2015:04:16:55 -0400] slapi_ldap_bind - Error: could not perform
interactive bind for id [] authentication mechanism [GSSAPI]: error -1
(Can't contact LDAP server)

The full log is attached




Hi Oleg,

could you also post the output of 'journalctl -xe' related to dirsrv (on 
master and also on replicas)? I have seen a couple of segfaults there 
during reviewing Petr Vobornik's topology* commands.


--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 1112 Add service constraint delegation plugin

2015-06-03 Thread Jan Cholasta

Dne 3.6.2015 v 11:34 Martin Basti napsal(a):

On 02/06/15 22:51, Rob Crittenden wrote:

Martin Basti wrote:

On 31/05/15 04:07, Rob Crittenden wrote:

Petr Vobornik wrote:

On 05/27/2015 08:17 PM, Martin Basti wrote:

On 27/05/15 19:27, Rob Crittenden wrote:

Martin Basti wrote:



Thank you.

I haven't finished review yet, but I have few notes in case you
will
modify the patch.

Please fix following issues:




3)
There are many PEP8 errors, can you fix some of them,?


Is PEP8 a concern? What kinds of errors do we fix? For example, the
current model for defining options generates a slew of indention
errors.


In old modules it's preferred to keep the old indentation style for
options(not to mix 2 styles). New modules should use following pep8
compliant style:
 Str(
 'cn',
 cli_name='name',
 primary_key=True,
 label=_('Server name'),
 doc=_('IPA server hostname'),
 ),


We try to keep PEP8 in new code, mainly indentation, blank lines, too
long lines.
Yes in test definitions and option definitions, is better to keep the
same style, but other parts of code should be PEP8.

For example these should be fixed
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:37:13: E225
missing whitespace around operator
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:39:1: E302
expected 2 blank lines, found 1
./ipatests/test_xmlrpc/test_serviceconstraint_plugin.py:42:1: E302
expected 2 blank lines, found 1




I'll wait and see what falls out of the API review before making any
real changes.

rob


Updated API and addressed Martin's concerns. The regex must have been
a bad copy/paste, it is fixed now.

The design page has been updated as well.

rob


Hello,

comments below, in the right thread:

1)
+Str(
+'memberprincipal',
+label=_('Failed principals'),
+),
+Str(
+'ipaallowedtarget',
+label=_('Failed targets'),
+),
+Str(
+'servicedelegationrule',
+label=_('principal member'),
+),
Are these names correct?
# ipa servicedelegationrule-find
--
1 service delegation rule matched
--
Delegation name: ipa-http-delegation
Allowed Target: ipa-ldap-delegation-targets,
ipa-cifs-delegation-targets
Failed principals: HTTP/vm-093.example@example.com


Fixed.




2)
+ pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$',
+pattern_errmsg='may only include letters, numbers, _, -,
., '
+   'and a space inside',

This regex does not allow space inside
In [6]: print re.match(pattern, 'lalalala lalala')
None


Fixed. I'm tempted to just drop this regex entirely. Other plugins
have no such restrictions, but this should work better now.



3)
+yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
+  label=_('member %s') % name,
+  csv=True, alwaysask=True)

IMHO CSV values should not be supported.
Honza told me, the option doesn't work anyway.


Yeah, a copy and paste issue.


Patch with minor fixes attached.

I removed unused code and PEP8 complains


Incorporated and fixed a number of other things, including some typos
in the doc examples.

rob




Thank you, ACK!



Pushed to master: a92328452dced34d6d6df7ad6fe585563bb909f6

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Martin Babinsky

On 06/03/2015 12:23 PM, Ludwig Krispenz wrote:


On 06/03/2015 11:51 AM, Oleg Fayans wrote:

I confirm every point of this.

did you test with all the latest patches applied ? In your issues you
refer to crashes, the crashes reported should be resolved, if you still
have crashes, pleas provide a core dump or scenario to reproduce the crash.
With patch0009 ipa-replica-manage del worked for me

I thing I have missed this patch before, I will test it again with patch 
0009 applied.


On 06/03/2015 11:37 AM, Martin Babinsky wrote:

Hi everyone,

I have been playing with the topology related patches and I have
encountered a few issues that I would like to address in this thread:

1.) When replica install for whatever reason crashes _after_ the
setup of replication agreements etc., it leaves the topology plugin
with dangling segment pointing to the dysfunctional node. An attempt
to delete it leads to:


ipa: ERROR: Server is unwilling to perform: Removal of Segment
disconnects topology.Deletion not allowed.


Furthermore, any attempts to delete a segment (even a properly setup
one) lead to the same very error.


And you cannot reinstall the crashed replica because it complains
about existing replication agreements. It would probably help to be
able to force-remove the segments if one of the endpoints doesn't
exist/respond.

2.) I was not able to figure out a way remove replica from the
topology without explosions or tampering
'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del
doesn't work anymore (I have tried just for fun, it leads to SIGSEGV
of the host's dirsrv and leaves dangling segments to offending
replica, leading to point 1).

I managed to remove replica from the topology only by directly
uninstalling FreeIPA on the node and then deleting its' host entry
from 'cn=masters'. Only after this was the plugin able to
automagically removed the segments pointing to/from removed node.

The design page suggests that it should be enough to uninstall IPA
server on the replica. The plugin would then pick-up the dangling
segments and remove them automatically. However, this behavior seems
to require additional modification of the uninstall procedure (e.g.
the uninstalling replica should remove its' entry from cn=masters).

3.) It seems that the removal of topology suffixes containing
functioning segments is not handled well. I once tried to do this and
it led to segmentation fault on the dirsrv instance. What is the
expected behavior in this scenario?








--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Petr Vobornik

On 06/03/2015 11:37 AM, Martin Babinsky wrote:

Hi everyone,

I have been playing with the topology related patches and I have
encountered a few issues that I would like to address in this thread:



Additional stuff:

1. was able to add duplicate segment
- same left and right node
- same direction
- different cn

It did not allow me to remove it:

Server is unwilling to perform: Removal of Segment disconnects 
topology.Deletion not allowed.



2. topology plugin allows to create reflexive relation from the invalid 
duplicates(#1):


A - B
A - B
to
A - A
B - B

I.E. effective disconnect

it is forbidden in `ipa topologysegment-mod` but I think that even the 
plugin should not allow that


3. attempt to delete the invalid reflexive or duplicate segment ends with:

Server is unwilling to perform: Removal of Segment disconnects 
topology.Deletion not allowed.



--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Oleg Fayans

Hi Ludwig

On 06/03/2015 12:23 PM, Ludwig Krispenz wrote:


On 06/03/2015 11:51 AM, Oleg Fayans wrote:

I confirm every point of this.
did you test with all the latest patches applied ? In your issues you 
refer to crashes, the crashes reported should be resolved, if you 
still have crashes, pleas provide a core dump or scenario to reproduce 
the crash.

With patch0009 ipa-replica-manage del worked for me

Yep, patch 0009 is applied.
The full list of patches applied on top of the master branch (at it's 
state yesterday at 10 PM) is as follows:

freeipa-lkrispen-0007-replica-install-fails-with-domain-level-1.patch
freeipa-lkrispen-0008-plugin-uses-1-as-minimum-domain-level-to-become-acti.patch
freeipa-lkrispen-0009-crash-when-removing-a-replica.patch
freeipa-mbasti-0262-Installers-fix-remove-temporal-ccache.patch
freeipa-pvoborni-0857-1-topology-ipa-management-commands.patch
freeipa-pvoborni-0858-1-webui-IPA.command_dialog-a-new-dialog-base-class.patch
freeipa-pvoborni-0859-1-webui-use-command_dialog-as-a-base-class-for-passwor.patch
freeipa-pvoborni-0860-1-webui-make-usage-of-all-in-details-facet-optional.patch
freeipa-pvoborni-0861-2-webui-topology-plugin.patch
freeipa-pvoborni-0862-webui-configurable-refresh-command.patch

The scenario is pretty basic:
1. 3 fedora-21 vms with the latest directory server packages from 
mreynolds repo:

389-ds-base-2015_06_02-1.fc21.x86_64

2. setup master on one of them, prepare gpg files for two replicas
3. setup replicas using these gpg files.
4. Try to remove one of the replicas using command `ipa topologysegment-del`
5. Try to create a new user via web UI on any of the replicas





On 06/03/2015 11:37 AM, Martin Babinsky wrote:

Hi everyone,

I have been playing with the topology related patches and I have 
encountered a few issues that I would like to address in this thread:


1.) When replica install for whatever reason crashes _after_ the 
setup of replication agreements etc., it leaves the topology plugin 
with dangling segment pointing to the dysfunctional node. An attempt 
to delete it leads to:



ipa: ERROR: Server is unwilling to perform: Removal of Segment 
disconnects topology.Deletion not allowed.


Furthermore, any attempts to delete a segment (even a properly setup 
one) lead to the same very error.


And you cannot reinstall the crashed replica because it complains 
about existing replication agreements. It would probably help to be 
able to force-remove the segments if one of the endpoints doesn't 
exist/respond.


2.) I was not able to figure out a way remove replica from the 
topology without explosions or tampering 
'cn=masters,cn=ipa,cn=etc,$SUFFIX'. Obviously ipa-replica-manage del 
doesn't work anymore (I have tried just for fun, it leads to SIGSEGV 
of the host's dirsrv and leaves dangling segments to offending 
replica, leading to point 1).


I managed to remove replica from the topology only by directly 
uninstalling FreeIPA on the node and then deleting its' host entry 
from 'cn=masters'. Only after this was the plugin able to 
automagically removed the segments pointing to/from removed node.


The design page suggests that it should be enough to uninstall IPA 
server on the replica. The plugin would then pick-up the dangling 
segments and remove them automatically. However, this behavior seems 
to require additional modification of the uninstall procedure (e.g. 
the uninstalling replica should remove its' entry from cn=masters).


3.) It seems that the removal of topology suffixes containing 
functioning segments is not handled well. I once tried to do this 
and it led to segmentation fault on the dirsrv instance. What is the 
expected behavior in this scenario?








--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 857 topology: ipa management commands

2015-06-03 Thread Petr Vobornik

On 06/03/2015 02:38 PM, Martin Babinsky wrote:

On 06/03/2015 01:34 PM, Petr Vobornik wrote:

On 06/03/2015 10:59 AM, Martin Babinsky wrote:

On 06/03/2015 10:52 AM, Martin Babinsky wrote:

On 05/26/2015 03:31 PM, Petr Vobornik wrote:

On 05/26/2015 12:19 PM, Petr Vobornik wrote:

this patch is based on top of my patch #856 and tbabej'
s 325-9.

Obsoletes Ludwig's 0006.

ipalib part of topology management

Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology

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




New version attached:
- domainlevel_show usage changed to domainlevel_get
- updated VERSION
- added more attrs to default_attributes




Hi Petr,

the commands themselves seem to work just fine. I had encountered some
quirks in the underlying topology plugin, but I will address them in a
different thread in order to keep the discussion relevant to the
reviewed patch.

I have some minor coomments below:

1.)
  IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: pvoborni - added topology management commands

Several people were touching API in the meantime so please double-check
that you have correct VERSION and regenerate API.txt


Patch rebased.



2.)

+Str(
+'nsds5replicatedattributelist?',
+cli_name='replattrs',
+label='Attributes to replicate',
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a fractional update. E.g.,
`(objectclass=*) '
+  '$ EXCLUDE accountlockout memberof'),
+),
+Str(
+'nsds5replicatedattributelisttotal?',
+cli_name='replattrstotal',
+label=_('Attributes for total update'),
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a total update. E.g. (objectclass=*) $
EXCLUDE '
+  'accountlockout'),

The descriptions of these two options confused me greatly, are these
attributes supposed to be replicated or not, or is there some more
complex logic behind them that I failed to grasp? I am cc'ing
Ludwig, he
can probably explain them to us and then we can decide whether we may
alter the descriptions to be less confusing.

3.)

+takes_params = (
+Str(
+'cn',
+cli_name='name',
+primary_key=True,
+label=_('Suffix name'),
+),
+Str(
+'iparepltopoconfroot',
+maxlength=255,
+cli_name='suffix',
+label=_('Suffix to be managed'),
+normalizer=lambda value: value.lower(),
+),
+)

This also confused me at first, I suggest to change the label of
'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or
'LDAP subtree to be managed'.


Changed to 'LDAP suffix to be managed'



4.)

There is currently no way to rename existing topology
segments/suffixes.
In the case of hosts with funky FQDN's (pointing at you, ABC lab), the
segment cn's created during replica installs are mearly impossible to
remember and it would be nice to rename them to something more
manageable. However, this is not related to core functionality and can
be a subject of a separate patch once this gets pushed.

That's all from my side.



I also forgot to ask what is the expected policy when deleting a
non-empty topology suffix. If this is not supported and you have to
first remove all segments and then the suffix itself, the
'topologysuffix-del' command should issue an error pointing the user to
correct procedure.



Do we have a use case for creation or deletion of topology suffix?

That's a good question.

Anyway, I have noticed couple more things:

1.) it seems that there some of unused imports in topology.py. Please
investigate whether all of them are really needed.


Fixed



2.)

+from ipalib.plugins.baseldap import *
+from ipalib.plugins import baseldap

I do not like that starred import at all. Either import the particular
classes you use (like e.g. in basuser.py), or just leave the second
import statetement and use the appropriate namespace
(baseldap.LDAPObject etc.).


Fixed



3.) there are couple of pep8 complaints, please try to fix them unless
it impairs readability:

./ipalib/constants.py:121:80: E501 line too long (81  79 characters)
./ipalib/plugins/topology.py:72:80: E501 line too long (88  79 characters)
./ipalib/plugins/topology.py:73:26: E131 continuation line unaligned for
hanging indent
./ipalib/plugins/topology.py:73:80: E501 line too long (93  79 characters)
./ipalib/plugins/topology.py:103:80: E501 line too long (80  79
characters)
./ipalib/plugins/topology.py:111:80: E501 line too long (80  79
characters)
./ipalib/plugins/topology.py:207:80: E501 line too long (80  79
characters)
./ipalib/plugins/topology.py:232:80: E501 line too long (80  79
characters)


won't fix



Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Alexander Bokovoy

On Wed, 03 Jun 2015, Endi Sukma Dewata wrote:

On 6/3/2015 1:41 AM, Martin Kosek wrote:

On 06/02/2015 11:22 PM, Alexander Bokovoy wrote:

On Tue, 02 Jun 2015, Endi Sukma Dewata wrote:

I think ideally the
client and server code should be in separate files (so they can be deployed
separately too), but the framework doesn't seem to allow that.



This exactly the case we have to use here and we are using that in
trusts case as well -- some code has to run on server only and shouldn't
cause to install Samba related packages on the client. This is because
IPA client is actually using the same IPA plugins that server uses, to
have access to the API calls metadata and client-side callbacks are
defined in the same place where server-side callbacks are. It is IPA
framework design, so we have to use what we have.


This is planned to be changed BTW, when we start with the Thin Client concept
and have different code/plugins for FreeIPA server side and client side.


Is there a ticket for this?


In other words, it is not necessarily an evil under conditions we are
dealing with.


Having to use the same plugins for client and server is a framework 
limitation/poor design. Having to use conditional imports to work 
around the limitation is a bad programming practice. The fact that 
trust plugin has to implement a similar workaround is not a 
justification, it just shows that the problem is not vault-specific.

There is another thing. Even when splitting client/server sides, we'll
need to check on the server side that certain functionality is
available. In trust case we have ID Views (a separate plugin) which does
use information about trusts to resolve users from AD to their
normalized references (SIDs) and few other places would be depending on
functionality only provided when Samba packages are installed.

To continue your approach, we would need to split also server-side parts
of plugins into separate callable units that would only be provided and
called when appropriate rpm subpackages are installed. This is unneeded
complication in place where we can simply handle dependencies in run
time and make sure the packaging deps are managed separately.
--
/ Alexander Bokovoy

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0026-0028] Fix nits in user-visible output

2015-06-03 Thread Martin Basti

On 14/04/15 09:43, Petr Spacek wrote:

On 14.4.2015 09:10, Martin Kosek wrote:

On 04/13/2015 05:05 PM, Petr Spacek wrote:

Hello,

documentation team proposed few changes in user-visible messages so here it
is. It was not worth a ticket and related overhead.

The changes look OK to me. I would just have one (prudish) request to not add
nazi reference to our git history - whether they are grammar or not. Please
keep the git technical :-)

Sure, here is the same patch with modified commit message.




0026 ACK
0027-2 ACK
0028 ACK

--
Martin Basti

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Endi Sukma Dewata

On 6/3/2015 8:52 AM, Alexander Bokovoy wrote:

Having to use the same plugins for client and server is a framework
limitation/poor design. Having to use conditional imports to work
around the limitation is a bad programming practice. The fact that
trust plugin has to implement a similar workaround is not a
justification, it just shows that the problem is not vault-specific.



There is another thing. Even when splitting client/server sides, we'll
need to check on the server side that certain functionality is
available. In trust case we have ID Views (a separate plugin) which does
use information about trusts to resolve users from AD to their
normalized references (SIDs) and few other places would be depending on
functionality only provided when Samba packages are installed.

To continue your approach, we would need to split also server-side parts
of plugins into separate callable units that would only be provided and
called when appropriate rpm subpackages are installed. This is unneeded
complication in place where we can simply handle dependencies in run
time and make sure the packaging deps are managed separately.


So there are two issues:
1. Conditional imports due to combined client and server plugin.
2. Conditional imports due to feature availability.

Issue #1 is generic and I think we pretty much agree that this is 
supposed to be fixed somehow.


Issue #2 is plugin-specific. I think there are different ways to solve 
this, some might be better than others. The solution that you pick will 
only affect that particular plugin and should not be generalized to 
other plugins or to justify #1.


In my opinion a code should have a fixed dependency, but some features 
can be enabled/disabled based on the configuration. Enabling a feature 
shouldn't be based on package installation because people might install 
a package for different reasons. So the code may look something like this:


  import module

  if feature is enabled:
  do something with the module

It shouldn't be like this:

  if package is installed:
  import module

  if package is installed:
  do something with the module

Of course this assumes that the package is lightweight enough to be 
installed regardless whether it will be used. I don't know if it's 
applicable to your case, but again, there are different ways to address 
issue #2.


--
Endi S. Dewata

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0010] KeyError raised upon replica installation

2015-06-03 Thread Martin Kosek
On 06/03/2015 04:10 PM, Petr Vobornik wrote:
 On 06/02/2015 02:20 PM, Ludwig Krispenz wrote:
 replicas installed from older versions do not have a binddn group
 just accept the errror
 
 ACK
 
 Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76
 
 Note that this group will be populated later. IMHO it should be done as a part
 of domain-level raise procedure before setting the new level.

As said in other mail, I am not sure why we should be overloading domain-level
raise command that way.

I thought, we will create this group when the first replica upgrades to 4.2.
Whenever a new replica is added/upgraded, it's principal will be added to the
group also (even if Domain Level is 0).

Domain Level 1 means that all replicas are 4.2 and thus the group is fully
populated and Topology can be used.

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0010] KeyError raised upon replica installation

2015-06-03 Thread Petr Vobornik

On 06/02/2015 02:20 PM, Ludwig Krispenz wrote:

replicas installed from older versions do not have a binddn group
just accept the errror


ACK

Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76

Note that this group will be populated later. IMHO it should be done as 
a part of domain-level raise procedure before setting the new level.

--
Petr Vobornik

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0010] KeyError raised upon replica installation

2015-06-03 Thread Ludwig Krispenz


On 06/03/2015 04:10 PM, Petr Vobornik wrote:

On 06/02/2015 02:20 PM, Ludwig Krispenz wrote:

replicas installed from older versions do not have a binddn group
just accept the errror


ACK

Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76

Note that this group will be populated later.
if you start with 4.2 the group is created and populated when the ldap 
principals are added to the replica as binddns. Only if you install the 
replica from an older version the database is initialized from the older 
master and it is gone. so it has to be populated later.
IMHO it should be done as a part of domain-level raise procedure 
before setting the new level.
It could also be populated as soon as the first 4.2 replica is 
installed, it doesn't require any schema changes and can be 
added/replicated to older serevrs as well


--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Use Exception class instead of BaseException

2015-06-03 Thread Petr Viktorin
On 06/01/2015 06:33 AM, Niranjan wrote:
 Greetings,
 
 I would like to present patch for replacing StandardError exception
 with Exception class in ipapython/adminutil.py. Also replacing
 BaseException class with Exception class. 
 
 Though the use of StandardError is many places. I would like to start
 with ipapython/adminutil.py
 
 This is my first patch. Please let me know if my approach on this is 
 correct.
 
 Regards
 Niranjan
 
 
 0001-Use-Exception-class-instead-of-BaseException.patch
 
 
 From 018312f76952ea86c8c6e2396657e0531d2d61ba Mon Sep 17 00:00:00 2001
 From: Niranjan Mallapadi mrniran...@redhat.com
 Date: Mon, 1 Jun 2015 09:41:05 +0530
 Subject: [PATCH] Use Exception class instead of BaseException
 
 1. Replace BaseException with Exception class.

I don't see a reason for this change. This is top-level CLI code that
handles calling our Python library. We really do want to catch all
exceptions here, including KeyboardInterrupt and SystemExit.

 2. Remove StandardError and use Exception class. StandError is deprecated 
 (Python3)

I'm okay with this change, as long as tests still pass.

 3 .From python3.0 use of , is not recommended, instead
 use as keyword (PEP 3110)

+1


-- 
Petr Viktorin

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Use Exception class instead of BaseException

2015-06-03 Thread Martin Basti

On 01/06/15 06:33, Niranjan wrote:

Greetings,

I would like to present patch for replacing StandardError exception
with Exception class in ipapython/adminutil.py. Also replacing
BaseException class with Exception class.

Though the use of StandardError is many places. I would like to start
with ipapython/adminutil.py

This is my first patch. Please let me know if my approach on this is
correct.

Regards
Niranjan



Thank you,

I have another objection:

1)
Please do not copy/paste code, use this for except

except (Exception, SystemExit) as exception:


Martin Basti
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] KeyError raised upon replica installation

2015-06-03 Thread Oleg Fayans

BTW, Ludwig, it seems you forgot to attach the 0010 patch to your email.
At least, your first letter from 06/02/2015 05:08 PM, containing PATCH 
0010 does not have the actual patch


On 06/03/2015 02:53 PM, Oleg Fayans wrote:

Hi Ludwig,

I'll rebuild the packages again with the whole set of patches 
including 0010 and 0011 and try again. Thanks!


On 06/03/2015 02:21 PM, Ludwig Krispenz wrote:


On 06/03/2015 02:05 PM, Oleg Fayans wrote:

Update:

The original error occurs ONLY when installing a replica from a gpg 
file prepared on a master running FreeIPA 4.1.2.

but this should be covere with patch 0010

If The master runs the upstream code, it works.

On 06/02/2015 02:11 PM, Martin Babinsky wrote:

On 06/02/2015 02:07 PM, Martin Babinsky wrote:

On 06/02/2015 12:09 PM, Oleg Fayans wrote:

Hi all,

The following error was caught during replica installation (I 
used all

the latest patches from Ludwig and Martin Basti):

root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca
--setup-dns --forwarder 10.38.5.26
/var/lib/ipa/replica-info-replica1.zaeba.li.gpg
Directory Manager (existing master) password:

Existing BIND configuration detected, overwrite? [no]: yes
Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file
Checking forwarders, please wait ...
Using reverse zone(s) 122.168.192.in-addr.arpa.
Run connection check to master
Check connection from replica to remote master 
'upgrademaster.zaeba.li':

Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos Kpasswd: TCP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

The following list of ports use UDP protocol and would need to be
checked manually:
Kerberos KDC: UDP (88): SKIPPED
Kerberos Kpasswd: UDP (464): SKIPPED

Connection from replica to master is OK.
Start listening on required ports for remote master check
Get credentials to log in to remote master
ad...@zaeba.li password:

Check SSH connection to remote master
Execute check on remote master
Check connection from master to remote replica 'replica1.zaeba.li':
Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos KDC: UDP (88): OK
Kerberos Kpasswd: TCP (464): OK
Kerberos Kpasswd: UDP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

Connection from master to replica is OK.

Connection check OK
Configuring NTP daemon (ntpd)
   [1/4]: stopping ntpd
   [2/4]: writing configuration
   [3/4]: configuring ntpd to start on boot
   [4/4]: starting ntpd
Done configuring NTP daemon (ntpd).
Configuring directory server (dirsrv): Estimated time 1 minute
   [1/37]: creating directory server user
   [2/37]: creating directory server instance
   [3/37]: adding default schema
   [4/37]: enabling memberof plugin
   [5/37]: enabling winsync plugin
   [6/37]: configuring replication version plugin
   [7/37]: enabling IPA enrollment plugin
   [8/37]: enabling ldapi
   [9/37]: configuring uniqueness plugin
   [10/37]: configuring uuid plugin
   [11/37]: configuring modrdn plugin
   [12/37]: configuring DNS plugin
   [13/37]: enabling entryUSN plugin
   [14/37]: configuring lockout plugin
   [15/37]: configuring topology plugin
   [16/37]: creating indices
   [17/37]: enabling referential integrity plugin
   [18/37]: configuring ssl for ds instance
   [19/37]: configuring certmap.conf
   [20/37]: configure autobind for root
   [21/37]: configure new location for managed entries
   [22/37]: configure dirsrv ccache
   [23/37]: enable SASL mapping fallback
   [24/37]: restarting directory server
   [25/37]: setting up initial replication
Starting replication, please wait until this has completed.
Update in progress, 7 seconds elapsed
Update succeeded

   [26/37]: updating schema
   [27/37]: setting Auto Member configuration
   [28/37]: enabling S4U2Proxy delegation
   [29/37]: importing CA certificates from LDAP
   [30/37]: initializing group membership
   [31/37]: adding master entry
ipa : CRITICAL Failed to load master-entry.ldif: Command
''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H'
'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y'
'/tmp/tmpk_R0Lm'' returned non-zero exit status 68
   [32/37]: initializing domain level
   [33/37]: configuring Posix uid/gid generation
   [34/37]: adding replication acis
   [35/37]: enabling compatibility plugin
   [36/37]: tuning directory server
   [37/37]: configuring directory to start on boot
Done configuring directory server (dirsrv).
Configuring certificate server (pki-tomcatd): Estimated time 3 
minutes

30 seconds
   [1/21]: creating certificate server user
   [2/21]: configuring certificate server instance
   [3/21]: stopping certificate server instance to update CS.cfg
   [4/21]: backing up CS.cfg
   [5/21]: disabling nonces
   [6/21]: set up CRL publishing
   

Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Endi Sukma Dewata

On 6/3/2015 1:41 AM, Martin Kosek wrote:

On 06/02/2015 11:22 PM, Alexander Bokovoy wrote:

On Tue, 02 Jun 2015, Endi Sukma Dewata wrote:

I think ideally the
client and server code should be in separate files (so they can be deployed
separately too), but the framework doesn't seem to allow that.



This exactly the case we have to use here and we are using that in
trusts case as well -- some code has to run on server only and shouldn't
cause to install Samba related packages on the client. This is because
IPA client is actually using the same IPA plugins that server uses, to
have access to the API calls metadata and client-side callbacks are
defined in the same place where server-side callbacks are. It is IPA
framework design, so we have to use what we have.


This is planned to be changed BTW, when we start with the Thin Client concept
and have different code/plugins for FreeIPA server side and client side.


Is there a ticket for this?


In other words, it is not necessarily an evil under conditions we are
dealing with.


Having to use the same plugins for client and server is a framework 
limitation/poor design. Having to use conditional imports to work around 
the limitation is a bad programming practice. The fact that trust plugin 
has to implement a similar workaround is not a justification, it just 
shows that the problem is not vault-specific.


--
Endi S. Dewata

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] KeyError raised upon replica installation

2015-06-03 Thread Ludwig Krispenz


On 06/03/2015 02:05 PM, Oleg Fayans wrote:

Update:

The original error occurs ONLY when installing a replica from a gpg 
file prepared on a master running FreeIPA 4.1.2.

but this should be covere with patch 0010

If The master runs the upstream code, it works.

On 06/02/2015 02:11 PM, Martin Babinsky wrote:

On 06/02/2015 02:07 PM, Martin Babinsky wrote:

On 06/02/2015 12:09 PM, Oleg Fayans wrote:

Hi all,

The following error was caught during replica installation (I used all
the latest patches from Ludwig and Martin Basti):

root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca
--setup-dns --forwarder 10.38.5.26
/var/lib/ipa/replica-info-replica1.zaeba.li.gpg
Directory Manager (existing master) password:

Existing BIND configuration detected, overwrite? [no]: yes
Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file
Checking forwarders, please wait ...
Using reverse zone(s) 122.168.192.in-addr.arpa.
Run connection check to master
Check connection from replica to remote master 
'upgrademaster.zaeba.li':

Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos Kpasswd: TCP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

The following list of ports use UDP protocol and would need to be
checked manually:
Kerberos KDC: UDP (88): SKIPPED
Kerberos Kpasswd: UDP (464): SKIPPED

Connection from replica to master is OK.
Start listening on required ports for remote master check
Get credentials to log in to remote master
ad...@zaeba.li password:

Check SSH connection to remote master
Execute check on remote master
Check connection from master to remote replica 'replica1.zaeba.li':
Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos KDC: UDP (88): OK
Kerberos Kpasswd: TCP (464): OK
Kerberos Kpasswd: UDP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

Connection from master to replica is OK.

Connection check OK
Configuring NTP daemon (ntpd)
   [1/4]: stopping ntpd
   [2/4]: writing configuration
   [3/4]: configuring ntpd to start on boot
   [4/4]: starting ntpd
Done configuring NTP daemon (ntpd).
Configuring directory server (dirsrv): Estimated time 1 minute
   [1/37]: creating directory server user
   [2/37]: creating directory server instance
   [3/37]: adding default schema
   [4/37]: enabling memberof plugin
   [5/37]: enabling winsync plugin
   [6/37]: configuring replication version plugin
   [7/37]: enabling IPA enrollment plugin
   [8/37]: enabling ldapi
   [9/37]: configuring uniqueness plugin
   [10/37]: configuring uuid plugin
   [11/37]: configuring modrdn plugin
   [12/37]: configuring DNS plugin
   [13/37]: enabling entryUSN plugin
   [14/37]: configuring lockout plugin
   [15/37]: configuring topology plugin
   [16/37]: creating indices
   [17/37]: enabling referential integrity plugin
   [18/37]: configuring ssl for ds instance
   [19/37]: configuring certmap.conf
   [20/37]: configure autobind for root
   [21/37]: configure new location for managed entries
   [22/37]: configure dirsrv ccache
   [23/37]: enable SASL mapping fallback
   [24/37]: restarting directory server
   [25/37]: setting up initial replication
Starting replication, please wait until this has completed.
Update in progress, 7 seconds elapsed
Update succeeded

   [26/37]: updating schema
   [27/37]: setting Auto Member configuration
   [28/37]: enabling S4U2Proxy delegation
   [29/37]: importing CA certificates from LDAP
   [30/37]: initializing group membership
   [31/37]: adding master entry
ipa : CRITICAL Failed to load master-entry.ldif: Command
''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H'
'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y'
'/tmp/tmpk_R0Lm'' returned non-zero exit status 68
   [32/37]: initializing domain level
   [33/37]: configuring Posix uid/gid generation
   [34/37]: adding replication acis
   [35/37]: enabling compatibility plugin
   [36/37]: tuning directory server
   [37/37]: configuring directory to start on boot
Done configuring directory server (dirsrv).
Configuring certificate server (pki-tomcatd): Estimated time 3 minutes
30 seconds
   [1/21]: creating certificate server user
   [2/21]: configuring certificate server instance
   [3/21]: stopping certificate server instance to update CS.cfg
   [4/21]: backing up CS.cfg
   [5/21]: disabling nonces
   [6/21]: set up CRL publishing
   [7/21]: enable PKIX certificate path discovery and validation
   [8/21]: starting certificate server instance
   [9/21]: creating RA agent certificate database
   [10/21]: importing CA chain to RA certificate database
   [11/21]: fixing RA database permissions
   [12/21]: setting up signing cert profile
   [13/21]: set certificate subject base
   [14/21]: enabling Subject Key Identifier
   [15/21]: 

[Freeipa-devel] [PATCH 0011] check-for-existing-and-self-referential-segments

2015-06-03 Thread Ludwig Krispenz

Hi,

this should prevent adding duplicate segments or segments with same 
start and end node
From 759790e3c6c87ebe75610fdcfda06c6d4bc00475 Mon Sep 17 00:00:00 2001
From: Ludwig Krispenz lkris...@redhat.com
Date: Wed, 3 Jun 2015 14:22:52 +0200
Subject: [PATCH] check for existing and self referential segments

---
 daemons/ipa-slapi-plugins/topology/topology_pre.c | 30 +++
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/daemons/ipa-slapi-plugins/topology/topology_pre.c b/daemons/ipa-slapi-plugins/topology/topology_pre.c
index 528f72b69dfe0e57c5312e558c6e0ac5f58801fb..0a0cd65b592e2dc796a179e035598e5f641bb01e 100644
--- a/daemons/ipa-slapi-plugins/topology/topology_pre.c
+++ b/daemons/ipa-slapi-plugins/topology/topology_pre.c
@@ -279,21 +279,31 @@ ipa_topo_check_connect_reject(Slapi_PBlock *pb)
 if (pi  0 == strcasecmp(pi, ipa_topo_get_plugin_id())) {
 return 0;
 }
-slapi_pblock_get(pb,SLAPI_DELETE_EXISTING_ENTRY,add_entry);
+slapi_pblock_get(pb,SLAPI_ADD_ENTRY,add_entry);
 if (TOPO_SEGMENT_ENTRY != ipa_topo_check_entry_type(add_entry)) {
 return 0;
 } else {
 /* a new segment is added
  * verify that the segment does not yet exist
  */
-TopoReplicaSegment *tsegm;
-TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(add_entry);
-tsegm = ipa_topo_util_find_segment(tconf, add_entry);
-if (tsegm) {
-slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
-segment to be added does already exist\n);
-rc = 1;
+char *leftnode = slapi_entry_attr_get_charptr(add_entry,ipaReplTopoSegmentLeftNode);
+char *rightnode = slapi_entry_attr_get_charptr(add_entry,ipaReplTopoSegmentRightNode);
+if (0 == strcasecmp(leftnode,rightnode)) {
+slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
+segment is self referential\n);
+rc = 1;
+} else {
+TopoReplicaSegment *tsegm;
+TopoReplica *tconf = ipa_topo_util_get_conf_for_segment(add_entry);
+tsegm = ipa_topo_util_find_segment(tconf, add_entry);
+if (tsegm) {
+slapi_log_error(SLAPI_LOG_FATAL, IPA_TOPO_PLUGIN_SUBSYSTEM,
+segment to be added does already exist\n);
+rc = 1;
+}
 }
+slapi_ch_free_string(leftnode);
+slapi_ch_free_string(rightnode);
 }
 return rc;
 }
@@ -378,8 +388,8 @@ int ipa_topo_pre_add(Slapi_PBlock *pb)
 } else if (ipa_topo_check_connect_reject(pb)) {
 int rc = LDAP_UNWILLING_TO_PERFORM;
 char *errtxt;
-errtxt = slapi_ch_smprintf(Segment already exists in topology.
-   Add rejected.\n);
+errtxt = slapi_ch_smprintf(Segment already exists in topology or
+is self referential. Add rejected.\n);
 slapi_pblock_set(pb, SLAPI_PB_RESULT_TEXT, errtxt);
 slapi_pblock_set(pb, SLAPI_RESULT_CODE, rc);
 result = SLAPI_PLUGIN_FAILURE;
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Endi Sukma Dewata

On 6/2/2015 1:34 PM, Simo Sorce wrote:

On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote:

Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a):

On 5/28/2015 12:46 AM, Jan Cholasta wrote:

On a related note, since KRA is optional, can we move the vaults
container to cn=kra,cn=vaults? This is the convetion used by the other
optional components (DNS and recently CA).


I mean cn=vaults,cn=kra of course.


If you are talking about the o=kra,PKI suffix, I'm not sure whether
the IPA framework will work with it.

If you are talking about adding a new cn=kra,IPA suffix entry on top
of cn=vaults, what is the purpose of this entry? Is the entry going to
be created/deleted automatically when the KRA is installed/removed? Is
it going to be used for something else other than vaults?


I'm talking about cn=kra,IPA suffix. It should be created only when
KRA is installed, although I think this can be done later after the
release, moving vaults to cn=kra should be good enough for now. It's
going to be used for everything KRA-specific.



There are a lot of questions that need to be answered before we can make
this change.


This is about sticking to a convention, which everyone should do, and
everyone except KRA already does.

I'm sorry I didn't realize this earlier, but the change must be done now.


We probably should revisit this issue after the core vault
functionality is added.



We can't revisit it later because after release we are stuck with
whatever is there forever.

See attachment for a patch which implements the change.



Shouldn't we s/kra/vault/ ?
After all the feature is called Vault, not KRA.

Simo.



Here are the options:
1. the original code uses cn=vaults,IPA suffix.
2. Honza proposed cn=vaults,cn=kra,IPA suffix, ACKed by Martin.

Are you proposing a third option cn=vaults,cn=vault,IPA suffix or 
did you mean the first option?


I think the first option would make more sense since we're not 
introducing KRA to the end user, but I'll let the IPA team decide. My 
next patch will be based on whatever pushed at the time.


--
Endi S. Dewata

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation

2015-06-03 Thread Petr Spacek
On 18.5.2015 13:48, Martin Basti wrote:
 On 15/05/15 18:11, Petr Spacek wrote:
 On 7.5.2015 18:12, Martin Basti wrote:
 On 07/05/15 12:19, Petr Spacek wrote:
 On 7.5.2015 08:59, David Kupka wrote:
 On 05/06/2015 03:20 PM, Martin Basti wrote:
 On 05/05/15 15:00, Martin Basti wrote:
 On 30/04/15 15:37, David Kupka wrote:
 On 04/24/2015 02:56 PM, Martin Basti wrote:
 Patches attached.




 Hi,
 thanks for patches.

 1. You changed message in DNSServerNotRespondingWarning class but not
 the test in ipatest/test_xmlrpc/test_dns_plugin.py

 nitpick. Please spell 'edns' correctly. I've seen several instances
 of 'ends'.

 Thank you,

 updated patches attached:
 * new error messages
 * logging to debug log server output if exception was raised
 * fixed test
 * fixed spelling



 Fixed tests (again)

 Updated patches attached

 The code looks good to me and tests are no longer broken. (I would prefer
 better fix of the tests but given that the priorities are different now
 it can
 wait.)

 Petr, can you please confirm that the patch set works for you?
 Sorry, NACK:

 $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
 Server will check DNS forwarder(s).
 This may take some time, please wait ...
 ipa: ERROR: an internal error has occurred

 # /var/log/httpd/error_log
 ipa: ERROR: non-public: AssertionError:
 Traceback (most recent call last):
 File /usr/lib/python2.7/site-packages/ipaserver/rpcserver.py, line
 350, in
 wsgi_execute
   result = self.Command[name](*args, **options)
 File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 443, 
 in
 __call__
   ret = self.run(*args, **options)
 File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 760,
 in run
   return self.execute(*args, **options)
 File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line
 , in
 execute
   **options)
 File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line
 4405, in
 _warning_if_forwarders_do_not_work
   log=self.log)
 File /usr/lib/python2.7/site-packages/ipalib/util.py, line 715, in
 validate_dnssec_zone_forwarder_step2
   timeout=timeout)
 File /usr/lib/python2.7/site-packages/ipalib/util.py, line 610, in
 _resolve_record
   assert isinstance(nameserver_ip, basestring)
 AssertionError
 ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(DNS
 name ptr.test., idnsforwarders=(u'10.34.47.236',), all=False, raw=False,
 version=u'2.116'): AssertionError

 This is constantly reproducible in my vm-090.abc. Let me know if you want 
 to
 take a look.


 I'm attaching little response.patch which improves compatibility with older
 python-dns packages. This patch allows IPA to work while error messages are
 simply not as nice as they could be with latest python-dns :-)

 check_fwd_msg.patch is a little nitpick, just to make sure everyone
 understands the message.

 BTW why some messages in check_forwarders() are printed using 'print' and
 others using logger? I would prefer to use logger for everything to make 
 sure
 that logs contain all the information, including warnings.

 Thank you for your time!

 Thank you, fixed.

 I  added missing except block after forwarders validation step2.
 I confirm that this works but I just discovered another deficiency.

 Setup:
 - DNSSEC validation is enabled on IPA server
 - forwarders uses fake TLD, e.g. 'test.'
 - remote DNS server is responding, supports EDNS0 and so on

 $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
 Server will check DNS forwarder(s).
 This may take some time, please wait ...
 ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The DNS query
 name does not exist: ptr.test..

 Huh? Let's check named log:
   forward zone 'ptr.test': loaded
   validating ./SOA: got insecure response; parent indicates it should be 
 secure

 Sometimes I get SERVFAIL from IPA server, too.


 Unfortunately this check was the main reason for writing this patchset so we
 need to improve it.

 Maybe validate_dnssec_zone_forwarder_step2() could special-case NXDOMAIN and
 print the DNSSEC-validation-failed error, too? The problem is that it could
 trigger some false positives because NXDOMAIN may simply be caused by a delay
 somewhere.

 Any ideas?
 I add catch block for NXDOMAIN

 By the way, this is also weird:

 $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
 Server will check DNS forwarder(s).
 This may take some time, please wait ...
 ipa: ERROR: DNS forward zone with name ptr.test. already exists

 Is it actually doing the check even if the forward zone exists already? (This
 is just nitpick, not a blocker!)

 The first part is written by IPA client, it is not response from server.
 It is just written when user use --forwarder option.
 
 Updated patch attached.

NACK, it does not work for me - it explodes when I try to add a forward zone:

$ ipa dnsforwardzone-add ptr.test. --forwarder=192.0.2.1

ipa: ERROR: non-public: TypeError: 

Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Jan Cholasta

Dne 3.6.2015 v 14:58 Endi Sukma Dewata napsal(a):

On 6/2/2015 1:34 PM, Simo Sorce wrote:

On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote:

Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a):

On 5/28/2015 12:46 AM, Jan Cholasta wrote:

On a related note, since KRA is optional, can we move the vaults
container to cn=kra,cn=vaults? This is the convetion used by the
other
optional components (DNS and recently CA).


I mean cn=vaults,cn=kra of course.


If you are talking about the o=kra,PKI suffix, I'm not sure whether
the IPA framework will work with it.

If you are talking about adding a new cn=kra,IPA suffix entry on top
of cn=vaults, what is the purpose of this entry? Is the entry going to
be created/deleted automatically when the KRA is installed/removed? Is
it going to be used for something else other than vaults?


I'm talking about cn=kra,IPA suffix. It should be created only when
KRA is installed, although I think this can be done later after the
release, moving vaults to cn=kra should be good enough for now. It's
going to be used for everything KRA-specific.



There are a lot of questions that need to be answered before we can
make
this change.


This is about sticking to a convention, which everyone should do, and
everyone except KRA already does.

I'm sorry I didn't realize this earlier, but the change must be done
now.


We probably should revisit this issue after the core vault
functionality is added.



We can't revisit it later because after release we are stuck with
whatever is there forever.

See attachment for a patch which implements the change.



Shouldn't we s/kra/vault/ ?
After all the feature is called Vault, not KRA.

Simo.



Here are the options:
1. the original code uses cn=vaults,IPA suffix.
2. Honza proposed cn=vaults,cn=kra,IPA suffix, ACKed by Martin.

Are you proposing a third option cn=vaults,cn=vault,IPA suffix or
did you mean the first option?

I think the first option would make more sense since we're not
introducing KRA to the end user, but I'll let the IPA team decide. My
next patch will be based on whatever pushed at the time.


The DNs are not exposed to the end user, they are only relevant for our 
internal organization of entries.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Jan Cholasta

Dne 2.6.2015 v 02:00 Endi Sukma Dewata napsal(a):

Please take a look at the updated patch.

On 5/27/2015 12:39 AM, Jan Cholasta wrote:

21) vault_archive is not a retrieve operation, it should be
based on
LDAPUpdate instead of LDAPRetrieve. Or Command actually, since it
does
not do anything with LDAP. The same applies to vault_retrieve.


The vault_archive does not actually modify the LDAP entry because it
stores the data in KRA. It is actually an LDAPRetrieve operation
because
it needs to get the vault info before it can perform the archival
operation. Same thing with vault_retrieve.


It is not a LDAPRetrieve operation, because it has different
semantics.
Please use Command as base class and either use ldap2 for direct
LDAP or
call vault_show instead of hacking around LDAPRetrieve.


It's been changed to inherit from LDAPQuery instead.


NACK, it's not a LDAPQuery operation, because it has different
semantics. There is more to a command than executing code, so you
should
use a correct base class.


Changed to inherit from Command as requested. Now these commands no
longer have a direct access to the vault object (self.obj) although they
are accessing vault objects like other vault commands. Also now the
vault name argument has to be added explicitly on each command.


You can inherit from crud.Retrieve and crud.Update to get self.obj and
the argument back.


I tried this:

   class vault_retrieve(Command, crud.Retrieve):

and it gave me an error:

   TypeError: Error when calling the metaclass bases
   Cannot create a consistent method resolution
   order (MRO) for bases Retrieve, Command

I'm sticking with the original code since it works fine although not
ideal. I'm not a Python expert, so if you know how to fix this properly
please feel free to post a patch on top of this.


The class hierarchy is as follows:

   frontend.Command
   frontend.Method
   crud.PKQuery
   crud.Retrieve
   cdur.Update

So removing Command from the list of base classes should fix it.




If KRA is not installed, vault-archive and vault-retrieve fail with
internal error.


Added a code to check KRA installation in all vault commands. If you
know a way not to load the vault plugin if the KRA is not installed
please let me know, that's probably even better. Not sure how that will
work on the client side though.


I see this has been already resolved in the other thread.




The commands still behave differently based on whether they were called
from API which was initialized with in_server set to True or False.


That is unfortunately a restriction imposed by the framework. In order
to guarantee the security, the vault is designed to have separate client
and server code. The client code encrypts the secret, the server code
forwards the encrypted secret to KRA. To archive a secret into a vault
properly, you are supposed to call the client code. If you're calling
the server code directly, you are responsible to do your own encryption
(i.e. generating session key, nonce, and vault data).


I understand why the code has to be separated, what I don't understand 
is why it is in fact *not* separated and crammed into a single command, 
making weird and undefined behavior possible.




If another plugin wants to use vault, it should implement a client code
which calls the vault client code to perform the archival from the
client side.

What is the use case for calling the vault API from the server side
anyway? Wouldn't that defeat the purpose of having a vault? If a secret
exists on the server side in an unencrypted form doesn't it mean the
secret may already have been compromised?


Server API is used not only by the server itself, but also by installers 
for example. Anyway the point is that there *can't* be a broken API like 
this, you should at least raise an error if the command is called from 
server API, although actually separating it into client and server parts 
would be preferable.





There is no point in exposing the session_key, nonce and vault_data
options in CLI when their value is always overwritten in forward().


I agree there is no need to expose them in CLI, but in this framework
the API also defines the CLI. If there's a way to keep them in the
server API but not expose them in the CLI please let me know. Or, if
there's a way to define completely separate server API (without a
matching client CLI) and client CLI (without a matching server API) that
will work too.


As I suggested above, you can split the commands into separate client 
and server commands. The client command should inherit from 
frontend.Local so that it is always executed locally and the server 
command should have a NO_CLI = True attribute so that it is not 
available in the CLI.





Will this always succeed?

+# deactivate vault record in KRA
+response = kra_client.keys.list_keys(
+client_key_id, pki.key.KeyClient.KEY_STATUS_ACTIVE)


Yes. If there's no active keys it will 

Re: [Freeipa-devel] KeyError raised upon replica installation

2015-06-03 Thread Oleg Fayans

Update:

The original error occurs ONLY when installing a replica from a gpg file 
prepared on a master running FreeIPA 4.1.2.

If The master runs the upstream code, it works.

On 06/02/2015 02:11 PM, Martin Babinsky wrote:

On 06/02/2015 02:07 PM, Martin Babinsky wrote:

On 06/02/2015 12:09 PM, Oleg Fayans wrote:

Hi all,

The following error was caught during replica installation (I used all
the latest patches from Ludwig and Martin Basti):

root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca
--setup-dns --forwarder 10.38.5.26
/var/lib/ipa/replica-info-replica1.zaeba.li.gpg
Directory Manager (existing master) password:

Existing BIND configuration detected, overwrite? [no]: yes
Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file
Checking forwarders, please wait ...
Using reverse zone(s) 122.168.192.in-addr.arpa.
Run connection check to master
Check connection from replica to remote master 
'upgrademaster.zaeba.li':

Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos Kpasswd: TCP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

The following list of ports use UDP protocol and would need to be
checked manually:
Kerberos KDC: UDP (88): SKIPPED
Kerberos Kpasswd: UDP (464): SKIPPED

Connection from replica to master is OK.
Start listening on required ports for remote master check
Get credentials to log in to remote master
ad...@zaeba.li password:

Check SSH connection to remote master
Execute check on remote master
Check connection from master to remote replica 'replica1.zaeba.li':
Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos KDC: UDP (88): OK
Kerberos Kpasswd: TCP (464): OK
Kerberos Kpasswd: UDP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

Connection from master to replica is OK.

Connection check OK
Configuring NTP daemon (ntpd)
   [1/4]: stopping ntpd
   [2/4]: writing configuration
   [3/4]: configuring ntpd to start on boot
   [4/4]: starting ntpd
Done configuring NTP daemon (ntpd).
Configuring directory server (dirsrv): Estimated time 1 minute
   [1/37]: creating directory server user
   [2/37]: creating directory server instance
   [3/37]: adding default schema
   [4/37]: enabling memberof plugin
   [5/37]: enabling winsync plugin
   [6/37]: configuring replication version plugin
   [7/37]: enabling IPA enrollment plugin
   [8/37]: enabling ldapi
   [9/37]: configuring uniqueness plugin
   [10/37]: configuring uuid plugin
   [11/37]: configuring modrdn plugin
   [12/37]: configuring DNS plugin
   [13/37]: enabling entryUSN plugin
   [14/37]: configuring lockout plugin
   [15/37]: configuring topology plugin
   [16/37]: creating indices
   [17/37]: enabling referential integrity plugin
   [18/37]: configuring ssl for ds instance
   [19/37]: configuring certmap.conf
   [20/37]: configure autobind for root
   [21/37]: configure new location for managed entries
   [22/37]: configure dirsrv ccache
   [23/37]: enable SASL mapping fallback
   [24/37]: restarting directory server
   [25/37]: setting up initial replication
Starting replication, please wait until this has completed.
Update in progress, 7 seconds elapsed
Update succeeded

   [26/37]: updating schema
   [27/37]: setting Auto Member configuration
   [28/37]: enabling S4U2Proxy delegation
   [29/37]: importing CA certificates from LDAP
   [30/37]: initializing group membership
   [31/37]: adding master entry
ipa : CRITICAL Failed to load master-entry.ldif: Command
''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H'
'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y'
'/tmp/tmpk_R0Lm'' returned non-zero exit status 68
   [32/37]: initializing domain level
   [33/37]: configuring Posix uid/gid generation
   [34/37]: adding replication acis
   [35/37]: enabling compatibility plugin
   [36/37]: tuning directory server
   [37/37]: configuring directory to start on boot
Done configuring directory server (dirsrv).
Configuring certificate server (pki-tomcatd): Estimated time 3 minutes
30 seconds
   [1/21]: creating certificate server user
   [2/21]: configuring certificate server instance
   [3/21]: stopping certificate server instance to update CS.cfg
   [4/21]: backing up CS.cfg
   [5/21]: disabling nonces
   [6/21]: set up CRL publishing
   [7/21]: enable PKIX certificate path discovery and validation
   [8/21]: starting certificate server instance
   [9/21]: creating RA agent certificate database
   [10/21]: importing CA chain to RA certificate database
   [11/21]: fixing RA database permissions
   [12/21]: setting up signing cert profile
   [13/21]: set certificate subject base
   [14/21]: enabling Subject Key Identifier
   [15/21]: enabling Subject Alternative Name
   [16/21]: enabling CRL and OCSP extensions for 

Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates

2015-06-03 Thread Fraser Tweedale
On Wed, Jun 03, 2015 at 01:55:47PM +0200, Milan Kubik wrote:
 On 06/03/2015 01:17 PM, Martin Basti wrote:
 On 02/06/15 16:03, Jan Cholasta wrote:
 Dne 2.6.2015 v 12:36 Martin Basti napsal(a):
 On 02/06/15 11:42, Fraser Tweedale wrote:
 On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote:
 On 01/06/15 06:40, Fraser Tweedale wrote:
 New version of patch; ``{host,service}-show --out=FILE`` now writes
 all certs to FILE.  Rebased on latest master.
 
 Thanks,
 Fraser
 
 On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote:
 Updated patch attached.  Notably restores/adds revocation behaviour
 to host-mod and service-mod.
 
 Thanks,
 Fraser
 
 On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:
 On 27/05/15 15:53, Fraser Tweedale wrote:
 This patch adds supports for multiple user / host
 certificates.  No
 schema change is needed ('usercertificate' attribute is already
 multi-value).  The revoke-previous-cert behaviour of host-mod and
 user-mod has been removed but revocation behaviour of -del and
 -disable is preserved.
 
 The latest profiles/caacl patchset (0001..0013 v5) depends
 on this
 patch for correct cert-request behaviour.
 
 There is one design question (or maybe more, let me know): the
 `--out=FILENAME' option to {host,service} show saves ONE
 certificate
 to the named file.  I propose to either:
 
 a) write all certs, suffixing suggested filename with either a
 sequential numerical index, e.g. cert.pem becomes
 cert.pem.1, cert.pem.2, and so on; or
 
 b) as above, but suffix with serial number and, if there are
 different issues, some issuer-identifying information.
 
 Let me know your thoughts.
 
 Thanks,
 Fraser
 
 
 Is there a possible way how to store certificates into one file?
 I read about possibilities to have multiple certs in one .pem
 file, but I'm
 not cert guru :)
 
 I personally vote for serial number in case there are multiple
 certificates,
 if ^ is no possible.
 
 
 1)
 +if len(certs)  0:
 
 please use only,
 if certs:
 
 2)
 You need to re-generate API/ACI.txt in this patch
 
 3)
 syntax error:
 +for dercert in certs_der
 
 
 4)
 command
 ipa user-mod ca_user --certificate=ceritifcate
 
 removes the current certificate from the LDAP, by design.
 Should be the old certificate(s) revoked? You removed that part in
 the code.
 
 only the --addattr='usercertificate=cert' appends new
 value there
 
 -- 
 Martin Basti
 
 My objections/proposed solutions in attached patch.
 
 * VERSION
 * In the previous version normalized values was stored in LDAP, so I
 added
 it back.  (I dont know why there is no normalization in param
 settings, but
 normalization for every certificate is done in callback. I will
 file a
 ticket for this)
 * IMO only normalized certificates should be compared in the old
 certificates detection
 
 I incorporated your suggested changes in new patch (attached).
 
 There were no proposed changes to the other patchset (0001..0013)
 since rebase.
 
 Thanks,
 Fraser
 Thank you,
 ACK
 Martin^2
 
 
 Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212
 
 Regression found.
 
 Patch to fix the issue is attached.
 
 The fix works, thanks.
 
 Milan

Thanks for finding, fixing and testing!  ACK from me.

I also present a fix of my own.  It fixes a problem where
service-mod deleted all certificates when
'--addattr usercertificate=XXX' was used instead of
'--usercertificate=XXX' options.

Cheers,
Fraser
From 5816c655b75a516068301186b20ddc36b966073c Mon Sep 17 00:00:00 2001
From: Fraser Tweedale ftwee...@redhat.com
Date: Wed, 3 Jun 2015 02:49:28 -0400
Subject: [PATCH] Fix certificate management with service-mod

Adding or removing certificates from a service via --addattr or
--delattr is broken.  Get certificates from entry_attrs instead of
options.
---
 ipalib/plugins/service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipalib/plugins/service.py b/ipalib/plugins/service.py
index 
c290344cf6c14155ec1b103525ff8642a7a8e2af..369321d76a7b8e4e0a0d572fa1d26145cca010f4
 100644
--- a/ipalib/plugins/service.py
+++ b/ipalib/plugins/service.py
@@ -598,7 +598,7 @@ class service_mod(LDAPUpdate):
 (service, hostname, realm) = split_principal(keys[-1])
 
 # verify certificates
-certs = options.get('usercertificate') or []
+certs = entry_attrs.get('usercertificate') or []
 certs_der = map(x509.normalize_certificate, certs)
 for dercert in certs_der:
 x509.verify_cert_subject(ldap, hostname, dercert)
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH] 857 topology: ipa management commands

2015-06-03 Thread Martin Babinsky

On 06/03/2015 01:34 PM, Petr Vobornik wrote:

On 06/03/2015 10:59 AM, Martin Babinsky wrote:

On 06/03/2015 10:52 AM, Martin Babinsky wrote:

On 05/26/2015 03:31 PM, Petr Vobornik wrote:

On 05/26/2015 12:19 PM, Petr Vobornik wrote:

this patch is based on top of my patch #856 and tbabej'
s 325-9.

Obsoletes Ludwig's 0006.

ipalib part of topology management

Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology

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




New version attached:
- domainlevel_show usage changed to domainlevel_get
- updated VERSION
- added more attrs to default_attributes




Hi Petr,

the commands themselves seem to work just fine. I had encountered some
quirks in the underlying topology plugin, but I will address them in a
different thread in order to keep the discussion relevant to the
reviewed patch.

I have some minor coomments below:

1.)
  IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: pvoborni - added topology management commands

Several people were touching API in the meantime so please double-check
that you have correct VERSION and regenerate API.txt


Patch rebased.



2.)

+Str(
+'nsds5replicatedattributelist?',
+cli_name='replattrs',
+label='Attributes to replicate',
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a fractional update. E.g., `(objectclass=*) '
+  '$ EXCLUDE accountlockout memberof'),
+),
+Str(
+'nsds5replicatedattributelisttotal?',
+cli_name='replattrstotal',
+label=_('Attributes for total update'),
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a total update. E.g. (objectclass=*) $
EXCLUDE '
+  'accountlockout'),

The descriptions of these two options confused me greatly, are these
attributes supposed to be replicated or not, or is there some more
complex logic behind them that I failed to grasp? I am cc'ing Ludwig, he
can probably explain them to us and then we can decide whether we may
alter the descriptions to be less confusing.

3.)

+takes_params = (
+Str(
+'cn',
+cli_name='name',
+primary_key=True,
+label=_('Suffix name'),
+),
+Str(
+'iparepltopoconfroot',
+maxlength=255,
+cli_name='suffix',
+label=_('Suffix to be managed'),
+normalizer=lambda value: value.lower(),
+),
+)

This also confused me at first, I suggest to change the label of
'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or
'LDAP subtree to be managed'.


Changed to 'LDAP suffix to be managed'



4.)

There is currently no way to rename existing topology segments/suffixes.
In the case of hosts with funky FQDN's (pointing at you, ABC lab), the
segment cn's created during replica installs are mearly impossible to
remember and it would be nice to rename them to something more
manageable. However, this is not related to core functionality and can
be a subject of a separate patch once this gets pushed.

That's all from my side.



I also forgot to ask what is the expected policy when deleting a
non-empty topology suffix. If this is not supported and you have to
first remove all segments and then the suffix itself, the
'topologysuffix-del' command should issue an error pointing the user to
correct procedure.



Do we have a use case for creation or deletion of topology suffix?

That's a good question.

Anyway, I have noticed couple more things:

1.) it seems that there some of unused imports in topology.py. Please 
investigate whether all of them are really needed.


2.)

+from ipalib.plugins.baseldap import *
+from ipalib.plugins import baseldap

I do not like that starred import at all. Either import the particular 
classes you use (like e.g. in basuser.py), or just leave the second 
import statetement and use the appropriate namespace 
(baseldap.LDAPObject etc.).


3.) there are couple of pep8 complaints, please try to fix them unless 
it impairs readability:


./ipalib/constants.py:121:80: E501 line too long (81  79 characters)
./ipalib/plugins/topology.py:72:80: E501 line too long (88  79 characters)
./ipalib/plugins/topology.py:73:26: E131 continuation line unaligned for 
hanging indent

./ipalib/plugins/topology.py:73:80: E501 line too long (93  79 characters)
./ipalib/plugins/topology.py:103:80: E501 line too long (80  79 characters)
./ipalib/plugins/topology.py:111:80: E501 line too long (80  79 characters)
./ipalib/plugins/topology.py:207:80: E501 line too long (80  79 characters)
./ipalib/plugins/topology.py:232:80: E501 line too long (80  79 characters)
./ipalib/plugins/topology.py:269:80: E501 line too long (84  79 characters)

Re: [Freeipa-devel] KeyError raised upon replica installation

2015-06-03 Thread Oleg Fayans

Hi Ludwig,

I'll rebuild the packages again with the whole set of patches including 
0010 and 0011 and try again. Thanks!


On 06/03/2015 02:21 PM, Ludwig Krispenz wrote:


On 06/03/2015 02:05 PM, Oleg Fayans wrote:

Update:

The original error occurs ONLY when installing a replica from a gpg 
file prepared on a master running FreeIPA 4.1.2.

but this should be covere with patch 0010

If The master runs the upstream code, it works.

On 06/02/2015 02:11 PM, Martin Babinsky wrote:

On 06/02/2015 02:07 PM, Martin Babinsky wrote:

On 06/02/2015 12:09 PM, Oleg Fayans wrote:

Hi all,

The following error was caught during replica installation (I used 
all

the latest patches from Ludwig and Martin Basti):

root@localhost:/home/ofayans/rpms]$ ipa-replica-install --setup-ca
--setup-dns --forwarder 10.38.5.26
/var/lib/ipa/replica-info-replica1.zaeba.li.gpg
Directory Manager (existing master) password:

Existing BIND configuration detected, overwrite? [no]: yes
Adding [192.168.122.210 replica1.zaeba.li] to your /etc/hosts file
Checking forwarders, please wait ...
Using reverse zone(s) 122.168.192.in-addr.arpa.
Run connection check to master
Check connection from replica to remote master 
'upgrademaster.zaeba.li':

Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos Kpasswd: TCP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

The following list of ports use UDP protocol and would need to be
checked manually:
Kerberos KDC: UDP (88): SKIPPED
Kerberos Kpasswd: UDP (464): SKIPPED

Connection from replica to master is OK.
Start listening on required ports for remote master check
Get credentials to log in to remote master
ad...@zaeba.li password:

Check SSH connection to remote master
Execute check on remote master
Check connection from master to remote replica 'replica1.zaeba.li':
Directory Service: Unsecure port (389): OK
Directory Service: Secure port (636): OK
Kerberos KDC: TCP (88): OK
Kerberos KDC: UDP (88): OK
Kerberos Kpasswd: TCP (464): OK
Kerberos Kpasswd: UDP (464): OK
HTTP Server: Unsecure port (80): OK
HTTP Server: Secure port (443): OK

Connection from master to replica is OK.

Connection check OK
Configuring NTP daemon (ntpd)
   [1/4]: stopping ntpd
   [2/4]: writing configuration
   [3/4]: configuring ntpd to start on boot
   [4/4]: starting ntpd
Done configuring NTP daemon (ntpd).
Configuring directory server (dirsrv): Estimated time 1 minute
   [1/37]: creating directory server user
   [2/37]: creating directory server instance
   [3/37]: adding default schema
   [4/37]: enabling memberof plugin
   [5/37]: enabling winsync plugin
   [6/37]: configuring replication version plugin
   [7/37]: enabling IPA enrollment plugin
   [8/37]: enabling ldapi
   [9/37]: configuring uniqueness plugin
   [10/37]: configuring uuid plugin
   [11/37]: configuring modrdn plugin
   [12/37]: configuring DNS plugin
   [13/37]: enabling entryUSN plugin
   [14/37]: configuring lockout plugin
   [15/37]: configuring topology plugin
   [16/37]: creating indices
   [17/37]: enabling referential integrity plugin
   [18/37]: configuring ssl for ds instance
   [19/37]: configuring certmap.conf
   [20/37]: configure autobind for root
   [21/37]: configure new location for managed entries
   [22/37]: configure dirsrv ccache
   [23/37]: enable SASL mapping fallback
   [24/37]: restarting directory server
   [25/37]: setting up initial replication
Starting replication, please wait until this has completed.
Update in progress, 7 seconds elapsed
Update succeeded

   [26/37]: updating schema
   [27/37]: setting Auto Member configuration
   [28/37]: enabling S4U2Proxy delegation
   [29/37]: importing CA certificates from LDAP
   [30/37]: initializing group membership
   [31/37]: adding master entry
ipa : CRITICAL Failed to load master-entry.ldif: Command
''/usr/bin/ldapmodify' '-v' '-f' '/tmp/tmpFlM3mD' '-H'
'ldap://replica1.zaeba.li:389' '-x' '-D' 'cn=Directory Manager' '-y'
'/tmp/tmpk_R0Lm'' returned non-zero exit status 68
   [32/37]: initializing domain level
   [33/37]: configuring Posix uid/gid generation
   [34/37]: adding replication acis
   [35/37]: enabling compatibility plugin
   [36/37]: tuning directory server
   [37/37]: configuring directory to start on boot
Done configuring directory server (dirsrv).
Configuring certificate server (pki-tomcatd): Estimated time 3 
minutes

30 seconds
   [1/21]: creating certificate server user
   [2/21]: configuring certificate server instance
   [3/21]: stopping certificate server instance to update CS.cfg
   [4/21]: backing up CS.cfg
   [5/21]: disabling nonces
   [6/21]: set up CRL publishing
   [7/21]: enable PKIX certificate path discovery and validation
   [8/21]: starting certificate server instance
   [9/21]: creating RA agent certificate database
   [10/21]: importing CA chain to RA certificate database
   

Re: [Freeipa-devel] [PATCH 424] install: Introduce installer framework ipapython.install

2015-06-03 Thread Martin Basti

On 02/06/15 15:21, Jan Cholasta wrote:

Dne 11.5.2015 v 13:41 Jan Cholasta napsal(a):

Dne 6.5.2015 v 08:22 Jan Cholasta napsal(a):

Dne 6.5.2015 v 08:11 Martin Kosek napsal(a):

On 04/29/2015 06:25 PM, Jan Cholasta wrote:

Dne 20.4.2015 v 16:56 Jan Cholasta napsal(a):

Dne 20.4.2015 v 15:14 Martin Basti napsal(a):

On 17/04/15 16:15, Jan Cholasta wrote:

Dne 16.4.2015 v 16:46 Jan Cholasta napsal(a):

Hi,

the attached patch adds the basics of the new installer 
framework.


As a next step, I plan to convert the install scripts to use the
framework with their old code (the old code will be gradually
ported to
the framework later).

(Note I didn't manage to write docstrings today, expect update
tomorrow.)


Added some docstrings.

Also updated the patch to reflect little brainstorming David and I
had
this morning.



Honza





Hello, see comments bellow:

1) We started using new shorter License header in files:
#
# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
#


OK.



2) IMO this will not work, NoneType has no 'obj' attribute
+else:
+if isinstance(value, from_):
+value = None
+stack.append(value.obj)
+continue


Right.



3) Multiple inheritance. I do not like it much.
+class CompositeInstaller(Installer, CompositeConfigurator):


I guess you are antagonistic to multiple inheritance because of how
other languages (like C++) do it. In Python it can be pretty elegant
and
is basis for e.g. the mixin design pattern.



Installer and CompositeConfigurator inherites from Configurator
class,
and all of them implements _generator method.


Both of them call super()._generator(), so it's no problem (same for
other methods).



If I understand correctly
(https://www.python.org/download/releases/2.3/mro/) the
Installer._generator method will be used in this case.
However in case when CompositeConfigurator has more levels
(respectively
it is more specialized) of inheritance, it could take precedence
and its
_generator method may be used instead.


The order of precedence is defined by the order of base classes 
in the

class definition.



I'm afraid this may suddenly stop working.
Maybe I'm wrong, please fix me.


As long as you call the super class, it will work fine.



And Multiple inheritance is not easily readable, this is even a
diamond
inheritance model.


Cooperative inheritance is used by design and IMHO is easily
readable if
you know how to read it. Every class defines a single bit of 
behavior.
Without cooperative inheritance, it would have to be hardcoded 
and/or

hacked around, which I wanted to avoid.

This blog post explains it nicely:
https://rhettinger.wordpress.com/2011/05/26/super-considered-super/. 





Updated patch attached.

Also attached is patch 425 which migrates ipa-server-install to the
install
framework.


Good job there. I am just curious, will this framework and new option
processing be friendly to other types of option passing than just via
options?
I mean tickets

https://fedorahosted.org/freeipa/ticket/4517
https://fedorahosted.org/freeipa/ticket/4468

Especially 4517 is important, we need to be able to run

# cat install.conf
ds_password=Secret123
admin_password=Secret456
ip_address=123456
setup_dns=False

# ipa-server-install --unattended --conf install.conf

I assume yes, but I am just making sure.


Yes, definitely.



Updated patches attached.


Another update, patches attached.


thank you,

1)
ipa-server-install --uninstall prints 0
...
Unconfiguring ipa_memcached
Unconfiguring ipa-otpd
0
The ipa-server-install command was successful


2)
ipa-server-install --setup-dns
'ServerOptions' object has no attribute 'dnssec_master'

3)
For record, this will be fixed in extra patch.
info messages from ldapupdate are printed to console

4)
+if default is not _missing:
+class_dict['default'] = default

Why is new _missing object needed? Isn't NoneType enough?


--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Simo Sorce
On Wed, 2015-06-03 at 09:27 +0200, Martin Kosek wrote:
 On 06/02/2015 08:34 PM, Simo Sorce wrote:
  On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote:
  Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a):
  On 5/28/2015 12:46 AM, Jan Cholasta wrote:
  On a related note, since KRA is optional, can we move the vaults
  container to cn=kra,cn=vaults? This is the convetion used by the other
  optional components (DNS and recently CA).
 
  I mean cn=vaults,cn=kra of course.
 
  If you are talking about the o=kra,PKI suffix, I'm not sure whether
  the IPA framework will work with it.
 
  If you are talking about adding a new cn=kra,IPA suffix entry on top
  of cn=vaults, what is the purpose of this entry? Is the entry going to
  be created/deleted automatically when the KRA is installed/removed? Is
  it going to be used for something else other than vaults?
 
  I'm talking about cn=kra,IPA suffix. It should be created only when 
  KRA is installed, although I think this can be done later after the 
  release, moving vaults to cn=kra should be good enough for now. It's 
  going to be used for everything KRA-specific.
 
 
  There are a lot of questions that need to be answered before we can make
  this change.
 
  This is about sticking to a convention, which everyone should do, and 
  everyone except KRA already does.
 
  I'm sorry I didn't realize this earlier, but the change must be done now.
 
  We probably should revisit this issue after the core vault
  functionality is added.
 
 
  We can't revisit it later because after release we are stuck with 
  whatever is there forever.
 
  See attachment for a patch which implements the change.
 
  
  Shouldn't we s/kra/vault/ ?
  After all the feature is called Vault, not KRA.
 
 I thought we are naming it by the name of the optional subsystem, not the
 feature itself. If for example, another feature from KRA is used, it would
 still live in cn=kra, no?

For services so far we have CA, not dogtag, and LDAP, not 389ds, also
KDC not krb5kdc and kpasswd not kadmind, etc... we normally named
everything after the function. Now kra is probably a somewhat generic
term, but I have not been able to find what it means exactly in 5
minutes, and it is quite obscure as a name. OTOH cn=Vault would make it
really clear what's in it. I do not have a very strong opinion but a
generic and clear name is important for the DIT.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] Password vault

2015-06-03 Thread Jan Cholasta

Dne 3.6.2015 v 15:20 Simo Sorce napsal(a):

On Wed, 2015-06-03 at 09:27 +0200, Martin Kosek wrote:

On 06/02/2015 08:34 PM, Simo Sorce wrote:

On Tue, 2015-06-02 at 12:04 +0200, Jan Cholasta wrote:

Dne 2.6.2015 v 02:02 Endi Sukma Dewata napsal(a):

On 5/28/2015 12:46 AM, Jan Cholasta wrote:

On a related note, since KRA is optional, can we move the vaults
container to cn=kra,cn=vaults? This is the convetion used by the other
optional components (DNS and recently CA).


I mean cn=vaults,cn=kra of course.


If you are talking about the o=kra,PKI suffix, I'm not sure whether
the IPA framework will work with it.

If you are talking about adding a new cn=kra,IPA suffix entry on top
of cn=vaults, what is the purpose of this entry? Is the entry going to
be created/deleted automatically when the KRA is installed/removed? Is
it going to be used for something else other than vaults?


I'm talking about cn=kra,IPA suffix. It should be created only when
KRA is installed, although I think this can be done later after the
release, moving vaults to cn=kra should be good enough for now. It's
going to be used for everything KRA-specific.



There are a lot of questions that need to be answered before we can make
this change.


This is about sticking to a convention, which everyone should do, and
everyone except KRA already does.

I'm sorry I didn't realize this earlier, but the change must be done now.


We probably should revisit this issue after the core vault
functionality is added.



We can't revisit it later because after release we are stuck with
whatever is there forever.

See attachment for a patch which implements the change.



Shouldn't we s/kra/vault/ ?
After all the feature is called Vault, not KRA.


I thought we are naming it by the name of the optional subsystem, not the
feature itself. If for example, another feature from KRA is used, it would
still live in cn=kra, no?


For services so far we have CA, not dogtag, and LDAP, not 389ds, also
KDC not krb5kdc and kpasswd not kadmind, etc... we normally named
everything after the function. Now kra is probably a somewhat generic
term, but I have not been able to find what it means exactly in 5
minutes, and it is quite obscure as a name. OTOH cn=Vault would make it
really clear what's in it. I do not have a very strong opinion but a
generic and clear name is important for the DIT.


There is also ipa-kra-install and I guess 
cn=KRA,cn=fqdn,cn=masters,cn=ipa,cn=etc. If we rename it, it should be 
renamed everywhere, and I'm not sure if that's worth it.


Also vault is too generic, it should be password vault, but that's 
too long, so IMO KRA is better, as it's short and descriptive.


Are vaults the only feature KRA provides? If there are more possible 
features provided by KRA, it's another reason to keep it KRA.


--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Ludwig Krispenz

Hi Petr,

good catch. I didn't check for self referential segments. There is a 
check for existing segments, but unfortuantely the entry lookup in the 
pblock was incorrect and the test always passed.


For the removal, there is teh assumption that no duplicate segments 
exist and so removal of A-B only succeeds if there is another path from 
A to B.


I'm building a patch and will sen to the list soon

Ludwig

On 06/03/2015 12:51 PM, Petr Vobornik wrote:

On 06/03/2015 11:37 AM, Martin Babinsky wrote:

Hi everyone,

I have been playing with the topology related patches and I have
encountered a few issues that I would like to address in this thread:



Additional stuff:

1. was able to add duplicate segment
- same left and right node
- same direction
- different cn

It did not allow me to remove it:

Server is unwilling to perform: Removal of Segment disconnects 
topology.Deletion not allowed.



2. topology plugin allows to create reflexive relation from the 
invalid duplicates(#1):


A - B
A - B
to
A - A
B - B

I.E. effective disconnect

it is forbidden in `ipa topologysegment-mod` but I think that even the 
plugin should not allow that


3. attempt to delete the invalid reflexive or duplicate segment ends 
with:


Server is unwilling to perform: Removal of Segment disconnects 
topology.Deletion not allowed.





--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0001] Migrate now accepts scope as argument

2015-06-03 Thread Martin Basti

On 02/06/15 22:32, Drew Erny wrote:
Sorry, the email address on that patch is wrong. It picked the old one 
off my personal box when I migrated my dotfiles. I don't know if 
that's important, but if the merger could 
s/dpe...@crimson.ua.edu/de...@redhat.com/g, that would be better. 
Sorry about that, I'll fix it in my next patch.


On 06/02/2015 04:23 PM, Drew Erny wrote:

Hi, all,

This is my first patch, which fixes Ticket #2547 at 
https://fedorahosted.org/freeipa/ticket/2547


It introduces a --scope option to ipa migrate-ds which allows the 
user to specify the search depth of a migration. The previous default 
behavior is the same as --scope=onelevel. To search nested OUs, the 
user uses --scope=subtree. --scope=base will cause the migrate script 
not to find anything, but has been included for completeness. Any 
other option is invalid and will cause the command to abort.


Please review this one carefully, because I'm only like 98% confident 
it doesn't break anything. The only thing I'm not sure about is that 
if you run ipa migrate-ds without --scope specified, it gives an 
interactive input for that option; I'm not sure if it's supposed to 
do that.


Thanks,

Drew Erny
de...@redhat.com








Hello,

thank you for your patch.

1)
Please don't use backslash

+doc=_('LDAP search scope for users and groups: base, 
onelevel, or '\

+  'subtree. Defaults to onelevel'),

2)
You can use dictionary:

_default_scope = 'onelevel'  # I do not like hardcoded index there
_supported_scopes = {'base': ldap.SCOPE_BASE, _default_scope: 
ldap.SCOPE_ONELEVEL, ...}



StrEnum(

values=_supported_scopes.keys(),
default=_default_scope
)

scope = _supported_scopes[options.get('scope', _default_scope)]   # or 
autofill=True should be in StrEnum param for scope instead, I'm not 
sure, you must test it :-)


3) do not forget to change the email

PS: I did not test the code, it is just example.

Martin^2

--
Martin Basti

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCHES 0233-0234] DNSSEC: forwarders validation

2015-06-03 Thread Martin Basti

On 03/06/15 14:57, Petr Spacek wrote:

On 18.5.2015 13:48, Martin Basti wrote:

On 15/05/15 18:11, Petr Spacek wrote:

On 7.5.2015 18:12, Martin Basti wrote:

On 07/05/15 12:19, Petr Spacek wrote:

On 7.5.2015 08:59, David Kupka wrote:

On 05/06/2015 03:20 PM, Martin Basti wrote:

On 05/05/15 15:00, Martin Basti wrote:

On 30/04/15 15:37, David Kupka wrote:

On 04/24/2015 02:56 PM, Martin Basti wrote:

Patches attached.





Hi,
thanks for patches.

1. You changed message in DNSServerNotRespondingWarning class but not
the test in ipatest/test_xmlrpc/test_dns_plugin.py

nitpick. Please spell 'edns' correctly. I've seen several instances
of 'ends'.


Thank you,

updated patches attached:
* new error messages
* logging to debug log server output if exception was raised
* fixed test
* fixed spelling




Fixed tests (again)

Updated patches attached


The code looks good to me and tests are no longer broken. (I would prefer
better fix of the tests but given that the priorities are different now
it can
wait.)

Petr, can you please confirm that the patch set works for you?

Sorry, NACK:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: an internal error has occurred

# /var/log/httpd/error_log
ipa: ERROR: non-public: AssertionError:
Traceback (most recent call last):
 File /usr/lib/python2.7/site-packages/ipaserver/rpcserver.py, line
350, in
wsgi_execute
   result = self.Command[name](*args, **options)
 File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 443, in
__call__
   ret = self.run(*args, **options)
 File /usr/lib/python2.7/site-packages/ipalib/frontend.py, line 760,
in run
   return self.execute(*args, **options)
 File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line
, in
execute
   **options)
 File /usr/lib/python2.7/site-packages/ipalib/plugins/dns.py, line
4405, in
_warning_if_forwarders_do_not_work
   log=self.log)
 File /usr/lib/python2.7/site-packages/ipalib/util.py, line 715, in
validate_dnssec_zone_forwarder_step2
   timeout=timeout)
 File /usr/lib/python2.7/site-packages/ipalib/util.py, line 610, in
_resolve_record
   assert isinstance(nameserver_ip, basestring)
AssertionError
ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(DNS
name ptr.test., idnsforwarders=(u'10.34.47.236',), all=False, raw=False,
version=u'2.116'): AssertionError

This is constantly reproducible in my vm-090.abc. Let me know if you want to
take a look.


I'm attaching little response.patch which improves compatibility with older
python-dns packages. This patch allows IPA to work while error messages are
simply not as nice as they could be with latest python-dns :-)

check_fwd_msg.patch is a little nitpick, just to make sure everyone
understands the message.

BTW why some messages in check_forwarders() are printed using 'print' and
others using logger? I would prefer to use logger for everything to make sure
that logs contain all the information, including warnings.

Thank you for your time!


Thank you, fixed.

I  added missing except block after forwarders validation step2.

I confirm that this works but I just discovered another deficiency.

Setup:
- DNSSEC validation is enabled on IPA server
- forwarders uses fake TLD, e.g. 'test.'
- remote DNS server is responding, supports EDNS0 and so on

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The DNS query
name does not exist: ptr.test..

Huh? Let's check named log:
   forward zone 'ptr.test': loaded
   validating ./SOA: got insecure response; parent indicates it should be secure

Sometimes I get SERVFAIL from IPA server, too.


Unfortunately this check was the main reason for writing this patchset so we
need to improve it.

Maybe validate_dnssec_zone_forwarder_step2() could special-case NXDOMAIN and
print the DNSSEC-validation-failed error, too? The problem is that it could
trigger some false positives because NXDOMAIN may simply be caused by a delay
somewhere.

Any ideas?

I add catch block for NXDOMAIN

By the way, this is also weird:

$ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236
Server will check DNS forwarder(s).
This may take some time, please wait ...
ipa: ERROR: DNS forward zone with name ptr.test. already exists

Is it actually doing the check even if the forward zone exists already? (This
is just nitpick, not a blocker!)


The first part is written by IPA client, it is not response from server.
It is just written when user use --forwarder option.

Updated patch attached.

NACK, it does not work for me - it explodes when I try to add a forward zone:

$ ipa dnsforwardzone-add ptr.test. --forwarder=192.0.2.1

ipa: ERROR: non-public: TypeError: _warning_if_forwarders_do_not_work() got
multiple values for 

Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Simo Sorce
On Wed, 2015-06-03 at 11:37 +0200, Martin Babinsky wrote:
 3.) It seems that the removal of topology suffixes containing 
 functioning segments is not handled well. I once tried to do this and
 it 
 led to segmentation fault on the dirsrv instance. What is the
 expected 
 behavior in this scenario?

Dirsrv crashes are always critical bugs, please file tickets if you see
any.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0010] KeyError raised upon replica installation

2015-06-03 Thread Simo Sorce
On Wed, 2015-06-03 at 16:10 +0200, Petr Vobornik wrote:
 On 06/02/2015 02:20 PM, Ludwig Krispenz wrote:
  replicas installed from older versions do not have a binddn group
  just accept the errror
 
 ACK
 
 Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76
 
 Note that this group will be populated later. IMHO it should be done as 
 a part of domain-level raise procedure before setting the new level.

Creating this group and populating it should be part of ipa-ldap-update
(sorry forgot the right name) and should be done when we install new
rpms. Each server must care by itself to populate this group with its
own membership.
In particular this *should* not be done when the domain level is raised,
it is already late then.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 857 topology: ipa management commands

2015-06-03 Thread Martin Babinsky

On 06/03/2015 03:53 PM, Petr Vobornik wrote:

On 06/03/2015 02:38 PM, Martin Babinsky wrote:

On 06/03/2015 01:34 PM, Petr Vobornik wrote:

On 06/03/2015 10:59 AM, Martin Babinsky wrote:

On 06/03/2015 10:52 AM, Martin Babinsky wrote:

On 05/26/2015 03:31 PM, Petr Vobornik wrote:

On 05/26/2015 12:19 PM, Petr Vobornik wrote:

this patch is based on top of my patch #856 and tbabej'
s 325-9.

Obsoletes Ludwig's 0006.

ipalib part of topology management

Design:
- http://www.freeipa.org/page/V4/Manage_replication_topology

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




New version attached:
- domainlevel_show usage changed to domainlevel_get
- updated VERSION
- added more attrs to default_attributes




Hi Petr,

the commands themselves seem to work just fine. I had encountered some
quirks in the underlying topology plugin, but I will address them in a
different thread in order to keep the discussion relevant to the
reviewed patch.

I have some minor coomments below:

1.)
  IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=121
-# Last change: pvoborni - added server-find and server-show
+IPA_API_VERSION_MINOR=122
+# Last change: pvoborni - added topology management commands

Several people were touching API in the meantime so please
double-check
that you have correct VERSION and regenerate API.txt


Patch rebased.



2.)

+Str(
+'nsds5replicatedattributelist?',
+cli_name='replattrs',
+label='Attributes to replicate',
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a fractional update. E.g.,
`(objectclass=*) '
+  '$ EXCLUDE accountlockout memberof'),
+),
+Str(
+'nsds5replicatedattributelisttotal?',
+cli_name='replattrstotal',
+label=_('Attributes for total update'),
+doc=_('Attributes that are not replicated to a consumer
server '
+  'during a total update. E.g. (objectclass=*) $
EXCLUDE '
+  'accountlockout'),

The descriptions of these two options confused me greatly, are these
attributes supposed to be replicated or not, or is there some more
complex logic behind them that I failed to grasp? I am cc'ing
Ludwig, he
can probably explain them to us and then we can decide whether we may
alter the descriptions to be less confusing.

3.)

+takes_params = (
+Str(
+'cn',
+cli_name='name',
+primary_key=True,
+label=_('Suffix name'),
+),
+Str(
+'iparepltopoconfroot',
+maxlength=255,
+cli_name='suffix',
+label=_('Suffix to be managed'),
+normalizer=lambda value: value.lower(),
+),
+)

This also confused me at first, I suggest to change the label of
'iparepltopoconfroot' to something like 'LDAP suffix to be managed' or
'LDAP subtree to be managed'.


Changed to 'LDAP suffix to be managed'



4.)

There is currently no way to rename existing topology
segments/suffixes.
In the case of hosts with funky FQDN's (pointing at you, ABC lab), the
segment cn's created during replica installs are mearly impossible to
remember and it would be nice to rename them to something more
manageable. However, this is not related to core functionality and can
be a subject of a separate patch once this gets pushed.

That's all from my side.



I also forgot to ask what is the expected policy when deleting a
non-empty topology suffix. If this is not supported and you have to
first remove all segments and then the suffix itself, the
'topologysuffix-del' command should issue an error pointing the user to
correct procedure.



Do we have a use case for creation or deletion of topology suffix?

That's a good question.

Anyway, I have noticed couple more things:

1.) it seems that there some of unused imports in topology.py. Please
investigate whether all of them are really needed.


Fixed



2.)

+from ipalib.plugins.baseldap import *
+from ipalib.plugins import baseldap

I do not like that starred import at all. Either import the particular
classes you use (like e.g. in basuser.py), or just leave the second
import statetement and use the appropriate namespace
(baseldap.LDAPObject etc.).


Fixed



3.) there are couple of pep8 complaints, please try to fix them unless
it impairs readability:

./ipalib/constants.py:121:80: E501 line too long (81  79 characters)
./ipalib/plugins/topology.py:72:80: E501 line too long (88  79
characters)
./ipalib/plugins/topology.py:73:26: E131 continuation line unaligned for
hanging indent
./ipalib/plugins/topology.py:73:80: E501 line too long (93  79
characters)
./ipalib/plugins/topology.py:103:80: E501 line too long (80  79
characters)
./ipalib/plugins/topology.py:111:80: E501 line too long (80  79
characters)
./ipalib/plugins/topology.py:207:80: E501 line too long (80  79
characters)
./ipalib/plugins/topology.py:232:80: E501 line too long (80  79
characters)

Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates

2015-06-03 Thread Martin Basti

On 03/06/15 15:21, Fraser Tweedale wrote:

On Wed, Jun 03, 2015 at 01:55:47PM +0200, Milan Kubik wrote:

On 06/03/2015 01:17 PM, Martin Basti wrote:

On 02/06/15 16:03, Jan Cholasta wrote:

Dne 2.6.2015 v 12:36 Martin Basti napsal(a):

On 02/06/15 11:42, Fraser Tweedale wrote:

On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote:

On 01/06/15 06:40, Fraser Tweedale wrote:

New version of patch; ``{host,service}-show --out=FILE`` now writes
all certs to FILE.  Rebased on latest master.

Thanks,
Fraser

On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote:

Updated patch attached.  Notably restores/adds revocation behaviour
to host-mod and service-mod.

Thanks,
Fraser

On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:

On 27/05/15 15:53, Fraser Tweedale wrote:

This patch adds supports for multiple user / host
certificates.  No
schema change is needed ('usercertificate' attribute is already
multi-value).  The revoke-previous-cert behaviour of host-mod and
user-mod has been removed but revocation behaviour of -del and
-disable is preserved.

The latest profiles/caacl patchset (0001..0013 v5) depends
on this
patch for correct cert-request behaviour.

There is one design question (or maybe more, let me know): the
`--out=FILENAME' option to {host,service} show saves ONE
certificate
to the named file.  I propose to either:

a) write all certs, suffixing suggested filename with either a
sequential numerical index, e.g. cert.pem becomes
cert.pem.1, cert.pem.2, and so on; or

b) as above, but suffix with serial number and, if there are
different issues, some issuer-identifying information.

Let me know your thoughts.

Thanks,
Fraser



Is there a possible way how to store certificates into one file?
I read about possibilities to have multiple certs in one .pem
file, but I'm
not cert guru :)

I personally vote for serial number in case there are multiple
certificates,
if ^ is no possible.


1)
+if len(certs)  0:

please use only,
if certs:

2)
You need to re-generate API/ACI.txt in this patch

3)
syntax error:
+for dercert in certs_der


4)
command
ipa user-mod ca_user --certificate=ceritifcate

removes the current certificate from the LDAP, by design.
Should be the old certificate(s) revoked? You removed that part in
the code.

only the --addattr='usercertificate=cert' appends new
value there

--
Martin Basti


My objections/proposed solutions in attached patch.

* VERSION
* In the previous version normalized values was stored in LDAP, so I
added
it back.  (I dont know why there is no normalization in param
settings, but
normalization for every certificate is done in callback. I will
file a
ticket for this)
* IMO only normalized certificates should be compared in the old
certificates detection


I incorporated your suggested changes in new patch (attached).

There were no proposed changes to the other patchset (0001..0013)
since rebase.

Thanks,
Fraser

Thank you,
ACK
Martin^2


Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212


Regression found.

Patch to fix the issue is attached.


The fix works, thanks.

Milan

Thanks for finding, fixing and testing!  ACK from me.

I also present a fix of my own.  It fixes a problem where
service-mod deleted all certificates when
'--addattr usercertificate=XXX' was used instead of
'--usercertificate=XXX' options.

Cheers,
Fraser

ACK

--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] Topology plugin quirks

2015-06-03 Thread Simo Sorce
On Wed, 2015-06-03 at 12:51 +0200, Petr Vobornik wrote:
 On 06/03/2015 11:37 AM, Martin Babinsky wrote:
  Hi everyone,
 
  I have been playing with the topology related patches and I have
  encountered a few issues that I would like to address in this thread:
 
 
 Additional stuff:
 
 1. was able to add duplicate segment
 - same left and right node
 - same direction
 - different cn
 
 It did not allow me to remove it:
 
 Server is unwilling to perform: Removal of Segment disconnects 
 topology.Deletion not allowed.
 

Odd, I would think that if you have 2 segments then either one would
satisfy the topology requirement.
Ludwig,
why is the plugin allowing 2 segments and then does not recognize there
is another one at  removal time ?

 2. topology plugin allows to create reflexive relation from the invalid 
 duplicates(#1):
 
 A - B
 A - B
 to
 A - A
 B - B
 
 I.E. effective disconnect
 
 it is forbidden in `ipa topologysegment-mod` but I think that even the 
 plugin should not allow that

Yes, the plugin must forbid this case on its own.

 3. attempt to delete the invalid reflexive or duplicate segment ends with:
 
 Server is unwilling to perform: Removal of Segment disconnects 
 topology.Deletion not allowed.

The plugin should not allow duplicates or reflexive segments in the
first place, so this should never be required then.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0011] check-for-existing-and-self-referential-segments

2015-06-03 Thread Simo Sorce
On Wed, 2015-06-03 at 14:53 +0200, Ludwig Krispenz wrote:
 Hi,
 
 this should prevent adding duplicate segments or segments with same 
 start and end node

LGTM!

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH] 822 webui: topology plugin

2015-06-03 Thread Martin Babinsky

On 05/27/2015 04:14 PM, Petr Vobornik wrote:

On 05/26/2015 12:22 PM, Petr Vobornik wrote:

On 05/15/2015 01:50 PM, Petr Vobornik wrote:

On 04/21/2015 04:09 PM, Petr Vobornik wrote:

First iteration of Topology plugin Web UI.

It reflects current state of topology plugin python part which is
implemented in [PATCH] manage replication topology in the shared tree
and my wip patch.

I expect that the server API part will change a bit therefore this will
as well.

Graphical visualization/management (ticket 4286)  will be
implemented in
separate patch.

https://fedorahosted.org/freeipa/ticket/4997
http://www.freeipa.org/page/V4/Manage_replication_topology




New version attached. It requires stage user web ui patches in order to
apply (I expect that user life cycle backend will be pushed sooner than
topology)

Changes:
- Left host and Right host fields are now host comboboxes
- Connectivity are radio buttons with both, left-right, right-left,
none options
- segment name is not a required field in its adder dialog

IMHO Attributes to strip, Attributes to replicate, Attributes for
total update, Initialize replica, Session timeout, Replication
agreement enabled fields should not be just free-form textboxes, but
they should be more specific, e.g. a checkbox for Replication agreement
enabled or integer for Session timeout, but that should be modified
first in the backend python plugin.




New patchset which replaces the old patch.

Contains Web UI for:
- topologysuffix, topologysegment, domain level, server

Backend is implemented in patches:
- tbabej 325-9
- pvoborni 855, 857




New update which reflects the API change in domain level patches.
(domainlevel-show changed to domainlevel-get).

Now it depends only on pvoborni 857-2, the rest was pushed.




The patches seem to do what they are supposed to do.

However, I have not found any UI element implementing the 
'topologysegment-refresh' functionality. Was this only an oversight or 
do you plan to implement it in next patch?


--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0010] KeyError raised upon replica installation

2015-06-03 Thread Simo Sorce
On Wed, 2015-06-03 at 16:50 +0200, Martin Kosek wrote:
 On 06/03/2015 04:10 PM, Petr Vobornik wrote:
  On 06/02/2015 02:20 PM, Ludwig Krispenz wrote:
  replicas installed from older versions do not have a binddn group
  just accept the errror
  
  ACK
  
  Pushed to master: 8457edc14dade724b486540800bcdafb7d9a6f76
  
  Note that this group will be populated later. IMHO it should be done as a 
  part
  of domain-level raise procedure before setting the new level.
 
 As said in other mail, I am not sure why we should be overloading domain-level
 raise command that way.

+1

 I thought, we will create this group when the first replica upgrades to 4.2.
 Whenever a new replica is added/upgraded, it's principal will be added to the
 group also (even if Domain Level is 0).

+1

 Domain Level 1 means that all replicas are 4.2 and thus the group is fully
 populated and Topology can be used.

+1

-- 
Simo Sorce * Red Hat, Inc * New York

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCH 0014] Support multiple user and host certificates

2015-06-03 Thread Martin Basti

On 02/06/15 16:56, Petr Vobornik wrote:

On 05/27/2015 03:53 PM, Fraser Tweedale wrote:

This patch adds supports for multiple user / host certificates.  No
schema change is needed ('usercertificate' attribute is already
multi-value).  The revoke-previous-cert behaviour of host-mod and
user-mod has been removed but revocation behaviour of -del and
-disable is preserved.

The latest profiles/caacl patchset (0001..0013 v5) depends on this
patch for correct cert-request behaviour.

There is one design question (or maybe more, let me know): the
`--out=FILENAME' option to {host,service} show saves ONE certificate
to the named file.  I propose to either:

a) write all certs, suffixing suggested filename with either a
sequential numerical index, e.g. cert.pem becomes
cert.pem.1, cert.pem.2, and so on; or

b) as above, but suffix with serial number and, if there are
different issues, some issuer-identifying information.

Let me know your thoughts.

Thanks,
Fraser



Has anybody tried it with Web UI?

Currently Web UI is designed only for one cert. I wonder if it still 
works even with just one.


We should probably file a ticket.


If there are 2 certificates in a host entry, then the WebUI just shows:
Status
 Valid Certificate Present

Then 'view certificate' shows the second certificate

the 'Get certificate' shows the first certificate

I will file a ticket.

Martin^2

--
Martin Basti

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code


Re: [Freeipa-devel] [PATCHES 0001-0013 v7] Profiles and CA ACLs

2015-06-03 Thread Martin Basti

On 03/06/15 16:17, Fraser Tweedale wrote:

On Tue, Jun 02, 2015 at 06:37:42PM +0200, Martin Basti wrote:

On 02/06/15 14:11, Fraser Tweedale wrote:

On Mon, Jun 01, 2015 at 05:22:28PM +1000, Fraser Tweedale wrote:

On Mon, Jun 01, 2015 at 05:10:58PM +1000, Fraser Tweedale wrote:

On Fri, May 29, 2015 at 01:03:46PM +0200, Martin Kosek wrote:

On 05/29/2015 11:21 AM, Martin Basti wrote:

On 29/05/15 06:17, Fraser Tweedale wrote:

On Thu, May 28, 2015 at 02:42:53PM +0200, Martin Basti wrote:

On 28/05/15 11:48, Martin Basti wrote:

On 27/05/15 16:04, Fraser Tweedale wrote:

Hello all,

Fresh certificate management patchset; Changelog:

- Now depends on patch freeipa-ftweedal-0014 for correct
cert-request behaviour with host and service principals.

- Updated Dogtag dependency to 10.2.4-1.  Should should be in
f22 soon, but for f22 right now or for f21, please grab from my
copr: https://copr.fedoraproject.org/coprs/ftweedal/freeipa/

   Martin^1 could you please add to the quasi-official freeipa
   copr?  SRPM lives at
   https://frase.id.au/pki-core-10.2.4-1.fc21.src.rpm.

- cert-request now verifies that for user principals, CSR CN
matches uid and, DN emailAddress and SAN rfc822Name match user's
email address, if either of those is present.

- Fixed one or two other sneaky little bugs.

On Wed, May 27, 2015 at 01:59:30AM +1000, Fraser Tweedale wrote:

Hi all,

Please find attached the latest certificate management
patchset, which introduces the `caacl' plugin and various fixes
and improvement to earlier patches.

One important change to earlier patches is reverting the name
of the default profile to 'caIPAserviceCert' and using the
existing instance of this profile on upgrade (but not install)
in case it has been modified.

Other notes:

- Still have changes in ipa-server-install (fewer lines now,
though)

- Still have the ugly import hack.  It is not a high priority
for me, i.e. I think it should wait until after alpha

- Still need to update 'service' and 'host' plugins to support
multiple certificates.  (The userCertificate attribute schema
itself is multi-valued, so there are no schema issues here)

- The TODOs in [1]; mostly certprofile CLI conveniences and
supporting multiple profiles for hosts and services (which
requires changes to framework only, not schema).  [1]:
http://idm.etherpad.corp.redhat.com/rhel72-cert-mgmt-progress

Happy reviewing!  I am pleased with the initial cut of the
caacl plugin but I'm sure you will find some things to be fixed
:)

Cheers, Fraser

[root@vm-093 ~]#  ipa-replica-prepare vm-094.example.com
--ip-address 10.34.78.94 Directory Manager (existing master)
password:

Preparing replica for vm-094.example.com from vm-093.example.com
Creating SSL certificate for the Directory Server not well-formed
(invalid token): line 2, column 14

I cannot create replica file.  It work on the upgraded server,
but it doesn't work on the newly installed server.  I'm not sure
if this causes your patches which modifies the ca-installer, or
the newer version of dogtag.

Or if there was any other changes in master, I will continue to
investigate with new RPM from master branch.

Martin^2


ipa-replica-prepare works for: * master branch * master branch +
pki-ca 10.2.4-1

So something in your patches is breaking it

Martin^2


Martin, master + my patches with pki 10.2.4-1 is working for me on
f21 and f22.  Can you provide ipa-replica-prepare --debug output and
Dogtag debug log?  ( /var/log/pki/pki-tomcat/ca/debug )

Thanks,
Fraser

I can not reproduce it today. And I already recycled the VMs from yesterday. :-(


In that case I would suggest ACKingpushing the patch and fixing the bug if
it comes again. The tree may now be a bit unstable, given the number of
patches going in.

My main motivation here is to unblock Fraser.

Thanks,
Martin

Rebased patchset attached; no other changes.

Heads up: I just discovered I have introduced a bug with
ipa-replica-install, when it is spawning the CA instance.  I think
replication it only causes issues with ``--setup-ca``.

I will try and sort it out tomorrow or later tonight (I have to head
out for a few hours now, though); and I'm not suggesting it should
block the push but it's something to be aware of.

Cheers,
Fraser


New patchset attached ; haven't gotten to the bottom of the
ipa-replica-install issue mentioned above, but it fixes an upgrade
bug.

The change is:

diff --git a/ipaserver/install/server/upgrade.py 
b/ipaserver/install/server/upgrade.py
index c288282..c5f4d37 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -316,7 +316,7 @@ def ca_enable_ldap_profile_subsystem(ca):
  caconfig.CS_CFG_PATH,
  directive,
  separator='=')
-if value == 'ProfileSubsystem':
+if value == 'com.netscape.cmscore.profile.ProfileSubsystem':
  needs_update = True
  break
  except OSError, e:
@@ -328,7 +328,7 @@ def 

Re: [Freeipa-devel] [PATCH] Use Exception class instead of BaseException

2015-06-03 Thread Niranjan
Petr Viktorin wrote:
 On 06/01/2015 06:33 AM, Niranjan wrote:
  Greetings,
  
  I would like to present patch for replacing StandardError exception
  with Exception class in ipapython/adminutil.py. Also replacing
  BaseException class with Exception class. 
  
  Though the use of StandardError is many places. I would like to start
  with ipapython/adminutil.py
  
  This is my first patch. Please let me know if my approach on this is 
  correct.
  
  Regards
  Niranjan
  
  
  0001-Use-Exception-class-instead-of-BaseException.patch
  
  
  From 018312f76952ea86c8c6e2396657e0531d2d61ba Mon Sep 17 00:00:00 2001
  From: Niranjan Mallapadi mrniran...@redhat.com
  Date: Mon, 1 Jun 2015 09:41:05 +0530
  Subject: [PATCH] Use Exception class instead of BaseException
  
  1. Replace BaseException with Exception class.
 
 I don't see a reason for this change. This is top-level CLI code that
 handles calling our Python library. We really do want to catch all
 exceptions here, including KeyboardInterrupt and SystemExit.
 
  2. Remove StandardError and use Exception class. StandError is deprecated 
  (Python3)
 
 I'm okay with this change, as long as tests still pass.
 
  3 .From python3.0 use of , is not recommended, instead
  use as keyword (PEP 3110)
 
 +1
I will send a modified patch and also run tests before sending them,
Thanks a lot for the review.

 
 
 -- 
 Petr Viktorin


pgpD9hPJvOkkW.pgp
Description: PGP signature
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 0001 v2] Migrate now accepts scope as argument

2015-06-03 Thread Drew Erny

Hi, all,

This is an updated patch, with the code changes suggested by Martin 
Batsi in my test email. The biggest difference is that I had to do


 from ldap import SCOPE_BASE, SCOPE_ONELEVEL, SCOPE_SUBTREE

To get access to those constants in the global scope. This seems like a 
fairly clean solution, but if it's a code smell, feel free to suggest 
improvements. This should have identical behavior to the last patch, 
except it will autofill scope and no longer prompt interactively.


Thanks,

Drew Erny
de...@redhat.com
From 168e910aef41bd1df661317168236287b2994822 Mon Sep 17 00:00:00 2001
From: Drew Erny de...@redhat.com
Date: Wed, 27 May 2015 09:52:42 -0400
Subject: [PATCH] Migration now accepts scope as argument

Adds a new option to command ipa migrate-ds,
--scope=[base,onelevel,subtree], which allows the user to specify LDAP
search depth for users and groups. 'onelevel' was the previous default
level. Specify 'subtree' to to search nested OUs for users and groups.

fedorahosted.org/freeipa/ticket/2547
---
 API.txt |  3 ++-
 ipalib/plugins/migration.py | 18 +-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/API.txt b/API.txt
index d987bc949948a280018f0f20d5af93838ecaeb20..da124c2d659510cf81d25a5708835cf8ed176efa 100644
--- a/API.txt
+++ b/API.txt
@@ -2450,7 +2450,7 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
 output: PrimaryKey('value', None, None)
 command: migrate_ds
-args: 2,18,4
+args: 2,19,4
 arg: Str('ldapuri', cli_name='ldap_uri')
 arg: Password('bindpw', cli_name='password', confirm=False)
 option: DNParam('basedn?', cli_name='base_dn')
@@ -2466,6 +2466,7 @@ option: Str('groupignoreobjectclass*', autofill=True, cli_name='group_ignore_obj
 option: Str('groupobjectclass+', autofill=True, cli_name='group_objectclass', csv=True, default=(u'groupOfUniqueNames', u'groupOfNames'))
 option: Flag('groupoverwritegid', autofill=True, cli_name='group_overwrite_gid', default=False)
 option: StrEnum('schema?', autofill=True, cli_name='schema', default=u'RFC2307bis', values=(u'RFC2307bis', u'RFC2307'))
+option: StrEnum('scope', autofill=True, cli_name='scope', default=u'onelevel', values=(u'base', u'subtree', u'onelevel'))
 option: DNParam('usercontainer', autofill=True, cli_name='user_container', default=ipapython.dn.DN('ou=people'))
 option: Str('userignoreattribute*', autofill=True, cli_name='user_ignore_attribute', csv=True, default=())
 option: Str('userignoreobjectclass*', autofill=True, cli_name='user_ignore_objectclass', csv=True, default=())
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index c8379420d539ac35901d99f981b4c8e2f0f89ffc..d922d67cbf1a91a201b3b985af36a34e7956300a 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -35,6 +35,8 @@ from ipapython.ipautil import write_tmp_file
 import datetime
 from ipaplatform.paths import paths
 
+from ldap import SCOPE_BASE, SCOPE_ONELEVEL, SCOPE_SUBTREE
+
 __doc__ = _(
 Migration to IPA
 
@@ -140,6 +142,9 @@ _dn_err_msg = _('Malformed DN')
 
 _supported_schemas = (u'RFC2307bis', u'RFC2307')
 
+# search scopes for users and groups when migrating
+_supported_scopes = {u'base': SCOPE_BASE, u'onelevel': SCOPE_ONELEVEL, u'subtree': SCOPE_SUBTREE}
+_default_scope = u'onelevel'
 
 def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs):
 assert isinstance(dn, DN)
@@ -607,6 +612,15 @@ class migrate_ds(Command):
 doc=_('Load CA certificate of LDAP server from FILE'),
 default=None
 ),
+StrEnum('scope',
+cli_name='scope',
+label=_('Search scope'),
+doc=_('LDAP search scope for users and groups: base, onelevel, or '
+  'subtree. Defaults to onelevel'),
+values=tuple(_supported_scopes.keys()),
+default=_default_scope,
+autofill=True,
+),
 )
 
 has_output = (
@@ -711,13 +725,15 @@ can use their Kerberos accounts.''')
 exclude = options['exclude_%ss' % to_cli(ldap_obj_name)]
 context = dict(ds_ldap = ds_ldap)
 
+scope = _supported_scopes[options.get('scope')]
+
 migrated[ldap_obj_name] = []
 failed[ldap_obj_name] = {}
 
 try:
 entries, truncated = ds_ldap.find_entries(
 search_filter, ['*'], search_bases[ldap_obj_name],
-ds_ldap.SCOPE_ONELEVEL,
+scope,
 time_limit=0, size_limit=-1,
 search_refs=True# migrated DS may contain search references
 )
-- 
2.4.2

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0014 v3] Support multiple user and host certificates

2015-06-03 Thread Jan Cholasta

Dne 3.6.2015 v 17:44 Martin Basti napsal(a):

On 03/06/15 15:21, Fraser Tweedale wrote:

On Wed, Jun 03, 2015 at 01:55:47PM +0200, Milan Kubik wrote:

On 06/03/2015 01:17 PM, Martin Basti wrote:

On 02/06/15 16:03, Jan Cholasta wrote:

Dne 2.6.2015 v 12:36 Martin Basti napsal(a):

On 02/06/15 11:42, Fraser Tweedale wrote:

On Mon, Jun 01, 2015 at 02:50:45PM +0200, Martin Basti wrote:

On 01/06/15 06:40, Fraser Tweedale wrote:

New version of patch; ``{host,service}-show --out=FILE`` now
writes
all certs to FILE.  Rebased on latest master.

Thanks,
Fraser

On Thu, May 28, 2015 at 09:18:04PM +1000, Fraser Tweedale wrote:

Updated patch attached.  Notably restores/adds revocation
behaviour
to host-mod and service-mod.

Thanks,
Fraser

On Wed, May 27, 2015 at 06:12:50PM +0200, Martin Basti wrote:

On 27/05/15 15:53, Fraser Tweedale wrote:

This patch adds supports for multiple user / host
certificates.  No
schema change is needed ('usercertificate' attribute is already
multi-value).  The revoke-previous-cert behaviour of
host-mod and
user-mod has been removed but revocation behaviour of -del and
-disable is preserved.

The latest profiles/caacl patchset (0001..0013 v5) depends
on this
patch for correct cert-request behaviour.

There is one design question (or maybe more, let me know): the
`--out=FILENAME' option to {host,service} show saves ONE
certificate
to the named file.  I propose to either:

a) write all certs, suffixing suggested filename with either a
sequential numerical index, e.g. cert.pem becomes
cert.pem.1, cert.pem.2, and so on; or

b) as above, but suffix with serial number and, if there are
different issues, some issuer-identifying information.

Let me know your thoughts.

Thanks,
Fraser



Is there a possible way how to store certificates into one file?
I read about possibilities to have multiple certs in one .pem
file, but I'm
not cert guru :)

I personally vote for serial number in case there are multiple
certificates,
if ^ is no possible.


1)
+if len(certs)  0:

please use only,
if certs:

2)
You need to re-generate API/ACI.txt in this patch

3)
syntax error:
+for dercert in certs_der


4)
command
ipa user-mod ca_user --certificate=ceritifcate

removes the current certificate from the LDAP, by design.
Should be the old certificate(s) revoked? You removed that
part in
the code.

only the --addattr='usercertificate=cert' appends new
value there

--
Martin Basti


My objections/proposed solutions in attached patch.

* VERSION
* In the previous version normalized values was stored in LDAP,
so I
added
it back.  (I dont know why there is no normalization in param
settings, but
normalization for every certificate is done in callback. I will
file a
ticket for this)
* IMO only normalized certificates should be compared in the old
certificates detection


I incorporated your suggested changes in new patch (attached).

There were no proposed changes to the other patchset (0001..0013)
since rebase.

Thanks,
Fraser

Thank you,
ACK
Martin^2


Pushed to master: 7f7c247bb5a4b0030d531f4f14c156162e808212


Regression found.

Patch to fix the issue is attached.


The fix works, thanks.

Milan

Thanks for finding, fixing and testing!  ACK from me.

I also present a fix of my own.  It fixes a problem where
service-mod deleted all certificates when
'--addattr usercertificate=XXX' was used instead of
'--usercertificate=XXX' options.

Cheers,
Fraser

ACK



Pushed both to master: 62e98671142cbc30366109a2a1b631c1ef0cae5c

--
Jan Cholasta

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code