[Freeipa-devel] beware of 389-ds-base-1.3.5.4-1.fc24.x86_64: weird filter/ACI evaluation

[Petr Spacek]

Hello,

TL;DR version:
Upgrade to 389-ds-base-1.3.5.6-1.fc24.

I was facing weird filter/ACI evaluation with 389 DS
389-ds-base-1.3.5.4-1.fc24.x86_64. Here is full story (written before I
realized that DS is old one ...):


Test

First, let's try LDAP search with OR filter consisting of 5 components:

[16/Jun/2016:06:05:18.145159021 +0200] conn=119 op=2 RESULT err=0 tag=97
nentries=0 etime=0
dn="krbprincipalname=dns/vm-046.abc.idm.lab.eng.brq.redhat@dom-046.abc.idm.lab.eng.brq.redhat.com,cn=services,cn=accounts,dc=toplevel"
[16/Jun/2016:06:05:18.145920002 +0200] conn=119 op=3 SRCH
base="cn=dns,dc=toplevel" scope=2
filter="(|(objectClass=idnsConfigObject)(&(objectClass=idnsServerConfigObject)(idnsServerId=vm-046.abc.idm.lab.eng.brq.redhat.com))(objectClass=idnsZone)(objectClass=idnsForwardZone)(objectClass=idnsRecord))"
attrs="objectClass"
[16/Jun/2016:06:05:18.149821586 +0200] conn=119 op=3 RESULT err=0 tag=101
nentries=1 etime=0
[16/Jun/2016:06:05:18.150433307 +0200] conn=119 op=4 UNBIND
[16/Jun/2016:06:05:18.150459102 +0200] conn=119 op=4 fd=108 closed - U1

It returns 1 entry - the idnsServerConfigObject object.



Now let us re-try shortened filter containing only 4 OR components. I would
expect to get less entries but that is not the case:

[16/Jun/2016:06:05:21.007494823 +0200] conn=120 fd=108 slot=108 SSL connection
from 2620:52:0:224e:21a:4aff:fe23:12d2 to 2620:52:0:224e:21a:4aff:fe23:12d2
[16/Jun/2016:06:05:21.022115576 +0200] conn=120 TLS1.2 128-bit AES
[16/Jun/2016:06:05:21.029902095 +0200] conn=120 op=0 BIND dn="" method=sasl
version=3 mech=GSSAPI
[16/Jun/2016:06:05:21.042047525 +0200] conn=120 op=0 RESULT err=14 tag=97
nentries=0 etime=0, SASL bind in progress
[16/Jun/2016:06:05:21.043007851 +0200] conn=120 op=1 BIND dn="" method=sasl
version=3 mech=GSSAPI
[16/Jun/2016:06:05:21.044811757 +0200] conn=120 op=1 RESULT err=14 tag=97
nentries=0 etime=0, SASL bind in progress
[16/Jun/2016:06:05:21.045183711 +0200] conn=120 op=2 BIND dn="" method=sasl
version=3 mech=GSSAPI
[16/Jun/2016:06:05:21.046395695 +0200] conn=120 op=2 RESULT err=0 tag=97
nentries=0 etime=0
dn="krbprincipalname=dns/vm-046.abc.idm.lab.eng.brq.redhat@dom-046.abc.idm.lab.eng.brq.redhat.com,cn=services,cn=accounts,dc=toplevel"
[16/Jun/2016:06:05:21.046947437 +0200] conn=120 op=3 SRCH
base="cn=dns,dc=toplevel" scope=2
filter="(|(objectClass=idnsConfigObject)(objectClass=idnsZone)(objectClass=idnsForwardZone)(objectClass=idnsRecord))"
attrs="objectClass"
[16/Jun/2016:06:05:21.052008250 +0200] conn=120 op=3 RESULT err=0 tag=101
nentries=11 etime=0

Huh? Now we got 11 entries.


When I do the first search as Directory Manager it returns all 12 matching
entries (which is expected number, at least according to my match-by-eye
algorithm :-)).


Schema
==
idnsServerId attribute definition contains an equality specification:
( 2.16.840.1.113730.3.8.5.31 NAME 'idnsServerId' DESC 'DNS server identifier'
EQUALITY caseIgnoreMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 SINGLE-VALUE
X-ORIGIN 'IPA v4.4' )


Indices
===
The attribute itself is not indexed but that should not hurt I guess.

Mere addition of equality index to the attribute did not help.

Reindexing using
https://access.redhat.com/documentation/en-US/Red_Hat_Directory_Server/10/html/Administration_Guide/applying-indexes.html#index-cn-tasks
did not help either.



ACI
===
Relevant ACIs are:
(targetattr = "createtimestamp || entryusn || idnsforwarders ||
idnsforwardpolicy || idnsserverid || idnssoamname || idnssubstitutionvariable
|| modifytimestamp || objectclass")(targetfilter =
"(objectclass=idnsServerConfigObject)")(version 3.0;acl "permission:System:
Read DNS Servers Configuration";allow (compare,read,search) groupdn =
"ldap:///cn=System: Read DNS Servers
Configuration,cn=permissions,cn=pbac,dc=toplevel";)

(targetattr = "idnsforwarders || idnsforwardpolicy || idnssoamname ||
idnssubstitutionvariable")(targetfilter =
"(objectclass=idnsServerConfigObject)")(version 3.0;acl "permission:System:
Modify DNS Servers Configuration";allow (write) groupdn = "ldap:///cn=System:
Modify DNS Servers Configuration,cn=permissions,cn=pbac,dc=toplevel";)


BIND DN
krbprincipalname=dns/vm-046.abc.idm.lab.eng.brq.redhat@dom-046.abc.idm.lab.eng.brq.redhat.com,cn=services,cn=accounts,dc=toplevel
is member of
cn=DNS Servers,cn=privileges,cn=pbac,dc=toplevel
which is member of
cn=System: Read DNS Servers Configuration,cn=permissions,cn=pbac,dc=toplevel

so we should be all good.



Now was totally confused and was looking for a bug in bind-dyndb-ldap until I
realized that DS is returning weird results... Upgrade to
389-ds-base-1.3.5.6-1.fc24.x86_64 fixed that.

This would be a blocker for FreeIPA 4.4 because the old version totally breaks
bind-dyndb-ldap.

-- 
Petr^2 Spacek

-- 
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] Fix minor typos

[Petr Spacek]

On 15.6.2016 20:57, Yuri Chornoivan wrote:
> Hi,
> 
> There are several minor typos in the new portion of FreeIPA code (see the
> patch attached).
> 
> Thanks for fixing them.

Thank *you* for fixing them!

ACK

-- 
Petr^2 Spacek

-- 
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] Fix minor typos

[Yuri Chornoivan]

Hi,

There are several minor typos in the new portion of FreeIPA code (see the  
patch attached).


Thanks for fixing them.

Best regards,
Yuri

0001-Fix-minor-typos.patch
Description: Binary data
-- 
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] Schema caching for thin client

[Petr Vobornik]

On 06/15/2016 02:36 PM, David Kupka wrote:
> Hello!
> Schema caching for thin client is available here:
> 
> https://github.com/dkupka/freeipa/commits/schema_cache
> 
> Comments and reviews welcome.
> 
> Enjoy!

Not doing proper review. I'll test by using it. But:

1. lint fails

Pylint is running, please wait ...
* Module ipaclient.remote_plugins.schema_cache
ipaclient/remote_plugins/schema_cache.py:283: [W1612(unicode-builtin),
_refresh_schema] unicode built-in referenced)
Makefile:137: recipe for target 'lint' failed
make: *** [lint] Error 1

I.e, you miss:

  import six

  if six.PY3:
  unicode = str

-- 
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] 0021 slapi-nis should allow password update on a virtual entry

[thierry bordaz]

Thanks Alexander for the review.
You are right I forgot to remove those lines during the cleanup.

thanks
thierry

On 06/15/2016 05:54 PM, Alexander Bokovoy wrote:

On Wed, 15 Jun 2016, thierry bordaz wrote:

From 6cd06b9004f8ab72e13c26742d11ee31d30bbc79 Mon Sep 17 00:00:00 2001

From: Thierry Bordaz 
Date: Mon, 13 Jun 2016 18:13:04 +0200
Subject: [PATCH] slapi-nis should allow password update on a virtual 
entry


During password modification ext. op (1.3.6.1.4.1.4203.1.11.1),
if the target entry is in the compat tree, slapi-nis should
remap the entry to the real entry.

This needs to be done in a pre-op extop that calls the callback
function handling a given OID.
The password mod. callback does a reverse mapping of
extop USERID and set it in SLAPI_TARGET_SDN.
---
configure.ac   |   1 +
src/back-sch.c | 217 
+

src/back-sch.h |  16 +
src/plug-sch.c |  24 +++
4 files changed, 258 insertions(+)

diff --git a/configure.ac b/configure.ac
index 5b10376..9ce6bcf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,6 +113,7 @@ dirsrv)
SLAPI_PLUGIN_BE_TXN_POST_MODIFY_FN,
SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN,
SLAPI_PLUGIN_BE_TXN_POST_DELETE_FN,
+SLAPI_PLUGIN_PRE_EXTOP_FN,
NULL]
   ,,,
   [AC_INCLUDES_DEFAULT
diff --git a/src/back-sch.c b/src/back-sch.c
index 32b1d9e..f9ab812 100644
--- a/src/back-sch.c
+++ b/src/back-sch.c
@@ -54,6 +54,8 @@
#include "map.h"
#include "back-sch.h"

+backend_extop_handlers_t extop_handlers[] = {{EXTOP_PASSWD_OID, 
(IFP) backend_passwdmod_extop}, +{NULL, NULL}};

static void
backend_entries_to_return_push(struct backend_search_cbdata *cbdata, 
Slapi_Entry *e);


@@ -2223,6 +2225,203 @@ done_with_lock:
return ret;
}

+/* This callback handles EXTOP_PASSWD_OID "1.3.6.1.4.1.4203.1.11.1"
+ * If the extop defines a USERID, it sets SLAPI_TARGET_SDN to
+ * the reverse mapping of the USERID.
+ *
+ * If it is not possible to retrieve USERID in the ber
+ * then value of SLAPI_TARGET_SDN is unchanged.
+ *
+ * Else the value of SLAPI_TARGET_SDN is freed and replaced
+ * either by the USERID or the reverse mapping of USERID (if it exists)
+ */
+static int
+backend_passwdmod_extop(Slapi_PBlock *pb)
+{
+struct backend_entry_data *data;
+struct plugin_state *state;
+Slapi_DN *sdn = NULL;
+char *extopdn;
+char *ndn;
+char *username = NULL;
+char *group = NULL;
+const char *entry_group = NULL;
+char *set = NULL;
+const char *entry_set = NULL;
+struct berval*extop_value = NULL;
+BerElement*ber = NULL;
+ber_tag_ttag = 0;
+ber_len_tlen = (ber_len_t) -1;
+
+if (wrap_get_call_level() > 0) {
+return 0;
+}
+
+slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, );
+if (state->ready_to_serve == 0) {
+/* No data to serve yet */
+goto free_and_return;
+}
+/* Retrieve the original DN from the ber request */
+/* Get the ber value of the extended operation */
+slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, _value);
+if (!BV_HAS_DATA(extop_value)) {
+goto free_and_return;
+}
+
+if ((ber = ber_init(extop_value)) == NULL) {
+goto free_and_return;
+}
++/* Format of request to parse
+ *
+ * PasswdModifyRequestValue ::= SEQUENCE {
+ * userIdentity[0]  OCTET STRING OPTIONAL
+ * oldPasswd   [1]  OCTET STRING OPTIONAL
+ * newPasswd   [2]  OCTET STRING OPTIONAL }
+ *
+ * The request value field is optional. If it is
+ * provided, at least one field must be filled in.
+ */
+
+/* ber parse code */
+if ( ber_scanf( ber, "{") == LBER_ERROR ) {
+/* The request field wasn't provided.  We'll
+ * now try to determine the userid and verify
+ * knowledge of the old password via other
+ * means.
+ */
+goto free_and_return;
+} else {
+tag = ber_peek_tag( ber, );
+}
+
+/* identify userID field by tags */
+if (tag == LDAP_EXTOP_PASSMOD_TAG_USERID ) {
+
+if ( ber_scanf( ber, "a", ) == LBER_ERROR ) {
+slapi_ch_free_string();
+goto free_and_return;
+}
+
+slapi_log_error(SLAPI_LOG_PLUGIN, 
"backend_passwdmod_extop",

+"extopdn = %s\n", extopdn ? extopdn : "" );
+
+/* Free the current target_DN */
+slapi_pblock_get(pb, SLAPI_TARGET_SDN, );
+if (sdn) {
+const char *olddn;
+olddn = slapi_sdn_get_ndn(sdn);
+slapi_log_error(SLAPI_LOG_PLUGIN, 
"backend_passwdmod_extop",
+  "olddn = %s (unknown expected)\n", 
olddn ? olddn : "" );

+slapi_sdn_free();
+}
+
+/* replace it with the one in the extop req*/
+sdn = slapi_sdn_new_dn_byref(extopdn);
+slapi_pblock_set(pb, 

Re: [Freeipa-devel] [PATCH 0041] Increase nsslapd-db-locks

[Martin Basti]

On 14.06.2016 16:27, Martin Basti wrote:



On 09.06.2016 12:42, Stanislav Laznicka wrote:

On 06/07/2016 08:56 AM, thierry bordaz wrote:



On 06/06/2016 07:23 PM, Martin Basti wrote:




On 03.06.2016 13:38, Stanislav Laznicka wrote:

Hello,

The attached patch implements solution to 
https://fedorahosted.org/freeipa/ticket/5914. The patch is rather 
hacky as nsslapd-db-locks requires to be modified when DS is not 
running although I accept proposals for better solution.


Standa

LGTM and works for me, but I want ack from thierry because of the 
value


* value set by this patch is 100K, it is okay or should be rather 
50K as is mentioned in the ticket
* should be upgrade for this change, or should be this done only 
for new installations? (patch solves new installations only and I 
don't think that we should change it by upgrade, users may have 
already tuned DS)


Martin^2

Hello,

This value also depends of the data. For example adding groups with 
10K members spikes the dblock consumption up to 40K because of 
membership computation during the update.
The size of the entries can also contribute if for example we have 
several entries per page or they are too large and fit on several 
pages.


IMHO we should support out of the box groups with 10K, so 
'nsslapd-db-locks: 5' looks good to me.
However it could be documented why it can consum so much lock and 
how to tune it.


thanks
thierry

Thank you, Thierry. Reduced the 100k locks to 50k locks in the patch 
(attached).


ACK, I will push it later



Pushed to master: 8e3b7b24c1bec5b7e2519e7c9466e3c336f9a409

--
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 0033] Fix CA being presented as running even if it weren't

[Martin Basti]

On 02.06.2016 19:21, Martin Basti wrote:



On 31.05.2016 16:32, Stanislav Laznicka wrote:

On 05/31/2016 11:40 AM, Stanislav Laznicka wrote:

On 05/31/2016 10:22 AM, Stanislav Laznicka wrote:

On 05/30/2016 12:54 PM, Jan Cholasta wrote:

On 30.5.2016 12:36, Martin Basti wrote:



On 26.05.2016 19:31, Stanislav Laznicka wrote:


Self NACK. I should not post patches when tired, sorry. Minor 
fix is

attached.


On 05/26/2016 07:21 PM, Stanislav Laznicka wrote:

Hello,

Please, see the attached patch. Fixes
https://fedorahosted.org/freeipa/ticket/5898

Standa





LGTM, if nobody is against this, I will push it in 2 days


NACK, please add `wait` argument and call 
self.wait_until_running(), same as in start() and restart().



A pretty good point, please see the modified patch.
Self.NACK - can't add 'wait' agrument to service.Service.is_running 
this easy.



Should be fixed now.



works for me, can be pushed if Honza agree

Martin^2


ACK

Pushed to master: fb4e19713d509a0a14acb7eee37f5fee7a9fb375

--
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] 0021 slapi-nis should allow password update on a virtual entry

[Alexander Bokovoy]

On Wed, 15 Jun 2016, thierry bordaz wrote:

From 6cd06b9004f8ab72e13c26742d11ee31d30bbc79 Mon Sep 17 00:00:00 2001

From: Thierry Bordaz 
Date: Mon, 13 Jun 2016 18:13:04 +0200
Subject: [PATCH] slapi-nis should allow password update on a virtual entry

During password modification ext. op (1.3.6.1.4.1.4203.1.11.1),
if the target entry is in the compat tree, slapi-nis should
remap the entry to the real entry.

This needs to be done in a pre-op extop that calls the callback
function handling a given OID.
The password mod. callback does a reverse mapping of
extop USERID and set it in SLAPI_TARGET_SDN.
---
configure.ac   |   1 +
src/back-sch.c | 217 +
src/back-sch.h |  16 +
src/plug-sch.c |  24 +++
4 files changed, 258 insertions(+)

diff --git a/configure.ac b/configure.ac
index 5b10376..9ce6bcf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,6 +113,7 @@ dirsrv)
SLAPI_PLUGIN_BE_TXN_POST_MODIFY_FN,
SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN,
SLAPI_PLUGIN_BE_TXN_POST_DELETE_FN,
+   SLAPI_PLUGIN_PRE_EXTOP_FN,
NULL]
   ,,,
   [AC_INCLUDES_DEFAULT
diff --git a/src/back-sch.c b/src/back-sch.c
index 32b1d9e..f9ab812 100644
--- a/src/back-sch.c
+++ b/src/back-sch.c
@@ -54,6 +54,8 @@
#include "map.h"
#include "back-sch.h"

+backend_extop_handlers_t extop_handlers[] = {{EXTOP_PASSWD_OID, (IFP) backend_passwdmod_extop}, 
+	{NULL, NULL}};

static void
backend_entries_to_return_push(struct backend_search_cbdata *cbdata, 
Slapi_Entry *e);

@@ -2223,6 +2225,203 @@ done_with_lock:
return ret;
}

+/* This callback handles EXTOP_PASSWD_OID "1.3.6.1.4.1.4203.1.11.1"
+ * If the extop defines a USERID, it sets SLAPI_TARGET_SDN to
+ * the reverse mapping of the USERID.
+ *
+ * If it is not possible to retrieve USERID in the ber
+ * then value of SLAPI_TARGET_SDN is unchanged.
+ *
+ * Else the value of SLAPI_TARGET_SDN is freed and replaced
+ * either by the USERID or the reverse mapping of USERID (if it exists)
+ */
+static int
+backend_passwdmod_extop(Slapi_PBlock *pb)
+{
+   struct backend_entry_data *data;
+   struct plugin_state *state;
+   Slapi_DN *sdn = NULL;
+   char *extopdn;
+   char *ndn;
+   char *username = NULL;
+   char *group = NULL;
+   const char *entry_group = NULL;
+   char *set = NULL;
+   const char *entry_set = NULL;
+   struct berval   *extop_value = NULL;
+   BerElement  *ber = NULL;
+   ber_tag_t   tag = 0;
+ber_len_t  len = (ber_len_t) -1;
+   
+   if (wrap_get_call_level() > 0) {
+   return 0;
+   }
+
+   slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, );
+   if (state->ready_to_serve == 0) {
+   /* No data to serve yet */
+   goto free_and_return;
+   }
+   /* Retrieve the original DN from the ber request */
+   /* Get the ber value of the extended operation */
+   slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, _value);
+   if (!BV_HAS_DATA(extop_value)) {
+   goto free_and_return;
+   }
+
+if ((ber = ber_init(extop_value)) == NULL) {
+   goto free_and_return;
+   }
+
+	/* Format of request to parse

+*
+* PasswdModifyRequestValue ::= SEQUENCE {
+* userIdentity[0]  OCTET STRING OPTIONAL
+* oldPasswd   [1]  OCTET STRING OPTIONAL
+* newPasswd   [2]  OCTET STRING OPTIONAL }
+*
+* The request value field is optional. If it is
+* provided, at least one field must be filled in.
+*/
+
+   /* ber parse code */
+   if ( ber_scanf( ber, "{") == LBER_ERROR ) {
+   /* The request field wasn't provided.  We'll
+* now try to determine the userid and verify
+* knowledge of the old password via other
+* means.
+*/
+   goto free_and_return;
+   } else {
+   tag = ber_peek_tag( ber, );
+   }
+
+   /* identify userID field by tags */
+   if (tag == LDAP_EXTOP_PASSMOD_TAG_USERID ) {
+
+   if ( ber_scanf( ber, "a", ) == LBER_ERROR ) {
+   slapi_ch_free_string();
+   goto free_and_return;
+   }
+
+slapi_log_error(SLAPI_LOG_PLUGIN, "backend_passwdmod_extop",
+   "extopdn = %s\n", extopdn ? extopdn : "" );
+   
+   /* Free the current target_DN */
+   slapi_pblock_get(pb, SLAPI_TARGET_SDN, );
+   if (sdn) {
+   const char *olddn;
+   olddn = slapi_sdn_get_ndn(sdn);
+   slapi_log_error(SLAPI_LOG_PLUGIN, 
"backend_passwdmod_extop",
+  

Re: [Freeipa-devel] [PATCH] 0021 slapi-nis should allow password update on a virtual entry

[Martin Basti]

On 15.06.2016 17:19, thierry bordaz wrote:

Hello,

This patch is for https://fedorahosted.org/freeipa/ticket/5955

Please put this link to commit message


This is the last patch related "IdM user password change support for 
legacy client compat tree"


  * It requires DS > 1.3.5.5 (https://fedorahosted.org/389/ticket/48880)

Please bump version in freeipa.spec.in and put DS srpms to 
@freeipa/freeipa-master if new DS is not at least in updates testing




 *



  * PATCH 0020 https://fedorahosted.org/freeipa/ticket/5946 ipapwd
(review by Alexander)
  * this PATCH 0021

This patch is not the final one because I had to locally define 
SLAPI_PLUGIN_PRE_EXTOP_FN in order to build on copr.

The define SLAPI_PLUGIN_PRE_EXTOP_FN comes with DS > 1.3.5.5

A test case is:

create a user 'tb1'

# step 1 verify that there is no passwd/krbkeys
ldapsearch -LLL -D "cn=directory manager" -w xxx -b
"uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey


# step 2 verify that tb1 has a password/krbkeys
ldappasswd -D "cn=directory manager" -w xxx
"uid=tb1,cn=users,cn=*accounts*,SUFFIX" -s yyy
ldapsearch -LLL -D "cn=directory manager" -w xxx -b
"uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey

# step 3 verify that tb1 has different passwd/krbkeys than in step 2
ldappasswd -D "cn=directory manager" -w xxx
"uid=tb1,cn=users,cn=*accounts*,SUFFIX" -s yyy
ldapsearch -LLL -D "cn=directory manager" -w xxx -b
"uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey


# step 4 verify that tb1 has different passwd/krbkeys than in step 3
ldappasswd -D "cn=directory manager" -w xxx
"uid=tb1,cn=users,cn=*compat*,SUFFIX" -s yyy
ldapsearch -LLL -D "cn=directory manager" -w xxx -b
"uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey

# step 5 verify that tb1 has different passwd/krbkeys than in step 4
ldappasswd -D "cn=directory manager" -w xxx
"uid=tb1,cn=users,cn=*compat*,SUFFIX" -s yyy
ldapsearch -LLL -D "cn=directory manager" -w xxx -b
"uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey


Please put these steps to reproduce into ticket, we will need this for QA.


thanks
thierry




Thank you,
Martin^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 0503-0513, 0515-0519] DNS locations

[Petr Spacek]

On 15.6.2016 15:45, Martin Basti wrote:
> 
> 
> On 15.06.2016 14:52, Martin Basti wrote:
>> 
>> Hydra patching: Updated patches attached + new patches for dnsserver-*
>> commands attached
>>>
>>>
>> Updated+rebased patches after Honza's interactive review
>>
>>
> Minor nitpick fixed
> 
> 
> 
freeipa-mbasti-0503.3-DNS-Locations-add-index-for-ipalocation-attribute.patch
ACK

freeipa-mbasti-0505.3-DNS-Locations-add-idnsTemplateObject-objectclass.patch
ACK


I will get to the rest later on.

-- 
Petr^2 Spacek

-- 
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] 0068 upgrade: do not try to start CA if not configured

[Martin Basti]

On 15.06.2016 17:19, Martin Basti wrote:



On 15.06.2016 17:17, Martin Basti wrote:



On 15.06.2016 16:41, Petr Spacek wrote:

On 15.6.2016 14:18, Fraser Tweedale wrote:

Attached patch fixes https://fedorahosted.org/freeipa/ticket/5958.
The regression was introduced in fix for
https://fedorahosted.org/freeipa/ticket/5868.

It works for me, ACK.


Pushed to master: 01795fca831ca5ce59a56755d261cb3100d6c3d4

If commit that causes regression was pushed to 4.3.2, shouldn't go 
this to 4.3.2 too?



Should, pushed to ipa-4-3:
* 7514b8b6df46b57ec9353a02d071347c931ea387 upgrade: do not try to start 
CA if not configured


--
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] 0021 slapi-nis should allow password update on a virtual entry

[thierry bordaz]

Hello,

This patch is for https://fedorahosted.org/freeipa/ticket/5955

This is the last patch related "IdM user password change support for 
legacy client compat tree"


 * It requires DS > 1.3.5.5 (https://fedorahosted.org/389/ticket/48880)
 * PATCH 0020 https://fedorahosted.org/freeipa/ticket/5946 ipapwd
   (review by Alexander)
 * this PATCH 0021

This patch is not the final one because I had to locally define 
SLAPI_PLUGIN_PRE_EXTOP_FN in order to build on copr.

The define SLAPI_PLUGIN_PRE_EXTOP_FN comes with DS > 1.3.5.5

A test case is:

   create a user 'tb1'

   # step 1 verify that there is no passwd/krbkeys
   ldapsearch -LLL -D "cn=directory manager" -w xxx -b
   "uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey


   # step 2 verify that tb1 has a password/krbkeys
   ldappasswd -D "cn=directory manager" -w xxx
   "uid=tb1,cn=users,cn=*accounts*,SUFFIX" -s yyy
   ldapsearch -LLL -D "cn=directory manager" -w xxx -b
   "uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey

   # step 3 verify that tb1 has different passwd/krbkeys than in step 2
   ldappasswd -D "cn=directory manager" -w xxx
   "uid=tb1,cn=users,cn=*accounts*,SUFFIX" -s yyy
   ldapsearch -LLL -D "cn=directory manager" -w xxx -b
   "uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey


   # step 4 verify that tb1 has different passwd/krbkeys than in step 3
   ldappasswd -D "cn=directory manager" -w xxx
   "uid=tb1,cn=users,cn=*compat*,SUFFIX" -s yyy
   ldapsearch -LLL -D "cn=directory manager" -w xxx -b
   "uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey

   # step 5 verify that tb1 has different passwd/krbkeys than in step 4
   ldappasswd -D "cn=directory manager" -w xxx
   "uid=tb1,cn=users,cn=*compat*,SUFFIX" -s yyy
   ldapsearch -LLL -D "cn=directory manager" -w xxx -b
   "uid=tb1,cn=users,cn=accounts,SUFFIX" userPassword krbPrincipalKey

thanks
thierry

>From 6cd06b9004f8ab72e13c26742d11ee31d30bbc79 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz 
Date: Mon, 13 Jun 2016 18:13:04 +0200
Subject: [PATCH] slapi-nis should allow password update on a virtual entry

During password modification ext. op (1.3.6.1.4.1.4203.1.11.1),
if the target entry is in the compat tree, slapi-nis should
remap the entry to the real entry.

This needs to be done in a pre-op extop that calls the callback
function handling a given OID.
The password mod. callback does a reverse mapping of
extop USERID and set it in SLAPI_TARGET_SDN.
---
 configure.ac   |   1 +
 src/back-sch.c | 217 +
 src/back-sch.h |  16 +
 src/plug-sch.c |  24 +++
 4 files changed, 258 insertions(+)

diff --git a/configure.ac b/configure.ac
index 5b10376..9ce6bcf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -113,6 +113,7 @@ dirsrv)
 			SLAPI_PLUGIN_BE_TXN_POST_MODIFY_FN,
 			SLAPI_PLUGIN_BE_TXN_POST_MODRDN_FN,
 			SLAPI_PLUGIN_BE_TXN_POST_DELETE_FN,
+			SLAPI_PLUGIN_PRE_EXTOP_FN,
 			NULL]
 		   ,,,
 		   [AC_INCLUDES_DEFAULT
diff --git a/src/back-sch.c b/src/back-sch.c
index 32b1d9e..f9ab812 100644
--- a/src/back-sch.c
+++ b/src/back-sch.c
@@ -54,6 +54,8 @@
 #include "map.h"
 #include "back-sch.h"
 
+backend_extop_handlers_t extop_handlers[] = {{EXTOP_PASSWD_OID, (IFP) backend_passwdmod_extop}, 
+	{NULL, NULL}};
 static void
 backend_entries_to_return_push(struct backend_search_cbdata *cbdata, Slapi_Entry *e);
 
@@ -2223,6 +2225,203 @@ done_with_lock:
 	return ret;
 }
 
+/* This callback handles EXTOP_PASSWD_OID "1.3.6.1.4.1.4203.1.11.1"
+ * If the extop defines a USERID, it sets SLAPI_TARGET_SDN to
+ * the reverse mapping of the USERID.
+ *
+ * If it is not possible to retrieve USERID in the ber
+ * then value of SLAPI_TARGET_SDN is unchanged.
+ *
+ * Else the value of SLAPI_TARGET_SDN is freed and replaced
+ * either by the USERID or the reverse mapping of USERID (if it exists)
+ */
+static int
+backend_passwdmod_extop(Slapi_PBlock *pb)
+{
+	struct backend_entry_data *data;
+	struct plugin_state *state;
+	Slapi_DN *sdn = NULL;
+	char *extopdn;
+	char *ndn;
+	char *username = NULL;
+	char *group = NULL;
+	const char *entry_group = NULL;
+	char *set = NULL;
+	const char *entry_set = NULL;
+	struct berval	*extop_value = NULL;
+	BerElement	*ber = NULL;
+	ber_tag_t	tag = 0;
+ber_len_t	len = (ber_len_t) -1;
+	
+	if (wrap_get_call_level() > 0) {
+		return 0;
+	}
+
+	slapi_pblock_get(pb, SLAPI_PLUGIN_PRIVATE, );
+	if (state->ready_to_serve == 0) {
+		/* No data to serve yet */
+		goto free_and_return;
+	}
+	/* Retrieve the original DN from the ber request */
+	/* Get the ber value of the extended operation */
+	slapi_pblock_get(pb, SLAPI_EXT_OP_REQ_VALUE, _value);
+	if (!BV_HAS_DATA(extop_value)) {
+		goto free_and_return;
+	}
+
+if ((ber = ber_init(extop_value)) == NULL) {
+		goto free_and_return;
+	}
+
+	/* Format of request to parse
+	 *
+	 * PasswdModifyRequestValue ::= SEQUENCE {
+	 * userIdentity

Re: [Freeipa-devel] [PATCH] 0068 upgrade: do not try to start CA if not configured

[Martin Basti]

On 15.06.2016 17:17, Martin Basti wrote:



On 15.06.2016 16:41, Petr Spacek wrote:

On 15.6.2016 14:18, Fraser Tweedale wrote:

Attached patch fixes https://fedorahosted.org/freeipa/ticket/5958.
The regression was introduced in fix for
https://fedorahosted.org/freeipa/ticket/5868.

It works for me, ACK.


Pushed to master: 01795fca831ca5ce59a56755d261cb3100d6c3d4

If commit that causes regression was pushed to 4.3.2, shouldn't go this 
to 4.3.2 too?


--
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] 0068 upgrade: do not try to start CA if not configured

[Martin Basti]

On 15.06.2016 16:41, Petr Spacek wrote:

On 15.6.2016 14:18, Fraser Tweedale wrote:

Attached patch fixes https://fedorahosted.org/freeipa/ticket/5958.
The regression was introduced in fix for
https://fedorahosted.org/freeipa/ticket/5868.

It works for me, ACK.


Pushed to master: 01795fca831ca5ce59a56755d261cb3100d6c3d4

--
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] [freeipa-devel][PATCH] Added missing translation to automount.py method

[Martin Basti]

On 15.06.2016 11:13, Abhijeet Kasurde wrote:

Hi All,

Please review the attached patch.

Fixes: https://fedorahosted.org/freeipa/ticket/5920





Thank you for the patch,

Please follow this page for howto create internationalized strings:
http://www.freeipa.org/page/Python_Coding_Style#Python_Internationalized_.28i18n.29_Strings

you must use named subsitution, because translators may need to know 
what should be there, or they may change the order of words depending on 
translated language


Martin^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

[Freeipa-devel] provisioning and RetroCL/Content_Sync

[thierry bordaz]

Hello,

   The subject of provisioning was discussed
   https://www.redhat.com/archives/freeipa-devel/2016-May/msg00065.html. The
   documentation of the provisioning procedure is still going on but
   reviewing it I have a doubt about RetroCL/Content_Sync.

   Provisioning will be done with high recommendations/constraints:

 * The provisioned instance should not be accessed by ldap client
   during provisioning.
 * The IPA deployment should contain only one server (the one used
   for provisioning) in order to prevent replication latency

   During provisioning, disabling RetroCL/Content_Sync gives a ~10%
   improvements (reducing the #ADD).

   The drawback of disabling RetroCL/Content_Sync is that the
   provisioned instance will not be able to send provisioned entries
   through syncRepl.
   Now considering that the provisioned instance is unique in the
   topology and will do full init of replicas, I think SyncRepl is
   useless and then we can disable RetroCL/Content_Sync during
   provisioning.

   Anyone is seeing a problem if those plugins are disabled during
   provisioning ?

   thanks
   thierry

-- 
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] 0053: webui: allow to set weight of server without location

[Pavel Vomacka]

Hello,

I've found a small bug in locations in WebUI. It is not allowed to set 
weight of a server with no location (i.e. adding new server). Attached 
patch allows that.


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

--
Pavel^3 Vomacka
From 1fc29c625bfcd5fc4a3eb5b6293986d7f9bacb2f Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Fri, 10 Jun 2016 09:39:36 +0200
Subject: [PATCH] Allow to set weight of a server without location

There was a bug when a new server was added it was not possible to set weight until
a location was set. This change corrects it and allows user to set a weight of server
without location.

Part of: https://fedorahosted.org/freeipa/ticket/5905
---
 install/ui/src/freeipa/topology.js | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/topology.js b/install/ui/src/freeipa/topology.js
index 139f9562506f338b32dc3afa56493ecb2c4a1cfc..53a7c70dd430cd383e8cefc829bd31c90971a27f 100644
--- a/install/ui/src/freeipa/topology.js
+++ b/install/ui/src/freeipa/topology.js
@@ -240,7 +240,8 @@ return {
 },
 {
 name: 'ipalocationweight',
-placeholder: '100'
+placeholder: '100',
+flags: ['w_if_no_aci']
 },
 {
 $type: 'association_table',
-- 
2.5.5

-- 
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] Schema caching for thin client

[David Kupka]

Hello!
Schema caching for thin client is available here:

https://github.com/dkupka/freeipa/commits/schema_cache

Comments and reviews welcome.

Enjoy!
--
David Kupka

--
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 0153-0158] move ipa-replica-manage del functionality into server-del

[Martin Babinsky]

On 06/15/2016 10:30 AM, Jan Cholasta wrote:

Hi,

On 12.6.2016 17:31, Martin Babinsky wrote:

On 06/09/2016 08:12 PM, Martin Babinsky wrote:

These patches expand `server_del` to a full fledged IPA master killer in
domain level 1.

Due to 'server uninstallation removed master from topology' use case,
the individual steps are not in the same order as in the original code
to facilitate self-removal from topology without introducing an array of
permissions for master to remove itself.

I had no opportunity to test out the CI test suite because of technical
problems so it would be nice if our upstream QE could give it a spin and
report errors.

http://www.freeipa.org/page/V4/Manage_replication_topology_4_4
https://fedorahosted.org/freeipa/ticket/5181




Attaching rebased patches and bumping for review.

Please note that they depend on 'Server Roles v2' patchset.


Patch 0153:

Should be an ipaserver module, unless it is required on clients as well,
in which case it should be an ipalib module.


Patch 0154: LGTM


Patch 0155:

In LDAPDelete subclasses, the primary key argument is multivalue, so I'm
guessing your post_callback won't work correctly.

Also, since this is *server*-del, s/master/server/ where applicable.


Patch 0156: LGTM


Patch 0157:

This looks suspicious:

+result = server_del_cmd(hostname, version=api_version, **options)

Version is automatically filled in in Command.__call__(), why do you add
it manually here?



Because this uses server API which calls command's `execute` directly so 
it does not pass version automatically.


So you get version mismatch warning every time this function is called.



Patch 0158: LGTM


Honza




--
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

[Freeipa-devel] [PATCH] 0068 upgrade: do not try to start CA if not configured

[Fraser Tweedale]

Attached patch fixes https://fedorahosted.org/freeipa/ticket/5958.
The regression was introduced in fix for
https://fedorahosted.org/freeipa/ticket/5868.

Thanks,
Fraser
From 9c6c1e2fb18f87b9e7e64756fd69e10b5949fb82 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 15 Jun 2016 22:12:52 +1000
Subject: [PATCH] upgrade: do not try to start CA if not configured

The upgrade script always attempts to start the CA, even on
instances where the CA is not configured.  Add guards.

Fixes: https://fedorahosted.org/freeipa/ticket/5958
---
 ipaserver/install/server/upgrade.py | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/ipaserver/install/server/upgrade.py 
b/ipaserver/install/server/upgrade.py
index 
0c5f32d954bb236b11eb3cfcda95e66c0c7913d5..044e36494df95f6624ce639ad6fda68025078aea
 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -1552,10 +1552,11 @@ def upgrade_configuration():
 )
 upgrade_pki(ca, fstore)
 
-# several upgrade steps require running CA
+# several upgrade steps require running CA.  If CA is configured,
 # always run ca.start() because we need to wait until CA is really ready
 # by checking status using http
-ca.start('pki-tomcat')
+if ca.is_configured():
+ca.start('pki-tomcat')
 
 certmonger_service = services.knownservices.certmonger
 if ca.is_configured() and not certmonger_service.is_running():
@@ -1736,10 +1737,11 @@ def upgrade_configuration():
 elif not ds_running and ds.is_running():
 ds.stop(ds_serverid)
 
-if ca_running and not ca.is_running():
-ca.start('pki-tomcat')
-elif not ca_running and ca.is_running():
-ca.stop('pki-tomcat')
+if ca.is_configured():
+if ca_running and not ca.is_running():
+ca.start('pki-tomcat')
+elif not ca_running and ca.is_running():
+ca.stop('pki-tomcat')
 
 
 def upgrade_check(options):
-- 
2.5.5

-- 
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] Using JSON for tlog config files

[Petr Spacek]

On 15.6.2016 13:52, Nikolai Kondrashov wrote:
> On 06/15/2016 02:41 PM, Martin Kosek wrote:
>> Removing the secondary list from this discussion.
>>
>> On 06/15/2016 01:29 PM, Nikolai Kondrashov wrote:
>>> Hi Simo,
>>>
>>> On 06/15/2016 12:25 AM, Simo Sorce wrote:
 On Tue, 2016-06-14 at 16:40 +0300, Nikolai Kondrashov wrote:
> Although this was mentioned several times before, I'd like to bring
> additional
> attention to the idea of using config files written in JSON for tlog,
> because
> there were some concerns over that being appropriate.

 What was the reasoning behind this questioning ?
>>>
>>> The concern that JSON in those files might be inconvenient for 
>>> administrators.
>>>
 In order to understand whether json is appropriate I need to ask who do
 you think will be the primary user of the configuration file.
 - Is it going to be mainly sysadmins manually setting things up ?
>>>
>>> At first, yes. Later minimally, because we plan to implement central control
>>> of most (if not all) of the parameters.
>>
>> Yes, but AFAIK, tlog will still also exist as standalone component and could 
>> be
>> configured without integration with FreeIPA. This means admins main
>> configuration file would still be the config file.
> 
> Yes, that's true.
> 
> It wouldn't make much sense for small setups to use tlog, though, because it
> implies ElasticSearch. At least Ansible or something like that will need to be
> used for bigger setups, if not FreeIPA/SSSD, but yes, in that case it will
> still be JSON.

I would say that properly indented JSON with extensive comments and examples
is good enough.

-- 
Petr^2 Spacek

-- 
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] [WIP] Thin client

[Jan Cholasta]

On 15.6.2016 13:56, David Kupka wrote:

On 06/15/2016 01:33 PM, David Kupka wrote:

On 04/28/2016 02:45 PM, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Next batch:

schema: exclude local commands
frontend: call `execute` rather than `forward` in Local
dns, passwd: fix output validation error
misc: fix CLI output of `env` and `plugins` commands
ipalib: replace Any with Dict
schema: generate client-side commands on demand
plugable: initialize plugins on demand
plugable: allow plugins to be non-classes

There is known regression that arguments with dynamic defaults are not
filled automatically. This will be fixed soon.

Otherwise works for me, ACK.



Fix for dynamic defaults regression is:

schema: fix client-side dynamic defaults

Works for me, ACK.


Pushed to master: d26e42ffb065cc524c63d65d87c2a2b5d943c54a

Attaching the patches for reference.

--
Jan Cholasta
From 108e818f772318b1dd0c6cc32f66750d0091b560 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 14 Jun 2016 13:02:30 +0200
Subject: [PATCH 1/9] plugable: allow plugins to be non-classes

Allow registering any object that is callable and has `name` and `bases`
attributes as a plugin.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipalib/plugable.py | 45 +++--
 ipalib/util.py | 26 ++
 2 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/ipalib/plugable.py b/ipalib/plugable.py
index 497b545..d8ff5c1 100644
--- a/ipalib/plugable.py
+++ b/ipalib/plugable.py
@@ -26,7 +26,6 @@ http://docs.python.org/ref/sequence-types.html
 """
 
 import sys
-import inspect
 import threading
 import os
 from os import path
@@ -40,6 +39,7 @@ import six
 from ipalib import errors
 from ipalib.config import Env
 from ipalib.text import _
+from ipalib.util import classproperty
 from ipalib.base import ReadOnly, NameSpace, lock, islocked
 from ipalib.constants import DEFAULT_CONFIG
 from ipapython.ipa_log_manager import (
@@ -101,8 +101,8 @@ class Registry(object):
 
 :param klass: A subclass of `Plugin` to attempt to register.
 """
-if not inspect.isclass(klass):
-raise TypeError('plugin must be a class; got %r' % klass)
+if not callable(klass):
+raise TypeError('plugin must be callable; got %r' % klass)
 
 # Raise DuplicateError if this exact class was already registered:
 if klass in self.__registry:
@@ -134,9 +134,18 @@ class Plugin(ReadOnly):
 self.__finalize_lock = threading.RLock()
 log_mgr.get_logger(self, True)
 
-@property
-def name(self):
-return type(self).__name__
+@classmethod
+def __name_getter(cls):
+return cls.__name__
+
+# you know nothing, pylint
+name = classproperty(__name_getter)
+
+@classmethod
+def __bases_getter(cls):
+return cls.__bases__
+
+bases = classproperty(__bases_getter)
 
 @property
 def doc(self):
@@ -571,12 +580,12 @@ class API(ReadOnly):
 :param klass: A subclass of `Plugin` to attempt to add.
 :param override: If true, override an already added plugin.
 """
-if not inspect.isclass(klass):
-raise TypeError('plugin must be a class; got %r' % klass)
+if not callable(klass):
+raise TypeError('plugin must be callable; got %r' % klass)
 
 # Find the base class or raise SubclassError:
-for base in self.bases:
-if issubclass(klass, self.bases):
+for base in klass.bases:
+if issubclass(base, self.bases):
 break
 else:
 raise errors.PluginSubclassError(
@@ -585,13 +594,13 @@ class API(ReadOnly):
 )
 
 # Check override:
-prev = self.__plugins.get(klass.__name__)
+prev = self.__plugins.get(klass.name)
 if prev:
 if not override:
 # Must use override=True to override:
 raise errors.PluginOverrideError(
 base=base.__name__,
-name=klass.__name__,
+name=klass.name,
 plugin=klass,
 )
 
@@ -601,12 +610,12 @@ class API(ReadOnly):
 # There was nothing already registered to override:
 raise errors.PluginMissingOverrideError(
 base=base.__name__,
-name=klass.__name__,
+name=klass.name,
 plugin=klass,
 )
 
 # The plugin is okay, add to sub_d:
-self.__plugins[klass.__name__] = klass
+self.__plugins[klass.name] = 

Re: [Freeipa-devel] [WIP] Thin client

[David Kupka]

On 06/15/2016 01:33 PM, David Kupka wrote:

On 04/28/2016 02:45 PM, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Next batch:

schema: exclude local commands
frontend: call `execute` rather than `forward` in Local
dns, passwd: fix output validation error
misc: fix CLI output of `env` and `plugins` commands
ipalib: replace Any with Dict
schema: generate client-side commands on demand
plugable: initialize plugins on demand
plugable: allow plugins to be non-classes

There is known regression that arguments with dynamic defaults are not
filled automatically. This will be fixed soon.

Otherwise works for me, ACK.



Fix for dynamic defaults regression is:

schema: fix client-side dynamic defaults

Works for me, ACK.

--
David Kupka

--
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 0159-0160] emancipate IPA NTP service into role

[Martin Basti]

On 15.06.2016 13:29, Petr Spacek wrote:

On 15.6.2016 09:57, Martin Basti wrote:


On 15.06.2016 09:55, Petr Vobornik wrote:

On 06/14/2016 07:28 PM, Martin Basti wrote:

On 14.06.2016 18:58, Martin Babinsky wrote:

On 06/14/2016 05:06 PM, Martin Basti wrote:

On 12.06.2016 17:37, Martin Babinsky wrote:

These two patches turn oft-neglected ntp service into a full fledged
role whose status can be queried centrally. They should also enable
generation of location-specific _ntp._udp records.

Please note that NTP is LDAP-enabled by additional call after DS
instance is configured. I was not feeling confident by swapping NTP
and DS configuration steps as I was afraid it will break things. If
not, I will happily update the patch accordingly.

https://fedorahosted.org/freeipa/ticket/5815
https://fedorahosted.org/freeipa/ticket/5826




Hello, I have a few comments:

Patch: 159
1)
+if ntp.is_configured():
+ntp.ldap_enable('NTP', fqdn, None, base_dn)
+ntp.enable()

All ipa services are in disabled state, ipactl starts them according
configuration in LDAP
IMO it should be something like:
ntp.disable()
if running:
  ntp.start()

2)
could you upgrade NTP only once in upgrade.py? Use sysupgrade state

3)
+'NTP': ('ntpd', 42),
I prefer 45, it is easier to put any service before NTP if needed
without huge renumbering


Patch 160: LGTM

Martin^2



Right, attaching updated patches.


Patches are good, but I'm curious if there is any chance for NTP to be
able synchronize time before replication on replica install. If no, IMO
better is to move NTP service configuration after dirserver to be able
to configure LDAP entry directly.
But if there is not time for this, I'm fine with opening ticket and
fixing it later.

Martin^2

Isn't it already done during Client part of replica installation?


With domain level 1 yes, with domain level 0, no.

ACK to current version of the patch.

Please keep the order as it is (NTP first) because it ensures proper time sync
even on domain level 0.


master:
* 567f00a59c53aca760336aea95423368ac621032 Add NTP to the list of 
services stored in IPA masters LDAP subtree

* 3e6af238bb695572e462ff49a3096ab0e2e85bc5 Introduce "NTP server" role

--
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] Using JSON for tlog config files

[Nikolai Kondrashov]

On 06/15/2016 02:41 PM, Martin Kosek wrote:

Removing the secondary list from this discussion.

On 06/15/2016 01:29 PM, Nikolai Kondrashov wrote:

Hi Simo,

On 06/15/2016 12:25 AM, Simo Sorce wrote:

On Tue, 2016-06-14 at 16:40 +0300, Nikolai Kondrashov wrote:

Although this was mentioned several times before, I'd like to bring additional
attention to the idea of using config files written in JSON for tlog, because
there were some concerns over that being appropriate.


What was the reasoning behind this questioning ?


The concern that JSON in those files might be inconvenient for administrators.


In order to understand whether json is appropriate I need to ask who do
you think will be the primary user of the configuration file.
- Is it going to be mainly sysadmins manually setting things up ?


At first, yes. Later minimally, because we plan to implement central control
of most (if not all) of the parameters.


Yes, but AFAIK, tlog will still also exist as standalone component and could be
configured without integration with FreeIPA. This means admins main
configuration file would still be the config file.


Yes, that's true.

It wouldn't make much sense for small setups to use tlog, though, because it
implies ElasticSearch. At least Ansible or something like that will need to be
used for bigger setups, if not FreeIPA/SSSD, but yes, in that case it will
still be JSON.

Nick

--
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] Using JSON for tlog config files

[Martin Kosek]

Removing the secondary list from this discussion.

On 06/15/2016 01:29 PM, Nikolai Kondrashov wrote:
> Hi Simo,
> 
> On 06/15/2016 12:25 AM, Simo Sorce wrote:
>> On Tue, 2016-06-14 at 16:40 +0300, Nikolai Kondrashov wrote:
>>> Although this was mentioned several times before, I'd like to bring 
>>> additional
>>> attention to the idea of using config files written in JSON for tlog, 
>>> because
>>> there were some concerns over that being appropriate.
>>
>> What was the reasoning behind this questioning ?
> 
> The concern that JSON in those files might be inconvenient for administrators.
> 
>> In order to understand whether json is appropriate I need to ask who do
>> you think will be the primary user of the configuration file.
>> - Is it going to be mainly sysadmins manually setting things up ?
> 
> At first, yes. Later minimally, because we plan to implement central control
> of most (if not all) of the parameters.

Yes, but AFAIK, tlog will still also exist as standalone component and could be
configured without integration with FreeIPA. This means admins main
configuration file would still be the config file.

> The suggestions so far were to have
> the same JSON simply stored in HBAC-bound LDAP entries verbatim. Although, I'm
> not sure if that would be best.
> 
>> - Is this going to be something that may need to be templated and
>> delivered via things like puppet ansible ?
> 
> I didn't plan specifically for that. I expect the chances of that are about
> the same as for any other config file.
> 
>> - Is it going to be a file generated by some other program (like sssd or
>> similar) based on rules stored in a central system ?
> 
> I'd like to avoid requiring any program controlling tlog to write any files.
> That's why I implemented passing the whole or part of the configuration via an
> environment variable value, overriding the global config. However, yes, that
> value is expected to be the same JSON at the moment.
> 
>> The answer to these questions will help understanding what format is
>> better suited.
> 
> Thanks, Simo!
> 
> Nick
> 

-- 
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] [WIP] Thin client

[David Kupka]

On 04/28/2016 02:45 PM, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza



Hello!

Next batch:

schema: exclude local commands
frontend: call `execute` rather than `forward` in Local
dns, passwd: fix output validation error
misc: fix CLI output of `env` and `plugins` commands
ipalib: replace Any with Dict
schema: generate client-side commands on demand
plugable: initialize plugins on demand
plugable: allow plugins to be non-classes

There is known regression that arguments with dynamic defaults are not 
filled automatically. This will be fixed soon.


Otherwise works for me, ACK.

--
David Kupka

--
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] Using JSON for tlog config files

[Nikolai Kondrashov]

Hi Simo,

On 06/15/2016 12:25 AM, Simo Sorce wrote:

On Tue, 2016-06-14 at 16:40 +0300, Nikolai Kondrashov wrote:

Although this was mentioned several times before, I'd like to bring additional
attention to the idea of using config files written in JSON for tlog, because
there were some concerns over that being appropriate.


What was the reasoning behind this questioning ?


The concern that JSON in those files might be inconvenient for administrators.


In order to understand whether json is appropriate I need to ask who do
you think will be the primary user of the configuration file.
- Is it going to be mainly sysadmins manually setting things up ?


At first, yes. Later minimally, because we plan to implement central control
of most (if not all) of the parameters. The suggestions so far were to have
the same JSON simply stored in HBAC-bound LDAP entries verbatim. Although, I'm
not sure if that would be best.


- Is this going to be something that may need to be templated and
delivered via things like puppet ansible ?


I didn't plan specifically for that. I expect the chances of that are about
the same as for any other config file.


- Is it going to be a file generated by some other program (like sssd or
similar) based on rules stored in a central system ?


I'd like to avoid requiring any program controlling tlog to write any files.
That's why I implemented passing the whole or part of the configuration via an
environment variable value, overriding the global config. However, yes, that
value is expected to be the same JSON at the moment.


The answer to these questions will help understanding what format is
better suited.


Thanks, Simo!

Nick

--
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 0159-0160] emancipate IPA NTP service into role

[Petr Spacek]

On 15.6.2016 09:57, Martin Basti wrote:
> 
> 
> On 15.06.2016 09:55, Petr Vobornik wrote:
>> On 06/14/2016 07:28 PM, Martin Basti wrote:
>>>
>>> On 14.06.2016 18:58, Martin Babinsky wrote:
 On 06/14/2016 05:06 PM, Martin Basti wrote:
>
> On 12.06.2016 17:37, Martin Babinsky wrote:
>> These two patches turn oft-neglected ntp service into a full fledged
>> role whose status can be queried centrally. They should also enable
>> generation of location-specific _ntp._udp records.
>>
>> Please note that NTP is LDAP-enabled by additional call after DS
>> instance is configured. I was not feeling confident by swapping NTP
>> and DS configuration steps as I was afraid it will break things. If
>> not, I will happily update the patch accordingly.
>>
>> https://fedorahosted.org/freeipa/ticket/5815
>> https://fedorahosted.org/freeipa/ticket/5826
>>
>>
>>
> Hello, I have a few comments:
>
> Patch: 159
> 1)
> +if ntp.is_configured():
> +ntp.ldap_enable('NTP', fqdn, None, base_dn)
> +ntp.enable()
>
> All ipa services are in disabled state, ipactl starts them according
> configuration in LDAP
> IMO it should be something like:
> ntp.disable()
> if running:
>  ntp.start()
>
> 2)
> could you upgrade NTP only once in upgrade.py? Use sysupgrade state
>
> 3)
> +'NTP': ('ntpd', 42),
> I prefer 45, it is easier to put any service before NTP if needed
> without huge renumbering
>
>
> Patch 160: LGTM
>
> Martin^2
>
>
 Right, attaching updated patches.

>>> Patches are good, but I'm curious if there is any chance for NTP to be
>>> able synchronize time before replication on replica install. If no, IMO
>>> better is to move NTP service configuration after dirserver to be able
>>> to configure LDAP entry directly.
>>> But if there is not time for this, I'm fine with opening ticket and
>>> fixing it later.
>>>
>>> Martin^2
>> Isn't it already done during Client part of replica installation?
>>
> With domain level 1 yes, with domain level 0, no.

ACK to current version of the patch.

Please keep the order as it is (NTP first) because it ensures proper time sync
even on domain level 0.

-- 
Petr^2 Spacek

-- 
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 551-552, 623-624] cert: add owner information, allow search by certificate

[Jan Cholasta]

On 14.6.2016 11:44, Jan Cholasta wrote:

On 21.4.2016 09:11, Jan Cholasta wrote:

On 6.4.2016 15:46, Pavel Vomacka wrote:



On 03/16/2016 01:50 PM, Jan Cholasta wrote:

Hi,

the attached patches implement the server-side part of
.

Honza


Hi,

thank you for the patches. I tested them and they work well. But I would
like to ask you whether would be possible to extend the response of
'basecert_find' method and probably also 'basecert_show' response. I
think of these information:

1) information whether the certificate is issued by our CA or not.


You can check for that by comparing the issuer name of the certificate
to "CN=Certificate Authority,$SUBJECT_BASE". You can get subject base
from config-show.



2) this probably wouldn't be possible (as we discussed), but I rather
write it too - the information about revocation reason. The same as the
'cert_show' provides.


Added --check-revocation flag to request this information. Currently it
works only on certificates issued by our CA.



3) MD5 and SHA1 fingerprints as the 'cert_show' method returns


Added, also included SHA-256.



Thank you again.


Updated patches attached.


Updated and rebased patches attached. Requires Fraser's sub-CA patches.


Attaching updated patch 623, which fixes these issues found by David: 
.


--
Jan Cholasta
From 4afb5a1bee3e2d152052d6ed924cafe4c1c51187 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Tue, 14 Jun 2016 09:09:10 +0200
Subject: [PATCH 3/4] cert: add owner information

Get owner information from LDAP in cert-show and cert-find. Allow search by
owner in cert-find.

https://fedorahosted.org/freeipa/ticket/5381
---
 API.txt   |  15 ++-
 VERSION   |   4 +-
 ipaserver/plugins/cert.py | 267 --
 3 files changed, 251 insertions(+), 35 deletions(-)

diff --git a/API.txt b/API.txt
index f01d3c1..94bf337 100644
--- a/API.txt
+++ b/API.txt
@@ -730,23 +730,33 @@ output: Entry('result')
 output: Output('summary', type=[, ])
 output: PrimaryKey('value')
 command: cert_find
-args: 1,20,4
+args: 1,30,4
 arg: Str('criteria?')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('cacn?', cli_name='ca')
 option: Flag('exactly?', autofill=True, default=False)
+option: Str('host*', cli_name='hosts')
+option: Str('idoverrideuser*', cli_name='idoverrideusers')
 option: DateTime('issuedon_from?', autofill=False)
 option: DateTime('issuedon_to?', autofill=False)
 option: DNParam('issuer?', autofill=False)
 option: Int('max_serial_number?', autofill=False)
 option: Int('min_serial_number?', autofill=False)
+option: Str('no_host*', cli_name='no_hosts')
+option: Str('no_idoverrideuser*', cli_name='no_idoverrideusers')
+option: Flag('no_members', autofill=True, default=True)
+option: Str('no_service*', cli_name='no_services')
+option: Str('no_user*', cli_name='no_users')
 option: Flag('pkey_only?', autofill=True, default=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Int('revocation_reason?', autofill=False, default=0)
 option: DateTime('revokedon_from?', autofill=False)
 option: DateTime('revokedon_to?', autofill=False)
+option: Str('service*', cli_name='services')
 option: Int('sizelimit?')
 option: Str('subject?', autofill=False)
+option: Int('timelimit?')
+option: Str('user*', cli_name='users')
 option: DateTime('validnotafter_from?', autofill=False)
 option: DateTime('validnotafter_to?', autofill=False)
 option: DateTime('validnotbefore_from?', autofill=False)
@@ -782,10 +792,11 @@ option: Int('revocation_reason', autofill=True, default=0)
 option: Str('version?')
 output: Output('result')
 command: cert_show
-args: 1,5,3
+args: 1,6,3
 arg: Int('serial_number')
 option: Flag('all', autofill=True, cli_name='all', default=False)
 option: Str('cacn?', cli_name='ca')
+option: Flag('no_members', autofill=True, default=False)
 option: Str('out?')
 option: Flag('raw', autofill=True, cli_name='raw', default=False)
 option: Str('version?')
diff --git a/VERSION b/VERSION
index 354aa62..a383578 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=185
-# Last change: cert: add object plugin
+IPA_API_VERSION_MINOR=186
+# Last change: cert: add owner information
diff --git a/ipaserver/plugins/cert.py b/ipaserver/plugins/cert.py
index 9364fdd..5698c3a 100644
--- a/ipaserver/plugins/cert.py
+++ b/ipaserver/plugins/cert.py
@@ -19,6 +19,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
 
+import base64
 import binascii
 import datetime
 import os
@@ -110,6 +111,9 @@ EXAMPLES:
  Search for certificates based on 

Re: [Freeipa-devel] Another batch of Python 3 patches

[Alexander Bokovoy]

On Wed, 15 Jun 2016, Petr Spacek wrote:

master:
* f753ad322dfdd81907a309827bddfcb1e47917a7 test_xmlrpc: Use absolute imports
* 6406c7a5935e9fb9cd41af49f67d6200021b3574 xmlrpc_test: Rename exception 
instance before working with it
* 890f83b0bbd5ec03397e817ed1282fa66efab7da radiusproxy plugin: Use str(error) 
rather than error.message
* 095d0cb7afc3d404829d87bc894d8691be2228ef xmlrpc_test: Expect bytes rather 
than strings for binary attributes
* baaa041b8a272e43c99f00f69fc645a2e92216db ipalib.rpc: Send base64-encoded data 
as string under Python 3
* 14aba1c7c16e3b4e6375305990fffbd86cefbfdd range plugin tests: Use bytes with 
MockLDAP under Python 3
* bdee89001455825bfe2c7e820c6b2d651f1f45eb radiusproxy plugin tests: Expect 
bytes, not text, for ipatokenradiussecret
* 6ddf0d657f70eb03d17af2e63eb52d6ef33305be certprofile plugin: Use binary mode 
for file with binary data
* 20a6a42567f0fa21945f02aa58420c31bc0c9127 test_add_remove_cert_cmd: Use bytes 
for base64.b64encode()

ipa-4-3:
* 5750c3ece9359ca5a1c9ddbbf727eb63862709ce test_xmlrpc: Use absolute imports
* ba3d77253a54e862249476e3d289ebf01f1a9f9a xmlrpc_test: Rename exception 
instance before working with it
* a514ebdc81548f37e5fedb7bb43ca7ddab473af8 radiusproxy plugin: Use str(error) 
rather than error.message
* 4c68bd671a89f3ddda0c09f9abffe0e1b87098cb xmlrpc_test: Expect bytes rather 
than strings for binary attributes
* 72fb2674117ee8c3c1cafa17512e524ea437f966 ipalib.rpc: Send base64-encoded data 
as string under Python 3
* 8f637bc9b1343979a0ead7346a5b0d771f133420 range plugin tests: Use bytes with 
MockLDAP under Python 3
* aa052a0976ce3ab643fd54a10bc5428711dd9b61 radiusproxy plugin tests: Expect 
bytes, not text, for ipatokenradiussecret
* ffb8fcc20848b570bff7a7776ad08d23ac882ff5 certprofile plugin: Use binary mode 
for file with binary data
* c009ea8a7622a567de2101d8577955942e67a44d test_add_remove_cert_cmd: Use bytes 
for base64.b64encode()



I just found out one other nit:
$ ipa help topics
prints some commands as b'command' instead of plain command:

$ ipa help topics
automount
b'automember' Auto Membership Rule.
b'automount'  Automount

These were the ones (and more) I reported in
https://fedorahosted.org/freeipa/ticket/5940
--
/ 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 0153-0158] move ipa-replica-manage del functionality into server-del

[Jan Cholasta]

Hi,

On 12.6.2016 17:31, Martin Babinsky wrote:

On 06/09/2016 08:12 PM, Martin Babinsky wrote:

These patches expand `server_del` to a full fledged IPA master killer in
domain level 1.

Due to 'server uninstallation removed master from topology' use case,
the individual steps are not in the same order as in the original code
to facilitate self-removal from topology without introducing an array of
permissions for master to remove itself.

I had no opportunity to test out the CI test suite because of technical
problems so it would be nice if our upstream QE could give it a spin and
report errors.

http://www.freeipa.org/page/V4/Manage_replication_topology_4_4
https://fedorahosted.org/freeipa/ticket/5181




Attaching rebased patches and bumping for review.

Please note that they depend on 'Server Roles v2' patchset.


Patch 0153:

Should be an ipaserver module, unless it is required on clients as well, 
in which case it should be an ipalib module.



Patch 0154: LGTM


Patch 0155:

In LDAPDelete subclasses, the primary key argument is multivalue, so I'm 
guessing your post_callback won't work correctly.


Also, since this is *server*-del, s/master/server/ where applicable.


Patch 0156: LGTM


Patch 0157:

This looks suspicious:

+result = server_del_cmd(hostname, version=api_version, **options)

Version is automatically filled in in Command.__call__(), why do you add 
it manually here?



Patch 0158: LGTM


Honza

--
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] Another batch of Python 3 patches

[Petr Spacek]

> master: 
> * f753ad322dfdd81907a309827bddfcb1e47917a7 test_xmlrpc: Use absolute imports 
> * 6406c7a5935e9fb9cd41af49f67d6200021b3574 xmlrpc_test: Rename exception 
> instance before working with it 
> * 890f83b0bbd5ec03397e817ed1282fa66efab7da radiusproxy plugin: Use str(error) 
> rather than error.message 
> * 095d0cb7afc3d404829d87bc894d8691be2228ef xmlrpc_test: Expect bytes rather 
> than strings for binary attributes 
> * baaa041b8a272e43c99f00f69fc645a2e92216db ipalib.rpc: Send base64-encoded 
> data as string under Python 3 
> * 14aba1c7c16e3b4e6375305990fffbd86cefbfdd range plugin tests: Use bytes with 
> MockLDAP under Python 3 
> * bdee89001455825bfe2c7e820c6b2d651f1f45eb radiusproxy plugin tests: Expect 
> bytes, not text, for ipatokenradiussecret 
> * 6ddf0d657f70eb03d17af2e63eb52d6ef33305be certprofile plugin: Use binary 
> mode for file with binary data 
> * 20a6a42567f0fa21945f02aa58420c31bc0c9127 test_add_remove_cert_cmd: Use 
> bytes for base64.b64encode() 
> 
> ipa-4-3: 
> * 5750c3ece9359ca5a1c9ddbbf727eb63862709ce test_xmlrpc: Use absolute imports 
> * ba3d77253a54e862249476e3d289ebf01f1a9f9a xmlrpc_test: Rename exception 
> instance before working with it 
> * a514ebdc81548f37e5fedb7bb43ca7ddab473af8 radiusproxy plugin: Use str(error) 
> rather than error.message 
> * 4c68bd671a89f3ddda0c09f9abffe0e1b87098cb xmlrpc_test: Expect bytes rather 
> than strings for binary attributes 
> * 72fb2674117ee8c3c1cafa17512e524ea437f966 ipalib.rpc: Send base64-encoded 
> data as string under Python 3 
> * 8f637bc9b1343979a0ead7346a5b0d771f133420 range plugin tests: Use bytes with 
> MockLDAP under Python 3 
> * aa052a0976ce3ab643fd54a10bc5428711dd9b61 radiusproxy plugin tests: Expect 
> bytes, not text, for ipatokenradiussecret 
> * ffb8fcc20848b570bff7a7776ad08d23ac882ff5 certprofile plugin: Use binary 
> mode for file with binary data 
> * c009ea8a7622a567de2101d8577955942e67a44d test_add_remove_cert_cmd: Use 
> bytes for base64.b64encode() 


I just found out one other nit:
$ ipa help topics
prints some commands as b'command' instead of plain command:

$ ipa help topics
automount
b'automember' Auto Membership Rule.
b'automount'  Automount

-- 
Petr^2 Spacek

-- 
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] 0206 adtrust optimize forest root LDAP filter

[Martin Basti]

On 15.06.2016 09:02, Martin Babinsky wrote:

On 06/14/2016 04:45 PM, Alexander Bokovoy wrote:

On Tue, 07 Jun 2016, Alexander Bokovoy wrote:

Hi,

`ipa trust-find' command should only show trusted forest root domains

The child domains should be visible via

  ipa trustdomain-find forest.root

The difference between forest root (or external domain) and child
domains is that root domain gets ipaIDObject class to allow assigning a
POSIX ID to the object. This POSIX ID is used by Samba when an Active
Directory domain controller connects as forest trusted domain object.

Child domains can only talk to IPA via forest root domain, thus they
don't need POSIX ID for their TDOs. This allows us a way to
differentiate objects for the purpose of 'trust-find' /
'trustdomain-find' commands.

Fixes https://fedorahosted.org/freeipa/ticket/5942


This patch needs review.



ACK.


Pushed to master: 905db92e61c2e56f8cce723e9c9d28e7968eccc4

--
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] 0045-47: webui: Sub-CAs

[Martin Basti]

On 14.06.2016 18:30, Petr Vobornik wrote:

On 06/14/2016 10:17 AM, Pavel Vomacka wrote:


On 06/14/2016 06:42 AM, Fraser Tweedale wrote:

On Mon, Jun 13, 2016 at 07:48:58PM +0200, Pavel Vomacka wrote:

On 06/13/2016 06:55 AM, Fraser Tweedale wrote:

On Fri, Jun 10, 2016 at 04:34:33PM +0200, Pavel Vomacka wrote:

Hello,

please review these new patches which add WebUI for Sub-CAs.

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


Hi Pavel, I have reviewed the functionality of the patches.
Functionality is good - a few minor comments below.

Hello, thank you for review.

Patch 45:

1) In the main `Certificate Authorities' table, `Subject DN' is
showing the DN of the IPA object, instead of the Subject DN.

Fixed.

2) In the `Certificate Authorities' detail table, there is an
unlabelled row showing the DN of the IPA object.  IMO we do not need
to show this value at all.

The field removed.

Patch 46:

3) I see a FIXME in certificate.js.  The behaviour (default to IPA
CA / 'ipa') is OK.  Alternatively, you could allow the user to not
specify a CA (this will allow the default - currently 'ipa' - to be
controlled by server).

FIXME comment removed.

Patch 47:

4) For backwards compatibility, a CA ACL without any specified CAs
(and not cacat=all) implies the 'ipa' CA.  It would be good to
indicate this in the UI somehow, or include a notice to explain.

I added tooltip next to the checkbox on adder dialog and also note
above the
table with CAs in CA ACL details view.


The message has a small typo: "specificed" should be "specified"
(the typo occurrs in both places).

Once this is fixed, ACK.

Updated patches attached.

Also ACK from Web UI internals perspective.

Patch 0045-2: ACK

Patch 0046-2: ACK, could be split into two patches but don't bother...

Patch 0047-3: ACK


sub-CA patches were pushed, we can push this
pushed to master:
* 6e78169e3bc71c5ff3824e91cce8a0d6d9580d7a Add new webui plugin - ca
* f4dd2446cd8b2c2a814c7bcb95415eb186b0a70f Extend certificate entity page
* 5e5df4abf037161d9c9d9fd5e6051f861dff4bd1 Extend caacl entity

--
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 0159-0160] emancipate IPA NTP service into role

[Martin Basti]

On 15.06.2016 09:55, Petr Vobornik wrote:

On 06/14/2016 07:28 PM, Martin Basti wrote:


On 14.06.2016 18:58, Martin Babinsky wrote:

On 06/14/2016 05:06 PM, Martin Basti wrote:


On 12.06.2016 17:37, Martin Babinsky wrote:

These two patches turn oft-neglected ntp service into a full fledged
role whose status can be queried centrally. They should also enable
generation of location-specific _ntp._udp records.

Please note that NTP is LDAP-enabled by additional call after DS
instance is configured. I was not feeling confident by swapping NTP
and DS configuration steps as I was afraid it will break things. If
not, I will happily update the patch accordingly.

https://fedorahosted.org/freeipa/ticket/5815
https://fedorahosted.org/freeipa/ticket/5826




Hello, I have a few comments:

Patch: 159
1)
+if ntp.is_configured():
+ntp.ldap_enable('NTP', fqdn, None, base_dn)
+ntp.enable()

All ipa services are in disabled state, ipactl starts them according
configuration in LDAP
IMO it should be something like:
ntp.disable()
if running:
 ntp.start()

2)
could you upgrade NTP only once in upgrade.py? Use sysupgrade state

3)
+'NTP': ('ntpd', 42),
I prefer 45, it is easier to put any service before NTP if needed
without huge renumbering


Patch 160: LGTM

Martin^2



Right, attaching updated patches.


Patches are good, but I'm curious if there is any chance for NTP to be
able synchronize time before replication on replica install. If no, IMO
better is to move NTP service configuration after dirserver to be able
to configure LDAP entry directly.
But if there is not time for this, I'm fine with opening ticket and
fixing it later.

Martin^2

Isn't it already done during Client part of replica installation?


With domain level 1 yes, with domain level 0, 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] [PATCH 0159-0160] emancipate IPA NTP service into role

[Petr Vobornik]

On 06/14/2016 07:28 PM, Martin Basti wrote:
> 
> 
> On 14.06.2016 18:58, Martin Babinsky wrote:
>> On 06/14/2016 05:06 PM, Martin Basti wrote:
>>>
>>>
>>> On 12.06.2016 17:37, Martin Babinsky wrote:
 These two patches turn oft-neglected ntp service into a full fledged
 role whose status can be queried centrally. They should also enable
 generation of location-specific _ntp._udp records.

 Please note that NTP is LDAP-enabled by additional call after DS
 instance is configured. I was not feeling confident by swapping NTP
 and DS configuration steps as I was afraid it will break things. If
 not, I will happily update the patch accordingly.

 https://fedorahosted.org/freeipa/ticket/5815
 https://fedorahosted.org/freeipa/ticket/5826



>>> Hello, I have a few comments:
>>>
>>> Patch: 159
>>> 1)
>>> +if ntp.is_configured():
>>> +ntp.ldap_enable('NTP', fqdn, None, base_dn)
>>> +ntp.enable()
>>>
>>> All ipa services are in disabled state, ipactl starts them according
>>> configuration in LDAP
>>> IMO it should be something like:
>>> ntp.disable()
>>> if running:
>>> ntp.start()
>>>
>>> 2)
>>> could you upgrade NTP only once in upgrade.py? Use sysupgrade state
>>>
>>> 3)
>>> +'NTP': ('ntpd', 42),
>>> I prefer 45, it is easier to put any service before NTP if needed
>>> without huge renumbering
>>>
>>>
>>> Patch 160: LGTM
>>>
>>> Martin^2
>>>
>>>
>>
>> Right, attaching updated patches.
>>
> 
> Patches are good, but I'm curious if there is any chance for NTP to be
> able synchronize time before replication on replica install. If no, IMO
> better is to move NTP service configuration after dirserver to be able
> to configure LDAP entry directly.
> But if there is not time for this, I'm fine with opening ticket and
> fixing it later.
> 
> Martin^2

Isn't it already done during Client part of replica installation?

-- 
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] 0206 adtrust optimize forest root LDAP filter

[Martin Babinsky]

On 06/14/2016 04:45 PM, Alexander Bokovoy wrote:

On Tue, 07 Jun 2016, Alexander Bokovoy wrote:

Hi,

`ipa trust-find' command should only show trusted forest root domains

The child domains should be visible via

  ipa trustdomain-find forest.root

The difference between forest root (or external domain) and child
domains is that root domain gets ipaIDObject class to allow assigning a
POSIX ID to the object. This POSIX ID is used by Samba when an Active
Directory domain controller connects as forest trusted domain object.

Child domains can only talk to IPA via forest root domain, thus they
don't need POSIX ID for their TDOs. This allows us a way to
differentiate objects for the purpose of 'trust-find' /
'trustdomain-find' commands.

Fixes https://fedorahosted.org/freeipa/ticket/5942


This patch needs review.



ACK.

--
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] 0059..0064 Lightweight sub-CAs

[Fraser Tweedale]

On Wed, Jun 15, 2016 at 07:30:26AM +0200, Jan Cholasta wrote:
> On 15.6.2016 04:02, Fraser Tweedale wrote:
> > On Tue, Jun 14, 2016 at 03:21:24PM +0200, Martin Babinsky wrote:
> > > On 06/14/2016 04:55 AM, Fraser Tweedale wrote:
> > > > On Tue, Jun 14, 2016 at 02:19:27AM +1000, Fraser Tweedale wrote:
> > > > > On Mon, Jun 13, 2016 at 04:35:54PM +0200, Martin Babinsky wrote:
> > > > > > > > > 
> > > > > > > > > Hi Fraser,
> > > > > > > > > 
> > > > > > > > > during functional review I found the following issues:
> > > > > > > > > 
> > > > > > > > > 1.)
> > > > > > > > > 
> > > > > > > > > If I create a CAACL rule tied to a specific sub-CA let's say 
> > > > > > > > > for user
> > > > > > > > > certificate issuance:
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > ipa caacl-show user_cert_issuance
> > > > > > > > >   Enabled: TRUE
> > > > > > > > >   User category: all
> > > > > > > > >   CAs: user_sub_ca
> > > > > > > > >   Profiles: caIPAuserCert
> > > > > > > > >   ACL name: user_cert_issuance
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > 
> > > > > > > > > I can still happily request certificate for a user using 
> > > > > > > > > root-CA:
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > >  ipa cert-request cert.csr --principal jdoe --ca ipa
> > > > > > > > >   Certificate: MIID9j.../Ov8mkjFA==
> > > > > > > > >   Subject: CN=jdoe,O=IPA.TEST
> > > > > > > > >   Issuer: CN=Certificate Authority,O=IPA.TEST
> > > > > > > > >   ...
> > > > > > > > > """
> > > > > > > > > 
> > > > > > > > > should not this be denied by CA-ACL rule?
> > > > > > > > > 
> > > > > > > > > The default IPA CAACL rule is like this:
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > ipa caacl-show hosts_services_caIPAserviceCert
> > > > > > > > >   Enabled: TRUE
> > > > > > > > >   Host category: all
> > > > > > > > >   Service category: all
> > > > > > > > >   Profiles: caIPAserviceCert
> > > > > > > > >   ACL name: hosts_services_caIPAserviceCert
> > > > > > > > > 
> > > > > > > > > """
> > > > > > > > > 
> > > > > > > > > so the default rule should not allow users to request certs 
> > > > > > > > > at all.
> > > > > > > > > 
> > > > > > > > Yes, these should be denied.  Looking into it.
> > > > > > > > 
> > > > > > > Were you using 'admin' account to request the cert?  admin has
> > > > > > > permission 'Request Certificate ignoring CA ACLs' via the
> > > > > > > 'Certificate Manager' privilege.
> > > > > > > 
> > > > > > > If so, please try again with less privileges (e.g. self-service as
> > > > > > > jdoe).
> > > > > > > 
> > > > > > 
> > > > > > You were right when I was requesting certs as user principals 
> > > > > > themselves
> > > > > > everything worked as expected.
> > > > > > 
> > > > > > Regarding usb-CAs not present on replica, both machines run the 
> > > > > > following
> > > > > > version of dogtag CA:
> > > > > > 
> > > > > > pki-ca-10.3.2-3.fc24.noarch
> > > > > > 
> > > > > > Here are the last 256 lines of the debug log:
> > > > > > 
> > > > > > http://paste.fedoraproject.org/378512/65828332
> > > > > > 
> > > > > Thanks for that.  It seems there are two issues.  The first one is:
> > > > > 
> > > > > Unable to read key retriever class from CS.cfg: Property
> > > > > features.authority.keyRetrieverClass missing value
> > > > > 
> > > > > This happens only on replica, until the first restart of Dogtag
> > > > > after installation.  I have attached a patch to fix it.
> > > > > 
> > > > > The second issue is:
> > > > > 
> > > > > Failed to update certificate
> > > > >   
> > > > > 
> > > > > This needs to be fixed in Dogtag, however, the error is not
> > > > > triggered if the key retrieval succeeds when the replica first
> > > > > observes the addition of a new CA.  The attached patch does help in
> > > > > this regard, so apply it and I hope it will see you through the rest
> > > > > of the functional testing of my patches... I hope.
> > > > > 
> > > > > Thank you for testing!
> > > > > 
> > > > > Cheers,
> > > > > Fraser
> > > > > 
> > > > Rebased patches attached (VERSION conflicts; no other changes).
> > > > 
> > > 
> > > Thanks Fraser,
> > > 
> > > the certificate issuance by sub-CA now works also on replica but I must
> > > first restart PKI for it to pick up stuff from master CA.
> > > 
> > > If I set up a replica first and then create a sub-CA on master Dogtag 
> > > picks
> > > this up and uses the new sub-CA and its key for signing without restart.
> > > 
> > > This seems to be pure Dogtag issue, however and I concluded that IPA side
> > > works ok.
> > > 
> > > If no one objects then I would give functional ACK to your patches and
> > > document/track the Dogtag issue in a separate ticket.
> 
> Pushed to master: f0915e61986f545ad9b282fa90a4b1d0538829c5
> 
> > > 
> > Thanks Martin,
> > 
> > The Dogtag ticket is https://fedorahosted.org/pki/ticket/2359.  Fix
> > is merged and will go out in 10.3.3 this week or early next week.
> 
> Once released,