Re: [Freeipa-devel] [Design Review Request] V4/Automatic_Certificate_Request_Generation

[Ben Lipton]

On 07/25/2016 07:45 AM, Jan Cholasta wrote:

On 25.7.2016 13:11, Alexander Bokovoy wrote:

On Mon, 25 Jul 2016, Jan Cholasta wrote:

On 20.7.2016 16:05, Ben Lipton wrote:

Hi,

Thanks very much for the feedback! Some responses below; I hope you'll
let me know what you think of my reasoning.


On 07/20/2016 04:20 AM, Jan Cholasta wrote:

Hi,

On 17.6.2016 00:06, Ben Lipton wrote:

On 06/14/2016 08:27 AM, Ben Lipton wrote:

Hello all,

I have written up a design proposal for making certificate requests
easier to generate when using alternate certificate profiles:
http://www.freeipa.org/page/V4/Automatic_Certificate_Request_Generation. 




The use case for this is described in
https://fedorahosted.org/freeipa/ticket/4899. I will be working on
implementing this design over the next couple of months. If you 
have

the time and interest, please take a look and share any comments or
concerns that you have.

Thanks!

Ben


Just a quick update to say that I've created a new document that
covers
the proposed schema additions in a more descriptive way (with
diagrams!)
I'm very new to developing with LDAP, so some more experienced 
eyes on

the proposal would be very helpful, even if you don't have time to
absorb the full design. Please take a look at
http://www.freeipa.org/page/V4/Automatic_Certificate_Request_Generation/Schema 




if you have a chance.


I finally had a chance to take a look at this, here are some 
comments:


1) I don't like how transformation rules are tied to a particular
helper and have to be duplicated for each of them. They should be
generic and work with any helper, as helpers are just an
implementation detail and their resulting data is the same.

In fact, I think I would prefer if the CSR was generated using
python-cryptography's CertificateSigningRequestBuilder [1] rather 
than

openssl or certutil or any other command line tool.


There are lots of tools that users might want to use to manage their
private keys, so I don't know if we can assume that whatever 
library we

prefer will actually be able to access the private key to sign a CSR,
which is why I thought it would be useful to support more than one.


python-cryptography has the notion of backends, which allow it to
support multiple crypto implementations. Upstream it currently
supports only OpenSSL [2], but some work has been done on PKCS#11
backend [3], which provides support for HSMs and soft-tokens (like NSS
databases).

Alternatively, for NSS databases (and other "simple" cases), you can
generate the private key with python-cryptography using the default
backend, export it to a file and import the file to the target
database, so you don't actually need the PKCS#11 backend for them.

So, the only thing that's currently lacking is HSM support, but given
that we don't support HSMs in IPA nor in certmonger, I don't think
it's an issue for now.


The
purpose of the mapping rule is to tie together the transformation 
rules

that produce the same data into an object that's
implementation-agnostic, so that profiles referencing those rules are
automatically compatible with all the helper options.


They are implementation-agnostic, as long as you consider `openssl`
and `certutil` the only implementations :-) But I don't think this
solution scales well to other possible implementations.

Anyway, my main grudge is that the transformation rules shouldn't
really be stored on and processed by the server. The server should
know the *what* (mapping rules), but not the *how* (transformation
rules). The *how* is an implementation detail and does not change in
time, so there's no benefit in handling it on the server. It should be
handled exclusively on the client, which I believe would also make the
whole thing more robust (it would not be possible for a bug on the
server to break all the clients).

This is a good point. However, for the scope of Ben's project can we
limit it by openssl and certutil support? Otherwise Ben wouldn't be able
to complete the project in time.


I'm fine with that, but I don't think it's up to me :-)




This is turning out to be a common (and, I think, reasonable) reaction
to the proposal. It is rather complex, and I worry that it will be
difficult to configure. On the other hand, there is some hidden
complexity to enabling a simpler config format, as well. One of the
goals of the project as it was presented to me was to allow the 
creation

of profiles that add certificate extensions *that FreeIPA doesn't yet
know about*. With the current proposal, one only has to add a rule
generating text that the helper will understand.


... which will be possible only as long as the helper understands the
extension. Which it might not, thus the current proposal works only
for *some* extensions that FreeIPA doesn't yet support.

We can go ad infinitum here but with any helper implementation, be it
python-cryptography or anything else, you will need to have a support
there as well.


My point was that the current proposal is 

Re: [Freeipa-devel] [PATCH 0213] support multiple uid values in slapi-nis users map

[thierry bordaz]

On 08/08/2016 05:20 PM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, thierry bordaz wrote:



On 08/08/2016 04:20 PM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, thierry bordaz wrote:



On 08/08/2016 10:56 AM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Lukas Slebodnik wrote:

On (08/08/16 11:35), Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Martin Basti wrote:



On 08.08.2016 09:34, Alexander Bokovoy wrote:

When SSSD resolves AD users on behalf of slapi-nis, it can

accept any

user identifier, including user principal name (UPN) which may be
different than the canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using

canonical user
name but the filter for search will refer to the original 
(aliased)

name. The search will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two 
values for
'uid' attribute: the canonical one and the aliased one. This 
way the

search will match.

Standard LDAP schema allows multiple values for 'uid' 
attribute. We

actually use the same trick for 'cn' attribute in the groups map
already.

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





Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?
No, this is not required. In Fedora we'll submit a combined 
update --
I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide 
already

but did not submit a Bodhi request.


How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
dnf update freeipa-server.
An update file in FreeIPA that is proposed by this patch does not 
affect

operation of the older slapi-nis deployment once update is applied.



Hi,

Is '%first' returning the first value of the attribute 'uid' ?
If there are several values (canonical, alias,... ), does the order 
matters ?

We insert the canonical one first and it seems that 389-ds does not
change the order, at least in my tests. You can see the output in the
bug https://bugzilla.redhat.com/show_bug.cgi?id=1361123#c2


From ldap pov (https://tools.ietf.org/html/rfc4511#section-4.1.7) the 
order of the values is not preserved.
I think it is a bit risky to rely on a specific order especially with 
complex updates (adding several values in a single mod, replace) and 
replication.
would it help to add canonical value with a subtype (e.g. 
'uid;canonical: ') ?

Not sure how what you are mention is relevant here. We talk about
slapi-nis map cache entries which are not modified, replaced or
replicated anywhere by anything other than slapi-nis itself. When
entries are changed by slapi-nis, they are regenerated from scratch.

Your worries do not apply here.

ok.
I understand my mistake, I was thinking the compat entry had a 
associated real entry in ldap and this real entry had two uid values.


--
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 0213] support multiple uid values in slapi-nis users map

[Alexander Bokovoy]

On Mon, 08 Aug 2016, thierry bordaz wrote:



On 08/08/2016 04:20 PM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, thierry bordaz wrote:



On 08/08/2016 10:56 AM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Lukas Slebodnik wrote:

On (08/08/16 11:35), Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Martin Basti wrote:



On 08.08.2016 09:34, Alexander Bokovoy wrote:
When SSSD resolves AD users on behalf of slapi-nis, it 
can

accept any

user identifier, including user principal name (UPN) which may be
different than the canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using

canonical user

name but the filter for search will refer to the original (aliased)
name. The search will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two values for
'uid' attribute: the canonical one and the aliased one. 
This way the

search will match.

Standard LDAP schema allows multiple values for 'uid' attribute. We
actually use the same trick for 'cn' attribute in the groups map
already.

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





Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?

No, this is not required. In Fedora we'll submit a combined update --
I've built slapi-nis-0.56.1-1 packages for f24, f25, and 
rawhide already

but did not submit a Bodhi request.


How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
dnf update freeipa-server.
An update file in FreeIPA that is proposed by this patch does 
not affect

operation of the older slapi-nis deployment once update is applied.



Hi,

Is '%first' returning the first value of the attribute 'uid' ?
If there are several values (canonical, alias,... ), does the 
order matters ?

We insert the canonical one first and it seems that 389-ds does not
change the order, at least in my tests. You can see the output in the
bug https://bugzilla.redhat.com/show_bug.cgi?id=1361123#c2


From ldap pov (https://tools.ietf.org/html/rfc4511#section-4.1.7) the 
order of the values is not preserved.
I think it is a bit risky to rely on a specific order especially with 
complex updates (adding several values in a single mod, replace) and 
replication.
would it help to add canonical value with a subtype (e.g. 
'uid;canonical: ') ?

Not sure how what you are mention is relevant here. We talk about
slapi-nis map cache entries which are not modified, replaced or
replicated anywhere by anything other than slapi-nis itself. When
entries are changed by slapi-nis, they are regenerated from scratch.

Your worries do not apply here.
--
/ 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 0213] support multiple uid values in slapi-nis users map

[thierry bordaz]

On 08/08/2016 04:20 PM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, thierry bordaz wrote:



On 08/08/2016 10:56 AM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Lukas Slebodnik wrote:

On (08/08/16 11:35), Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Martin Basti wrote:



On 08.08.2016 09:34, Alexander Bokovoy wrote:
When SSSD resolves AD users on behalf of slapi-nis, it can 

accept any

user identifier, including user principal name (UPN) which may be
different than the canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using 

canonical user

name but the filter for search will refer to the original (aliased)
name. The search will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two values for
'uid' attribute: the canonical one and the aliased one. This way 
the

search will match.

Standard LDAP schema allows multiple values for 'uid' attribute. We
actually use the same trick for 'cn' attribute in the groups map
already.

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





Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?

No, this is not required. In Fedora we'll submit a combined update --
I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide 
already

but did not submit a Bodhi request.


How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
dnf update freeipa-server.
An update file in FreeIPA that is proposed by this patch does not 
affect

operation of the older slapi-nis deployment once update is applied.



Hi,

Is '%first' returning the first value of the attribute 'uid' ?
If there are several values (canonical, alias,... ), does the order 
matters ?

We insert the canonical one first and it seems that 389-ds does not
change the order, at least in my tests. You can see the output in the
bug https://bugzilla.redhat.com/show_bug.cgi?id=1361123#c2


From ldap pov (https://tools.ietf.org/html/rfc4511#section-4.1.7) the 
order of the values is not preserved.
I think it is a bit risky to rely on a specific order especially with 
complex updates (adding several values in a single mod, replace) and 
replication.
would it help to add canonical value with a subtype (e.g. 
'uid;canonical: ') ?




--
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 0213] support multiple uid values in slapi-nis users map

[Alexander Bokovoy]

On Mon, 08 Aug 2016, thierry bordaz wrote:



On 08/08/2016 10:56 AM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Lukas Slebodnik wrote:

On (08/08/16 11:35), Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Martin Basti wrote:



On 08.08.2016 09:34, Alexander Bokovoy wrote:
When SSSD resolves AD users on behalf of slapi-nis, it can 

accept any

user identifier, including user principal name (UPN) which may be
different than the canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using 

canonical user

name but the filter for search will refer to the original (aliased)
name. The search will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two values for
'uid' attribute: the canonical one and the aliased one. This way the
search will match.

Standard LDAP schema allows multiple values for 'uid' attribute. We
actually use the same trick for 'cn' attribute in the groups map
already.

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





Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?

No, this is not required. In Fedora we'll submit a combined update --
I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide 
already

but did not submit a Bodhi request.


How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
dnf update freeipa-server.

An update file in FreeIPA that is proposed by this patch does not affect
operation of the older slapi-nis deployment once update is applied.



Hi,

Is '%first' returning the first value of the attribute 'uid' ?
If there are several values (canonical, alias,... ), does the order 
matters ?

We insert the canonical one first and it seems that 389-ds does not
change the order, at least in my tests. You can see the output in the
bug https://bugzilla.redhat.com/show_bug.cgi?id=1361123#c2
--
/ 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

[Freeipa-devel] [PATCH 0035] Remove Custodia server keys from LDAP

[Christian Heimes]

The server-del plugin now removes the Custodia keys for encryption and
key signing from LDAP.

https://fedorahosted.org/freeipa/ticket/6015
From be4d66075d108fd9188a3a0b906bace6f6ea5122 Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Mon, 8 Aug 2016 16:06:08 +0200
Subject: [PATCH] Remove Custodia server keys from LDAP

The server-del plugin now removes the Custodia keys for encryption and
key signing from LDAP.

https://fedorahosted.org/freeipa/ticket/6015
---
 ipalib/constants.py |  1 +
 ipaserver/plugins/server.py | 29 +
 2 files changed, 30 insertions(+)

diff --git a/ipalib/constants.py b/ipalib/constants.py
index 0574bb3aa457dd79a6d64f6b8a6b57161d32da92..9b351e260f15211330521453b3ffcd41433a04bb 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -124,6 +124,7 @@ DEFAULT_CONFIG = (
 ('container_locations', DN(('cn', 'locations'), ('cn', 'etc'))),
 ('container_ca', DN(('cn', 'cas'), ('cn', 'ca'))),
 ('container_dnsservers', DN(('cn', 'servers'), ('cn', 'dns'))),
+('container_custodia', DN(('cn', 'custodia'), ('cn', 'ipa'), ('cn', 'etc'))),
 
 # Ports, hosts, and URIs:
 ('xmlrpc_uri', 'http://localhost:/ipa/xml'),
diff --git a/ipaserver/plugins/server.py b/ipaserver/plugins/server.py
index b245dcf72a2f9f32f52ec9acf68d96c69d6169c5..d62c0232c5e33642e44a088dbfd9f10675d733f4 100644
--- a/ipaserver/plugins/server.py
+++ b/ipaserver/plugins/server.py
@@ -609,6 +609,32 @@ class server_del(LDAPDelete):
 message=_("Failed to remove server %(master)s from server "
   "list: %(err)s") % dict(master=master, err=e)))
 
+def _remove_server_custodia_keys(self, ldap, master):
+"""
+Delete all Custodia encryption and signing keys
+"""
+conn = self.Backend.ldap2
+env = self.api.env
+# search for memberPrincipal=*/fqdn@realm
+member_filter = ldap.make_filter_from_attr(
+'memberPrincipal', "/{}@{}".format(master, env.realm),
+exact=False, leading_wildcard=True, trailing_wildcard=False)
+custodia_subtree = DN(env.container_custodia, env.basedn)
+try:
+entries = conn.get_entries(custodia_subtree,
+   ldap.SCOPE_SUBTREE,
+   filter=member_filter)
+for entry in entries:
+conn.delete_entry(entry)
+except errors.NotFound:
+pass
+except Exception as e:
+self.add_message(
+messages.ServerRemovalWarning(
+message=_(
+"Failed to clean up Custodia keys for "
+"%(master)s: %(err)s") % dict(master=master, err=e)))
+
 def _remove_server_host_services(self, ldap, master):
 """
 delete server kerberos key and all its svc principals
@@ -682,6 +708,9 @@ class server_del(LDAPDelete):
 # remove the references to master's ldap/http principals
 self._remove_server_principal_references(pkey)
 
+# remove Custodia encryption and signing keys
+self._remove_server_custodia_keys(ldap, pkey)
+
 # finally destroy all Kerberos principals
 self._remove_server_host_services(ldap, pkey)
 
-- 
2.7.4



signature.asc
Description: OpenPGP digital 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 0034] Secure permissions of Custodia server.keys

[Christian Heimes]

I have split up patch 0032 into two smaller patches. This patch only
addresses the server.keys file.

Custodia's server.keys file contain the private RSA keys for encrypting
and signing Custodia messages. The file was created with permission 644
and is only secured by permission 700 of the directory
/etc/ipa/custodia. The installer and upgrader ensure that the file
has 600.

https://bugzilla.redhat.com/show_bug.cgi?id=1353936
https://fedorahosted.org/freeipa/ticket/6056
From 29cdaa5e27e7b8b3690d222c43eb0edfefdd82ba Mon Sep 17 00:00:00 2001
From: Christian Heimes 
Date: Mon, 8 Aug 2016 15:05:52 +0200
Subject: [PATCH] Secure permissions of Custodia server.keys

Custodia's server.keys file contain the private RSA keys for encrypting
and signing Custodia messages. The file was created with permission 644
and is only secured by permission 700 of the directory
/etc/ipa/custodia. The installer and upgrader ensure that the file
has 600.

https://bugzilla.redhat.com/show_bug.cgi?id=1353936
https://fedorahosted.org/freeipa/ticket/6056
---
 ipapython/secrets/kem.py  | 4 +++-
 ipaserver/install/custodiainstance.py | 4 
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/ipapython/secrets/kem.py b/ipapython/secrets/kem.py
index d45efe8cc4fb63ae9d8c0b2c920fd1f9e5331a9d..9c69adee2d30c246194ac1b05b644f07d365e5af 100644
--- a/ipapython/secrets/kem.py
+++ b/ipapython/secrets/kem.py
@@ -143,7 +143,9 @@ class KEMLdap(iSecLdap):
 def newServerKeys(path, keyid):
 skey = JWK(generate='RSA', use='sig', kid=keyid)
 ekey = JWK(generate='RSA', use='enc', kid=keyid)
-with open(path, 'w+') as f:
+with open(path, 'w') as f:
+os.fchmod(f.fileno(), 0o600)
+os.fchown(f.fileno(), 0, 0)
 f.write('[%s,%s]' % (skey.export(), ekey.export()))
 return [skey.get_op_key('verify'), ekey.get_op_key('encrypt')]
 
diff --git a/ipaserver/install/custodiainstance.py b/ipaserver/install/custodiainstance.py
index fd30430bbf9c39e7153986999199474cfca60d09..b2b32a26615539b62de7503b12cd3fb5f3684344 100644
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -73,6 +73,10 @@ class CustodiaInstance(SimpleServiceInstance):
 if not sysupgrade.get_upgrade_state("custodia", "installed"):
 root_logger.info("Custodia service is being configured")
 self.create_instance()
+mode = os.stat(self.server_keys).st_mode
+if stat.S_IMODE(mode) != 0o600:
+root_logger.info("Secure server.keys mode")
+os.chmod(self.server_keys, 0o600)
 
 def create_replica(self, master_host_name):
 suffix = ipautil.realm_to_suffix(self.realm)
-- 
2.7.4



signature.asc
Description: OpenPGP digital 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

Re: [Freeipa-devel] [PATCH 0213] support multiple uid values in slapi-nis users map

[thierry bordaz]

On 08/08/2016 10:56 AM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Lukas Slebodnik wrote:

On (08/08/16 11:35), Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Martin Basti wrote:



On 08.08.2016 09:34, Alexander Bokovoy wrote:
> When SSSD resolves AD users on behalf of slapi-nis, it can accept 
any

> user identifier, including user principal name (UPN) which may be
> different than the canonical user name which SSSD returns.
>
> As result, the entry created by slapi-nis will be using canonical 
user

> name but the filter for search will refer to the original (aliased)
> name. The search will not match the newly created entry.
>
> The issue is fixed  in slapi-nis-0.56.1 by returning two values for
> 'uid' attribute: the canonical one and the aliased one. This way the
> search will match.
>
> Standard LDAP schema allows multiple values for 'uid' attribute. We
> actually use the same trick for 'cn' attribute in the groups map
> already.
>
> https://fedorahosted.org/freeipa/ticket/6138
>
>
>
>
Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?

No, this is not required. In Fedora we'll submit a combined update --
I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide 
already

but did not submit a Bodhi request.


How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
 dnf update freeipa-server.

An update file in FreeIPA that is proposed by this patch does not affect
operation of the older slapi-nis deployment once update is applied.



Hi,

Is '%first' returning the first value of the attribute 'uid' ?
If there are several values (canonical, alias,... ), does the order 
matters ?


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

Re: [Freeipa-devel] [PATCH] ipa_pwd_extop: Fix warning declaration shadows previous

[Martin Basti]

On 08.08.2016 13:58, thierry bordaz wrote:



On 08/08/2016 01:56 PM, Lukas Slebodnik wrote:

On (08/08/16 13:30), thierry bordaz wrote:


On 08/05/2016 02:16 PM, Lukas Slebodnik wrote:

ehlo,

attached patches fixes few compiler warnings in ipa-extop.
Sorry for not following naming convention for patches.
But I do not remeber my numer and you will use github/pagure
anyway.

LS



Hi Lukas,

0001-ipa_pwd_extop-Fix-warning-decalration-shadows-previo.patch 
looks ok but

there is a leak in the remaining code.
In fact bind_sdn and target_sdn need to be freed (slapi_sdn_free())
before the end of the 'if (dn)' statement.
Do you want to fix it in your patch of should we use an other patch ?


0002-ipa-pwd-extop-Fix-warning-assignment-discards-const-.patch is 
ok. Ack



If I it is not introduced by this patch then it will be better
to prepare another patch. "git blame" would be confusing.

Thank you for review.

LS

Hi Lukas,

Ok I will prepare a patch for the leaks.

Both patches are ok. ACK

thanks
thierry


Pushed to master: 7e1898bd014234c176b0c5d4d00463c70fba27b0
Pushed to master: 50c53395de7b5b4c62f3bc9004d2c7a94792339f

--
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: Silence sshd messages during install

[Martin Basti]

On 08.08.2016 13:58, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Jan Cholasta wrote:

On 19.7.2016 08:40, Jan Cholasta wrote:

Hi,

On 9.7.2016 14:46, Ben Lipton wrote:

On 07/07/2016 11:19 AM, Ben Lipton wrote:


Thanks for the review! Comments below.


On 07/01/2016 07:42 AM, Martin Basti wrote:




On 29.06.2016 20:46, Ben Lipton wrote:
The attached patch silences some annoying messages I've been 
getting

when upgrading the freeipa-client package on F24:
"""
WARNING: 'UseLogin yes' is not supported in Fedora and may cause
several problems.

This will be fixed by openssh-7.2p2-9.fc24
(https://bugzilla.redhat.com/show_bug.cgi?id=1350347) so we probably
shouldn't worry about it.

Could not load host key: /etc/ssh/ssh_host_dsa_key

This is because by default sshd looks for all of
/etc/ssh/ssh_host_dsa_key, /etc/ssh/ssh_host_ecdsa_key,
/etc/ssh/ssh_host_ed25519_key and /etc/ssh/ssh_host_rsa_key, but
Fedora doesn't generate a DSA key by default.

"""

Since the script causing the message only looks at the return code
from sshd to determine the right options to use, I thought it might
be ok to discard the output. What do you think?

Ben




Hello, I don't like to hiding errors/warnings. Can you determine and
solve the root cause?


I definitely agree with this in principle, but in this case the
purpose of this code is to try different, potentially wrong,
parameters to sshd until it finds a combination that it accepts. It
seems like in some environments this would produce error messages 
that
aren't actionable and don't indicate any problem for package 
function,

which is why I didn't think these messages were necessarily worth
preserving.

On the other hand, if the code makes the wrong decision about sshd
version we might be interested in error logs that show why. Can we 
log

this to a file instead of the console, maybe?

If you'd prefer just addressing the root cause, a patch that prevents
the missing host key error is attached, but it won't stop the error
messages showing up when openssh is an older version.

Thanks,
Ben


Whoops, realized that my patch created a tempfile and didn't delete 
it.

Updated.


I think the first version of the patch was OK. sshd is called only to
check which set of authorized keys options to use, we don't really care
about anything else, so we can safely ignore whatever it puts to 
stderr.


Bump.

ACK on the first version of the patch 
(freeipa-blipton-0001-Silence-sshd-messages-during-install.patch).


Anyone against pushing it?

Given that newer OpenSSH version will silence it anyway, I'm OK with the
interim fix.

Pushed to master: c15ba1f9e8c7d236586d46271fce7c3950b509da

--
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] certmonger proxy configuration not possible ?

[Marx, Peter]

what I feared... 
ok. I will open an enhancement ticket. Hopefully somebody can provide a 
preliminary patch I can apply.

-Original Message-
From: Alexander Bokovoy [mailto:aboko...@redhat.com] 
Sent: Monday, August 08, 2016 11:48 AM
To: Marx, Peter
Cc: Rob Crittenden; 'freeipa-devel@redhat.com'
Subject: Re: [Freeipa-devel] certmonger proxy configuration not possible ?

On Mon, 08 Aug 2016, Marx, Peter wrote:
>I am trying this but it has no effect - as if the environment is not passed to 
>the called helper scep-submit.
>
>In /usr/lib/systemd/certmonger.service there is already a link defined to add 
>stuff:
>[Service]
>..
>EnvironmentFile=/etc/sysconfig/certmonger
>
>In /etc/sysconfig/certmonger I added my proxy like this:
>
>[Service]
>Environment="http_proxy=http://proxyuser:proxypassword@proxyserver:proxyport;
>
>After systemctl daemon-reload and systemctl restart certmonger my 
>requests still do not get to the proxy.
>
>Commenting out the EnvironmetFile line and adding the Environment line 
>directly in certmonger.service had the same result.
>
>Can somebody confirm that the proxy setting is visible to the called 
>scep-submit ?
I've checked certmonger source code and while libcurl can be configured to use 
proxy and proxy authentication, certmonger does not configure it to do so. As 
result, environmental variables have no influence on the use of libcurl by 
certmonger.

It is worth to open a ticket for certmonger to add proxy support.

--
/ Alexander Bokovoy

automechanika - 13.09.-17.09.2016 - Messe Frankfurt - Hall 3.0 - Stand G98 + E91
InnoTrans - 20.09.-23.09.2016 - Messe Berlin - Hall 1.2b - Stand 104 + 210
IAA - 22.09.-29.09.2016 - Messe Hannover - Hall 17 - Stand A30 + D131

Knorr-Bremse IT-Services GmbH
Sitz: Muenchen
Geschaeftsfuehrer: Helmut Draxler (Vorsitzender), Harald Jessen, Harald 
Schneider
Registergericht Muenchen, HR B 167 268

This transmission is intended solely for the addressee and contains 
confidential information.
If you are not the intended recipient, please immediately inform the sender and 
delete the message and any attachments from your system. 
Furthermore, please do not copy the message or disclose the contents to anyone 
unless agreed otherwise. To the extent permitted by law we shall in no way be 
liable for any damages, whatever their nature, arising out of transmission 
failures, viruses, external influence, delays and the like.

-- 
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: Silence sshd messages during install

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Jan Cholasta wrote:

On 19.7.2016 08:40, Jan Cholasta wrote:

Hi,

On 9.7.2016 14:46, Ben Lipton wrote:

On 07/07/2016 11:19 AM, Ben Lipton wrote:


Thanks for the review! Comments below.


On 07/01/2016 07:42 AM, Martin Basti wrote:




On 29.06.2016 20:46, Ben Lipton wrote:

The attached patch silences some annoying messages I've been getting
when upgrading the freeipa-client package on F24:
"""
WARNING: 'UseLogin yes' is not supported in Fedora and may cause
several problems.

This will be fixed by openssh-7.2p2-9.fc24
(https://bugzilla.redhat.com/show_bug.cgi?id=1350347) so we probably
shouldn't worry about it.

Could not load host key: /etc/ssh/ssh_host_dsa_key

This is because by default sshd looks for all of
/etc/ssh/ssh_host_dsa_key, /etc/ssh/ssh_host_ecdsa_key,
/etc/ssh/ssh_host_ed25519_key and /etc/ssh/ssh_host_rsa_key, but
Fedora doesn't generate a DSA key by default.

"""

Since the script causing the message only looks at the return code
from sshd to determine the right options to use, I thought it might
be ok to discard the output. What do you think?

Ben




Hello, I don't like to hiding errors/warnings. Can you determine and
solve the root cause?


I definitely agree with this in principle, but in this case the
purpose of this code is to try different, potentially wrong,
parameters to sshd until it finds a combination that it accepts. It
seems like in some environments this would produce error messages that
aren't actionable and don't indicate any problem for package function,
which is why I didn't think these messages were necessarily worth
preserving.

On the other hand, if the code makes the wrong decision about sshd
version we might be interested in error logs that show why. Can we log
this to a file instead of the console, maybe?

If you'd prefer just addressing the root cause, a patch that prevents
the missing host key error is attached, but it won't stop the error
messages showing up when openssh is an older version.

Thanks,
Ben



Whoops, realized that my patch created a tempfile and didn't delete it.
Updated.


I think the first version of the patch was OK. sshd is called only to
check which set of authorized keys options to use, we don't really care
about anything else, so we can safely ignore whatever it puts to stderr.


Bump.

ACK on the first version of the patch 
(freeipa-blipton-0001-Silence-sshd-messages-during-install.patch).


Anyone against pushing it?

Given that newer OpenSSH version will silence it anyway, I'm OK with the
interim fix.
--
/ 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] ipa_pwd_extop: Fix warning declaration shadows previous

[thierry bordaz]

On 08/08/2016 01:56 PM, Lukas Slebodnik wrote:

On (08/08/16 13:30), thierry bordaz wrote:


On 08/05/2016 02:16 PM, Lukas Slebodnik wrote:

ehlo,

attached patches fixes few compiler warnings in ipa-extop.
Sorry for not following naming convention for patches.
But I do not remeber my numer and you will use github/pagure
anyway.

LS



Hi Lukas,

0001-ipa_pwd_extop-Fix-warning-decalration-shadows-previo.patch looks ok but
there is a leak in the remaining code.
In fact bind_sdn and target_sdn need to be freed (slapi_sdn_free())
before the end of the 'if (dn)' statement.
Do you want to fix it in your patch of should we use an other patch ?


0002-ipa-pwd-extop-Fix-warning-assignment-discards-const-.patch is ok. Ack


If I it is not introduced by this patch then it will be better
to prepare another patch. "git blame" would be confusing.

Thank you for review.

LS

Hi Lukas,

Ok I will prepare a patch for the leaks.

Both patches are ok. ACK

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

Re: [Freeipa-devel] [PATCH] ipa_pwd_extop: Fix warning declaration shadows previous

[Lukas Slebodnik]

On (08/08/16 13:30), thierry bordaz wrote:
>
>
>On 08/05/2016 02:16 PM, Lukas Slebodnik wrote:
>> ehlo,
>> 
>> attached patches fixes few compiler warnings in ipa-extop.
>> Sorry for not following naming convention for patches.
>> But I do not remeber my numer and you will use github/pagure
>> anyway.
>> 
>> LS
>> 
>> 
>Hi Lukas,
>
>0001-ipa_pwd_extop-Fix-warning-decalration-shadows-previo.patch looks ok but
>there is a leak in the remaining code.
>In fact bind_sdn and target_sdn need to be freed (slapi_sdn_free())
>before the end of the 'if (dn)' statement.
>Do you want to fix it in your patch of should we use an other patch ?
>
>
>0002-ipa-pwd-extop-Fix-warning-assignment-discards-const-.patch is ok. Ack
>
If I it is not introduced by this patch then it will be better
to prepare another patch. "git blame" would be confusing.

Thank you for review.

LS

-- 
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 0154] client: RPM require initscripts to get *-domainname.service

[Petr Spacek]

On 8.8.2016 13:37, Jan Cholasta wrote:
> Hi,
> 
> On 8.8.2016 13:22, Petr Spacek wrote:
>> Hello,
>>
>> client: RPM require initscripts to get *-domainname.service
>>
>> https://fedorahosted.org/freeipa/ticket/4831
> 
> IIRC there was a task associated with the ticket to investigate if there is a
> better way of setting the domain name on boot. So... is there?
> 
> AFAIK we set the domain name using authconfig, shouldn't authconfig be in
> charge of enabling any required services?

As far as I can tell, this is the way to set NIS domain in RHEL/Fedora.

authconfig cannot set NIS domain only, it does unwanted additional magic like
enabling rpcbind and ypbind services etc.

Alternative is to drop sysctl file to /etc but I do not like it because it
would not be the Red Hat standard way of doing NIS domain config.

Opinions?

-- 
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] 0001: Silence sshd messages during install

[Jan Cholasta]

On 19.7.2016 08:40, Jan Cholasta wrote:

Hi,

On 9.7.2016 14:46, Ben Lipton wrote:

On 07/07/2016 11:19 AM, Ben Lipton wrote:


Thanks for the review! Comments below.


On 07/01/2016 07:42 AM, Martin Basti wrote:




On 29.06.2016 20:46, Ben Lipton wrote:

The attached patch silences some annoying messages I've been getting
when upgrading the freeipa-client package on F24:
"""
WARNING: 'UseLogin yes' is not supported in Fedora and may cause
several problems.

This will be fixed by openssh-7.2p2-9.fc24
(https://bugzilla.redhat.com/show_bug.cgi?id=1350347) so we probably
shouldn't worry about it.

Could not load host key: /etc/ssh/ssh_host_dsa_key

This is because by default sshd looks for all of
/etc/ssh/ssh_host_dsa_key, /etc/ssh/ssh_host_ecdsa_key,
/etc/ssh/ssh_host_ed25519_key and /etc/ssh/ssh_host_rsa_key, but
Fedora doesn't generate a DSA key by default.

"""

Since the script causing the message only looks at the return code
from sshd to determine the right options to use, I thought it might
be ok to discard the output. What do you think?

Ben




Hello, I don't like to hiding errors/warnings. Can you determine and
solve the root cause?


I definitely agree with this in principle, but in this case the
purpose of this code is to try different, potentially wrong,
parameters to sshd until it finds a combination that it accepts. It
seems like in some environments this would produce error messages that
aren't actionable and don't indicate any problem for package function,
which is why I didn't think these messages were necessarily worth
preserving.

On the other hand, if the code makes the wrong decision about sshd
version we might be interested in error logs that show why. Can we log
this to a file instead of the console, maybe?

If you'd prefer just addressing the root cause, a patch that prevents
the missing host key error is attached, but it won't stop the error
messages showing up when openssh is an older version.

Thanks,
Ben



Whoops, realized that my patch created a tempfile and didn't delete it.
Updated.


I think the first version of the patch was OK. sshd is called only to
check which set of authorized keys options to use, we don't really care
about anything else, so we can safely ignore whatever it puts to stderr.


Bump.

ACK on the first version of the patch 
(freeipa-blipton-0001-Silence-sshd-messages-during-install.patch).


Anyone against pushing it?



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] [PATCH] 0002 Added support for authentication with user certificate

[Martin Kosek]

On 08/08/2016 01:31 PM, Jan Pazdziora wrote:
> On Mon, Aug 08, 2016 at 12:52:33PM +0200, Martin Kosek wrote:
>>
>> I discussed this with Jan Pazdziora on IRC, outside of this mail thread, so 
>> let
>> me repeat my suggestion here. I still think it is premature to add plugins 
>> like
>> that to FreeIPA core git. We are not agreed yet how we will distribute 
>> FreeIPA
>> plugins, so I would not rush adding this plugin to FreeIPA core, especially
>> since it is very experimental and not even secure yet. FreeIPA plugin
>> distribution should be more thought through and discussed.
>>
>> As I proposed, this plugin can now live outside of FreeIPA core git, in it's
>> own life cycle (maybe in freeipa-plugins github git repo we create?) so that 
>> it
>> can be updated without updating whole FreeIPA core. In this effort, I would
>> suggest to only consider updates of
>>
>> * ipaserver/plugins/xmlserver.py
>> * ipaserver/rpcserver.py
>>
>> as these would have to patched by admin deploying this feature and would be
>> overwritten by RPM updates. The plugin itself or server.conf can be deployed
>> and installed separatenly, even via other RPM.
> 
> We want the feature (albeit experimental) to be available to upstream
> users and downstream customers, with as few steps to take and as few
> hoops to jump through as possible. Any bits we can get to users' and
> customers' hands via standard means are bits that they don't need to
> get from elsewhere, via nonstandard means, with us inventing and
> supporting these nonstandards processes.
> 
> Assuming the mere existence of the functionality (which will be
> disabled by default) does not decrease security of the default
> installations and configuration, I don't see why carrying it poses
> a problem.

I see your reasoning, I just think it is not strong enough to rush this new
method of delivering plugins in before discussing it more broadly.

Also, as I mentioned, we may want different life cycle for FreeIPA plugins that
we want for FreeIPA core bits. Thus the different repository suggestion. This
whole feature is (still) non-standard and experimental, so I do not personally
see that big problem in non-standard delivery mechanism.

Martin

-- 
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 685] parameters: move the `confirm` kwarg to Param

[Jan Cholasta]

On 8.8.2016 13:26, Martin Basti wrote:



On 08.08.2016 13:27, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza




Please document this change in Param dosctring

--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -418,6 +418,7 @@ class Param(ReadOnly):
 ('cli_metavar', str, None),
 ('no_convert', bool, False),
 ('deprecated', bool, False),
+('confirm', bool, True),


Martin^2





--
Jan Cholasta
From 9b523cd87d9ce2b9bc0505d7762d02d913400c1c Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 8 Aug 2016 13:09:39 +0200
Subject: [PATCH] parameters: move the `confirm` kwarg to Param

Whether a parameter is treated like password is determined by the
`password` class attribute defined in the Param class. Whether the CLI will
asks for confirmation of a password parameter depends on the value of the
`confirm` kwarg of the Password class.

Move the `confirm` kwarg from the Password class to the Param class, so
that it can be used by any Param subclass which has the `password` class
attribute set to True.

This fixes confirmation of the --key option of otptoken-add, which is a
Bytes subclass with `password` set to True.

https://fedorahosted.org/freeipa/ticket/6174
---
 ipaclient/remote_plugins/schema.py | 2 +-
 ipalib/parameters.py   | 6 ++
 ipaserver/plugins/otptoken.py  | 4 
 3 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index a215452..c06d6d2 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -167,7 +167,7 @@ class _SchemaPlugin(object):
 elif key in ('cli_metavar',
  'cli_name'):
 kwargs[key] = str(value)
-elif key == 'confirm' and issubclass(cls, Password):
+elif key == 'confirm':
 kwargs[key] = value
 elif key == 'default':
 default = value
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 1581b7d..6917c8d 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -377,6 +377,7 @@ class Param(ReadOnly):
 parameter is not `required`
   - sortorder: used to sort a list of parameters for Command. See
 `Command.finalize()` for further information
+  - confirm: if password, ask for confirmation
 """
 
 # This is a dummy type so that most of the functionality of Param can be
@@ -418,6 +419,7 @@ class Param(ReadOnly):
 ('cli_metavar', str, None),
 ('no_convert', bool, False),
 ('deprecated', bool, False),
+('confirm', bool, True),
 
 # The 'default' kwarg gets appended in Param.__init__():
 # ('default', self.type, None),
@@ -1511,10 +1513,6 @@ class Password(Str):
 
 password = True
 
-kwargs = Str.kwargs + (
-('confirm', bool, True),
-)
-
 def _convert_scalar(self, value, index=None):
 if isinstance(value, (tuple, list)) and len(value) == 2:
 (p1, p2) = value
diff --git a/ipaserver/plugins/otptoken.py b/ipaserver/plugins/otptoken.py
index 56b8c91..39012e2 100644
--- a/ipaserver/plugins/otptoken.py
+++ b/ipaserver/plugins/otptoken.py
@@ -79,10 +79,6 @@ class OTPTokenKey(Bytes):
 
 password = True
 
-kwargs = Bytes.kwargs + (
-('confirm', bool, True),
-)
-
 def _convert_scalar(self, value, index=None):
 if isinstance(value, (tuple, list)) and len(value) == 2:
 (p1, p2) = value
-- 
2.7.4

-- 
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 0154] client: RPM require initscripts to get *-domainname.service

[Jan Cholasta]

Hi,

On 8.8.2016 13:22, Petr Spacek wrote:

Hello,

client: RPM require initscripts to get *-domainname.service

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


IIRC there was a task associated with the ticket to investigate if there 
is a better way of setting the domain name on boot. So... is there?


AFAIK we set the domain name using authconfig, shouldn't authconfig be 
in charge of enabling any required services?


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] [PATCH] 0100: Fix question marks in adders in topology graph

[Pavel Vomacka]

On 08/05/2016 02:15 PM, Pavel Vomacka wrote:

Hello,

Please review attached patch.

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




Changed commit message.

--
Pavel^3 Vomacka

From c792fbdf68e338c763079615595de68f63c0107a Mon Sep 17 00:00:00 2001
From: Pavel Vomacka 
Date: Fri, 5 Aug 2016 14:04:03 +0200
Subject: [PATCH] Fix unicode characters in ca and domain adders

Topology graph didn't show plus icons correctly.

There is a problem with uglifying of javascript code. It does not leave unicode character
written in hexadecimal format unchanged. Therefore this workaround which inserts
needed character using Javascript function and uglifiyng does not affect it.

https://fedorahosted.org/freeipa/ticket/6175
---
 install/ui/src/freeipa/topology_graph.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/install/ui/src/freeipa/topology_graph.js b/install/ui/src/freeipa/topology_graph.js
index ce2ebeaff611987ae27f2655b5da80bdcd1b4f8a..4bc3668647979c77719efa78b7a663d0e899216e 100644
--- a/install/ui/src/freeipa/topology_graph.js
+++ b/install/ui/src/freeipa/topology_graph.js
@@ -530,12 +530,14 @@ topology_graph.TopoGraph = declare([Evented], {
 
 function add_labels(type, color, adder_group) {
 var label_radius = 3;
+var decimal_plus = parseInt('f067', 16); // Converts hexadecimal
+// code of plus icon to decimal.
 
 var plus = adder_group
 .append('text')
 .classed('plus', true)
 .classed(type + '_plus', true)
-.text('\uf067');
+.text(String.fromCharCode(decimal_plus));
 
 var label = adder_group.append('path')
 .attr('id', type + '_label');
-- 
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] [PATCH] 0002 Added support for authentication with user certificate

[Jan Pazdziora]

On Mon, Aug 08, 2016 at 12:52:33PM +0200, Martin Kosek wrote:
> 
> I discussed this with Jan Pazdziora on IRC, outside of this mail thread, so 
> let
> me repeat my suggestion here. I still think it is premature to add plugins 
> like
> that to FreeIPA core git. We are not agreed yet how we will distribute FreeIPA
> plugins, so I would not rush adding this plugin to FreeIPA core, especially
> since it is very experimental and not even secure yet. FreeIPA plugin
> distribution should be more thought through and discussed.
>
> As I proposed, this plugin can now live outside of FreeIPA core git, in it's
> own life cycle (maybe in freeipa-plugins github git repo we create?) so that 
> it
> can be updated without updating whole FreeIPA core. In this effort, I would
> suggest to only consider updates of
> 
> * ipaserver/plugins/xmlserver.py
> * ipaserver/rpcserver.py
> 
> as these would have to patched by admin deploying this feature and would be
> overwritten by RPM updates. The plugin itself or server.conf can be deployed
> and installed separatenly, even via other RPM.

We want the feature (albeit experimental) to be available to upstream
users and downstream customers, with as few steps to take and as few
hoops to jump through as possible. Any bits we can get to users' and
customers' hands via standard means are bits that they don't need to
get from elsewhere, via nonstandard means, with us inventing and
supporting these nonstandards processes.

Assuming the mere existence of the functionality (which will be
disabled by default) does not decrease security of the default
installations and configuration, I don't see why carrying it poses
a problem.

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
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] ipa_pwd_extop: Fix warning declaration shadows previous

[thierry bordaz]

On 08/05/2016 02:16 PM, Lukas Slebodnik wrote:

ehlo,

attached patches fixes few compiler warnings in ipa-extop.
Sorry for not following naming convention for patches.
But I do not remeber my numer and you will use github/pagure
anyway.

LS



Hi Lukas,

0001-ipa_pwd_extop-Fix-warning-decalration-shadows-previo.patch looks ok 
but there is a leak in the remaining code.
In fact bind_sdn and target_sdn need to be freed (slapi_sdn_free()) 
before the end of the 'if (dn)' statement.

Do you want to fix it in your patch of should we use an other patch ?


0002-ipa-pwd-extop-Fix-warning-assignment-discards-const-.patch is ok. Ack

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 0215-0216] Child domain fixes for AD trust

[Alexander Bokovoy]

Hi!

Attached two patches attempt to fix some of the issues we see with child
domains.

SSSD only 'sees' users from child domains if there is an ID range for
each of them. However, after refactoring of trust code when external
trust was introduced, part of the range creation had wrong assumption
that if a trusted domain exists, its range also exists. This is now
fixed to try to create range even if the domain exists. In fact, because
the older code was not going to the range creation for trusted domains
which already existed, adding ranges was done incorrectly: ID ranges use
full domain name and don't need - hierarchy, but the code
was passing both parent and the child names. As result, an attempt to
create an ID range for parent was done instead of the child. Parent ID
range already existed so we never got to create child ID ranges at all
in that case.

Finally, there is a fix in SSSD to properly generate CA paths so that
libkrb5 can calculate correct trust path via forest root (parent)
domain. While looking at that, I also decided to simplify logic in
ipa-kdb driver because for cross-forest trust we never can transit to
the child domain directly, we always have to use the forest root domain.
However, old code could actually set a immediate domain's parent instead
of the forest root for deep level trust relationship within the forest
we trust. As we still cannot get to second level or beyond directly or
via their actual parent domain, we always have to go through the forest
root domain. The simplified code enforces this logic.


--
/ Alexander Bokovoy
From 37e4ab4786aec94bfb057fa3146d4e18e30df391 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Sat, 6 Aug 2016 11:12:13 +0300
Subject: [PATCH 4/5] trust: make sure ID range is created for the child domain
 even if it exists

ID ranges for child domains of a forest trust were created incorrectly
in FreeIPA 4.4.0 due to refactoring of -- if the domain was already
existing, we never attempted to create the ID range for it.

At the same time, when domain was missing, we attempted to add ID range
and passed both forest root and the child domain names to add_range().
However, add_range() only looks at the first positional argument which
was the forest root name. That ID range always exists (it is created
before child domains are processed).

Modify the code to make sure child domain name is passed as the first
positional argument. In addition, the oddjob helper should explicitly
set context='server' so that idrange code will be able to see and use
ipaserver/dcerpc.py helpers.

Resolves: https://fedorahosted.org/freeipa/ticket/5738
---
 install/oddjob/com.redhat.idm.trust-fetch-domains |  2 +-
 ipaserver/plugins/trust.py| 10 +++---
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/install/oddjob/com.redhat.idm.trust-fetch-domains 
b/install/oddjob/com.redhat.idm.trust-fetch-domains
index 7c948fd..bffa021 100755
--- a/install/oddjob/com.redhat.idm.trust-fetch-domains
+++ b/install/oddjob/com.redhat.idm.trust-fetch-domains
@@ -76,7 +76,7 @@ env._bootstrap(debug=options.debug, log=None)
 env._finalize_core(**dict(DEFAULT_CONFIG))
 
 # Initialize the API with the proper debug level
-api.bootstrap(in_server=True, debug=env.debug, log=None)
+api.bootstrap(in_server=True, debug=env.debug, log=None, context='server')
 api.finalize()
 
 # Only import trust plugin after api is initialized or internal imports
diff --git a/ipaserver/plugins/trust.py b/ipaserver/plugins/trust.py
index f2e0b1e..f90d9c1 100644
--- a/ipaserver/plugins/trust.py
+++ b/ipaserver/plugins/trust.py
@@ -1673,15 +1673,19 @@ def add_new_domains_from_trust(myapi, trustinstance, 
trust_entry, domains, **opt
 if 'raw' in options:
 dom['raw'] = options['raw']
 
-res = myapi.Command.trustdomain_add(trust_name, name, **dom)
-result.append(res['result'])
+try:
+res = myapi.Command.trustdomain_add(trust_name, name, **dom)
+result.append(res['result'])
+except errors.DuplicateEntry:
+# Ignore updating duplicate entries
+pass
 
 if idrange_type != u'ipa-ad-trust-posix':
 range_name = name.upper() + '_id_range'
 dom['range_type'] = u'ipa-ad-trust'
 add_range(myapi, trustinstance,
   range_name, dom['ipanttrusteddomainsid'],
-  trust_name, name, **dom)
+  name, **dom)
 except errors.DuplicateEntry:
 # Ignore updating duplicate entries
 pass
-- 
2.7.4

From 767458d1532feb7029ff9a52e67e931fd87869ec Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Sun, 7 Aug 2016 21:42:14 +0300
Subject: [PATCH 5/5] ipa-kdb: simplify trusted domain parent search

In terms of cross-forest trust parent domain is the root domain of
the forest 

Re: [Freeipa-devel] [PATCH 685] parameters: move the `confirm` kwarg to Param

[Martin Basti]

On 08.08.2016 13:27, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza




Please document this change in Param dosctring

--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -418,6 +418,7 @@ class Param(ReadOnly):
 ('cli_metavar', str, None),
 ('no_convert', bool, False),
 ('deprecated', bool, False),
+('confirm', bool, True),


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] [PATCH 685] parameters: move the `confirm` kwarg to Param

[Jan Cholasta]

Hi,

the attached patch fixes .

Honza

--
Jan Cholasta
From 9756b9d426b09e38d1ecbdb1e84ec8f5b0f9a957 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Mon, 8 Aug 2016 13:09:39 +0200
Subject: [PATCH] parameters: move the `confirm` kwarg to Param

Whether a parameter is treated like password is determined by the
`password` class attribute defined in the Param class. Whether the CLI will
asks for confirmation of a password parameter depends on the value of the
`confirm` kwarg of the Password class.

Move the `confirm` kwarg from the Password class to the Param class, so
that it can be used by any Param subclass which has the `password` class
attribute set to True.

This fixes confirmation of the --key option of otptoken-add, which is a
Bytes subclass with `password` set to True.

https://fedorahosted.org/freeipa/ticket/6174
---
 ipaclient/remote_plugins/schema.py | 2 +-
 ipalib/parameters.py   | 5 +
 ipaserver/plugins/otptoken.py  | 4 
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index a215452..c06d6d2 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -167,7 +167,7 @@ class _SchemaPlugin(object):
 elif key in ('cli_metavar',
  'cli_name'):
 kwargs[key] = str(value)
-elif key == 'confirm' and issubclass(cls, Password):
+elif key == 'confirm':
 kwargs[key] = value
 elif key == 'default':
 default = value
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 1581b7d..b2e0434 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -418,6 +418,7 @@ class Param(ReadOnly):
 ('cli_metavar', str, None),
 ('no_convert', bool, False),
 ('deprecated', bool, False),
+('confirm', bool, True),
 
 # The 'default' kwarg gets appended in Param.__init__():
 # ('default', self.type, None),
@@ -1511,10 +1512,6 @@ class Password(Str):
 
 password = True
 
-kwargs = Str.kwargs + (
-('confirm', bool, True),
-)
-
 def _convert_scalar(self, value, index=None):
 if isinstance(value, (tuple, list)) and len(value) == 2:
 (p1, p2) = value
diff --git a/ipaserver/plugins/otptoken.py b/ipaserver/plugins/otptoken.py
index 56b8c91..39012e2 100644
--- a/ipaserver/plugins/otptoken.py
+++ b/ipaserver/plugins/otptoken.py
@@ -79,10 +79,6 @@ class OTPTokenKey(Bytes):
 
 password = True
 
-kwargs = Bytes.kwargs + (
-('confirm', bool, True),
-)
-
 def _convert_scalar(self, value, index=None):
 if isinstance(value, (tuple, list)) and len(value) == 2:
 (p1, p2) = value
-- 
2.7.4

-- 
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 0154] client: RPM require initscripts to get *-domainname.service

[Petr Spacek]

Hello,

client: RPM require initscripts to get *-domainname.service

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

-- 
Petr^2 Spacek
From b542e09b6d52b7ce22e47b6c08eb692b9f3b91b7 Mon Sep 17 00:00:00 2001
From: Petr Spacek 
Date: Mon, 8 Aug 2016 13:13:18 +0200
Subject: [PATCH] client: RPM require initscripts to get *-domainname.service

https://fedorahosted.org/freeipa/ticket/4831
---
 freeipa.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 135e9c980011c6c2730c6c29a3c22098e48270d5..06e35c25d2f8c8dd88c4a853953b86eac2fee113 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -325,6 +325,8 @@ Requires: krb5-workstation
 Requires: authconfig
 Requires: pam_krb5
 Requires: curl
+# NIS domain name config: /usr/lib/systemd/system/*-domainname.service
+Requires: initscripts
 Requires: libcurl >= 7.21.7-2
 Requires: xmlrpc-c >= 1.27.4
 Requires: sssd >= 1.14.0
-- 
2.7.4

-- 
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 0112-7] Speeding up cli help

[Jan Cholasta]

On 4.8.2016 16:32, David Kupka wrote:

On 03/08/16 16:33, Jan Cholasta wrote:

On 3.8.2016 16:23, David Kupka wrote:

On 21/07/16 10:12, Jan Cholasta wrote:

Hi,

On 20.7.2016 14:32, David Kupka wrote:

On 15/07/16 12:53, David Kupka wrote:

Hello!

After Honza introduced thin client that builds plugins and commands
dynamically from schema client became much slower. This is only
logical,
instead of importing a module client now must fetch the schema from
server, parse it and instantiate the commands using the data.

First step to speed it up was addition of schema cache to client.
That
removed the RTT and download time of fetching schema every time.

Now the most time consuming task became displaying help for lists of
topics and command and displaying individual topics. This is simply
because of the need to instantiate all the commands to find the
relations between topics and commands.

All the necessary bits for server commands and topics are already in
the
schema cache so we can skip this part and generate help from it,
right?
Not so fast!

There are client plugins with commands and topics. So we can generate
basic bits (list of all topics, list of all commands, list of
commands
for each topic) from schema and store it in cache. Then we need to go
through all client plugins and get similar bits for client plugins.
Then
we can merge and print.

Still the client response is not as fast as before and I this it even
can't be. Also first time you display particular topic or list takes
longer because it must be freshly generated and stored in cache for
next
use. And this is what the attached patches do.

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


Reimplemented so there is no need to distinguish client plugins and
remote plugins.
The main idea of this approach is to avoid creating instances of the
commands just to get the information about topic, name and summary
needed for displaying help. Instead class properties are used to
access
the information directly in schema.


Patch 0112:

I think this would better be done in Schema.read_namespace_member,
because Schema is where all the state is.

(BTW does _SchemaNameSpace.__getitem__ raise KeyError for non-existent
keys? It looks like it doesn't.)


Patch 0113:

How about setting _schema_cached to False in Schema.__init__() rather
that getattr()-ing it in _ensure_cached()?


Patch 0116:

ClientCommand.doc should be a class property as well, otherwise
.summary
won't work on it correctly.

_SchemaCommand.doc should not be a property, as it's not needed for
.summary to work on it correctly.


Otherwise works fine for me.

Honza



Updated patches attached.


Thanks, ACK.

Pushed to master: 229e2a1ed9ea9877cb5e879fadd99f9040f77c96



I've made and reviewer overlooked some errors. Attached patches fix them.


Shame on me.

Anyway,

1) When schema() raises SchemaUpToDate, the current code explodes:

ipa: ERROR: KeyError: 'fingerprint'
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/ipalib/cli.py", line 1348, in run
api.finalize()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 707, 
in finalize

self.__do_if_not_done('load_plugins')
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 422, 
in __do_if_not_done

getattr(self, name)()
  File "/usr/lib/python2.7/site-packages/ipalib/plugable.py", line 585, 
in load_plugins

for package in self.packages:
  File "/usr/lib/python2.7/site-packages/ipalib/__init__.py", line 919, 
in packages

ipaclient.remote_plugins.get_package(self),
  File 
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/__init__.py", 
line 92, in get_package

plugins = schema.get_package(api, server_info, client)
  File 
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py", 
line 558, in get_package

fingerprint = str(schema['fingerprint'])
  File 
"/usr/lib/python2.7/site-packages/ipaclient/remote_plugins/schema.py", 
line 483, in __getitem__

return self._dict[key]
KeyError: 'fingerprint'


2) We read server info every time get_package() is called. It should be 
cache similarly to how the schema is cached in the api object.



3) Some of the commands are still fully initialized during "ipa help 
commands".



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] [PATCH 0196] baseldap: Fix MidairCollision instantiation during entry modification

[thierry bordaz]

On 08/05/2016 01:33 PM, thierry bordaz wrote:



On 07/26/2016 05:22 PM, Alexander Bokovoy wrote:

On Tue, 26 Jul 2016, Martin Babinsky wrote:

Fix for https://fedorahosted.org/freeipa/ticket/6097

Since this issue was found during investigation of other ticket[1], 
you can test it by performing steps to reproduce #6041, but instead 
of internal error you should see the MidairCollision raised as 
public error with the right error message.


[1] https://fedorahosted.org/freeipa/ticket/6041

I have a preliminary patch for slapi-nis to fix 6041 (attached).


The slapi-nis patch looks good to me.
Ludwig may give the final ACK.

thanks
thierry


The fix for https://fedorahosted.org/freeipa/ticket/6041 is good.
ACK

thierry


As for this fix -- ACK.



--
Martin^3 Babinsky



From 8f0d6dab08f61fe2fd1ad64a63f7ab91fc5227d4 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 25 Jul 2016 14:05:08 +0200
Subject: [PATCH] baseldap: Fix MidairCollision instantiation during 
entry

modification

https://fedorahosted.org/freeipa/ticket/6097
---
ipaserver/plugins/baseldap.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/plugins/baseldap.py 
b/ipaserver/plugins/baseldap.py
index 
6107e43a6ee17d9b9a63d9dc109664d8b232069f..f7844e3e7c59c259b9c8367d135b2dbefc3f0016 
100644

--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -1466,7 +1466,7 @@ class LDAPUpdate(LDAPQuery, crud.Update):
entry_attrs.dn, attrs_list)
except errors.NotFound:
raise errors.MidairCollision(
-format=_('the entry was deleted while being modified')
+message=_('the entry was deleted while being 
modified')

)

self.obj.get_indirect_members(entry_attrs, attrs_list)
@@ -2344,7 +2344,7 @@ class BaseLDAPModAttribute(LDAPQuery):
entry_attrs.dn, attrs_list)
except errors.NotFound:
raise errors.MidairCollision(
-format=_('the entry was deleted while being modified')
+message=_('the entry was deleted while being 
modified')

)

for callback in self.get_callbacks('post'):
--
2.7.4




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











-- 
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] 0002 Added support for authentication with user certificate

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Martin Kosek wrote:

On 08/05/2016 02:57 PM, Tibor Dudlak wrote:

Hi,

I have extended my previous patch for authentication with user
certificate/smartcard. This patch includes patches and plugin described here:
http://www.freeipa.org/page/V4/External_Authentication/Setup
Page also contains steps to configure and test this feature. Once this patch is
merged and released we will simplify this page to not confuse customers.
Addressing ticket: https://fedorahosted.org/freeipa/ticket/5764

Thanks.


I discussed this with Jan Pazdziora on IRC, outside of this mail thread, so let
me repeat my suggestion here. I still think it is premature to add plugins like
that to FreeIPA core git. We are not agreed yet how we will distribute FreeIPA
plugins, so I would not rush adding this plugin to FreeIPA core, especially
since it is very experimental and not even secure yet. FreeIPA plugin
distribution should be more thought through and discussed.

As I proposed, this plugin can now live outside of FreeIPA core git, in it's
own life cycle (maybe in freeipa-plugins github git repo we create?) so that it
can be updated without updating whole FreeIPA core. In this effort, I would
suggest to only consider updates of

* ipaserver/plugins/xmlserver.py
* ipaserver/rpcserver.py

as these would have to patched by admin deploying this feature and would be
overwritten by RPM updates. The plugin itself or server.conf can be deployed
and installed separatenly, even via other RPM.

Right. This was my thinking too when I saw the patches.
--
/ 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 0214] Support schema files for external plugins

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Petr Vobornik wrote:

On 08/08/2016 12:26 PM, Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Alexander Bokovoy wrote:

Hi!

Attached patch is what is needed to allow external plugins for FreeIPA
framework to be functional if they need to extend a schema.

The idea is that we would have a separate directory as
/usr/share/ipa/schema.d and will allow to use schema (*.ldif) files from
it and its subdirectories during install and upgrade stages.

Without the patch only selected schema files from /usr/share/ipa are
used during install and upgrade. This leads to a failure to install IPA
server (or upgrade it) if a new plugin is added. If plugin defines
managed permissions, upgrade tool will generate ACIs which will fail to
be inserted into LDAP store due to references to missing attributes and
object classes.

The patch adds a directory to be installed and a helper utility that
loads files from the directory and adds them to the list of schema files
used during update of dsinstance and upgrade of the server.

With this patch I'm successfully managed to make FleetCommander
integration plugin completely independent of FreeIPA.

Patch attached now. ;)



I'll assume that we want to target 4.4.x therefore it can be pushed to
master, right? I.e. no need for creating ipa-4-4 branch atm.

Right.


Reasoning is that currently F24 has 4.3.x. F25 will most likely have
4.4.x because 4.5 is still in planning.

Sounds good to me. FleetCommander (which is one of drivers behind the
patches) is targeting F25 as well.

--
/ 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] 0002 Added support for authentication with user certificate

[Martin Kosek]

On 08/05/2016 02:57 PM, Tibor Dudlak wrote:
> Hi,
> 
> I have extended my previous patch for authentication with user 
> certificate/smartcard. This patch includes patches and plugin described here: 
> http://www.freeipa.org/page/V4/External_Authentication/Setup
> Page also contains steps to configure and test this feature. Once this patch 
> is 
> merged and released we will simplify this page to not confuse customers.
> Addressing ticket: https://fedorahosted.org/freeipa/ticket/5764
> 
> Thanks.

I discussed this with Jan Pazdziora on IRC, outside of this mail thread, so let
me repeat my suggestion here. I still think it is premature to add plugins like
that to FreeIPA core git. We are not agreed yet how we will distribute FreeIPA
plugins, so I would not rush adding this plugin to FreeIPA core, especially
since it is very experimental and not even secure yet. FreeIPA plugin
distribution should be more thought through and discussed.

As I proposed, this plugin can now live outside of FreeIPA core git, in it's
own life cycle (maybe in freeipa-plugins github git repo we create?) so that it
can be updated without updating whole FreeIPA core. In this effort, I would
suggest to only consider updates of

* ipaserver/plugins/xmlserver.py
* ipaserver/rpcserver.py

as these would have to patched by admin deploying this feature and would be
overwritten by RPM updates. The plugin itself or server.conf can be deployed
and installed separatenly, even via other RPM.

Martin

-- 
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 0214] Support schema files for external plugins

[Petr Vobornik]

On 08/08/2016 12:26 PM, Alexander Bokovoy wrote:
> On Mon, 08 Aug 2016, Alexander Bokovoy wrote:
>> Hi!
>>
>> Attached patch is what is needed to allow external plugins for FreeIPA
>> framework to be functional if they need to extend a schema.
>>
>> The idea is that we would have a separate directory as
>> /usr/share/ipa/schema.d and will allow to use schema (*.ldif) files from
>> it and its subdirectories during install and upgrade stages.
>>
>> Without the patch only selected schema files from /usr/share/ipa are
>> used during install and upgrade. This leads to a failure to install IPA
>> server (or upgrade it) if a new plugin is added. If plugin defines
>> managed permissions, upgrade tool will generate ACIs which will fail to
>> be inserted into LDAP store due to references to missing attributes and
>> object classes.
>>
>> The patch adds a directory to be installed and a helper utility that
>> loads files from the directory and adds them to the list of schema files
>> used during update of dsinstance and upgrade of the server.
>>
>> With this patch I'm successfully managed to make FleetCommander
>> integration plugin completely independent of FreeIPA.
> Patch attached now. ;)
> 

I'll assume that we want to target 4.4.x therefore it can be pushed to
master, right? I.e. no need for creating ipa-4-4 branch atm.

Reasoning is that currently F24 has 4.3.x. F25 will most likely have
4.4.x because 4.5 is still in planning.

-- 
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 0214] Support schema files for external plugins

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Petr Spacek wrote:

On 8.8.2016 11:34, Alexander Bokovoy wrote:

Hi!

Attached patch is what is needed to allow external plugins for FreeIPA
framework to be functional if they need to extend a schema.

The idea is that we would have a separate directory as
/usr/share/ipa/schema.d and will allow to use schema (*.ldif) files from
it and its subdirectories during install and upgrade stages.

Without the patch only selected schema files from /usr/share/ipa are
used during install and upgrade. This leads to a failure to install IPA
server (or upgrade it) if a new plugin is added. If plugin defines
managed permissions, upgrade tool will generate ACIs which will fail to
be inserted into LDAP store due to references to missing attributes and
object classes.

The patch adds a directory to be installed and a helper utility that
loads files from the directory and adds them to the list of schema files
used during update of dsinstance and upgrade of the server.

With this patch I'm successfully managed to make FleetCommander
integration plugin completely independent of FreeIPA.


1. I cannot see a patch attached to this e-mail :-)

See my other email. ;)


2. Needless to say that ticket in appropriate milestone is going to be required.

Sure. Moving ticket from one milestone to another is a simple act. I
wanted to show that it is actually an almost trivial patch to enable
external plugin development and argue by that fact we could have it
added, thus raising the ticket to a better milestone.

--
/ 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 0214] Support schema files for external plugins

[Petr Spacek]

On 8.8.2016 11:34, Alexander Bokovoy wrote:
> Hi!
> 
> Attached patch is what is needed to allow external plugins for FreeIPA
> framework to be functional if they need to extend a schema.
> 
> The idea is that we would have a separate directory as
> /usr/share/ipa/schema.d and will allow to use schema (*.ldif) files from
> it and its subdirectories during install and upgrade stages.
> 
> Without the patch only selected schema files from /usr/share/ipa are
> used during install and upgrade. This leads to a failure to install IPA
> server (or upgrade it) if a new plugin is added. If plugin defines
> managed permissions, upgrade tool will generate ACIs which will fail to
> be inserted into LDAP store due to references to missing attributes and
> object classes.
> 
> The patch adds a directory to be installed and a helper utility that
> loads files from the directory and adds them to the list of schema files
> used during update of dsinstance and upgrade of the server.
> 
> With this patch I'm successfully managed to make FleetCommander
> integration plugin completely independent of FreeIPA.

1. I cannot see a patch attached to this e-mail :-)

2. Needless to say that ticket in appropriate milestone is going to be required.

-- 
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 0214] Support schema files for external plugins

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Alexander Bokovoy wrote:

Hi!

Attached patch is what is needed to allow external plugins for FreeIPA
framework to be functional if they need to extend a schema.

The idea is that we would have a separate directory as
/usr/share/ipa/schema.d and will allow to use schema (*.ldif) files from
it and its subdirectories during install and upgrade stages.

Without the patch only selected schema files from /usr/share/ipa are
used during install and upgrade. This leads to a failure to install IPA
server (or upgrade it) if a new plugin is added. If plugin defines
managed permissions, upgrade tool will generate ACIs which will fail to
be inserted into LDAP store due to references to missing attributes and
object classes.

The patch adds a directory to be installed and a helper utility that
loads files from the directory and adds them to the list of schema files
used during update of dsinstance and upgrade of the server.

With this patch I'm successfully managed to make FleetCommander
integration plugin completely independent of FreeIPA.

Patch attached now. ;)

--
/ Alexander Bokovoy
From 045c7b38c387c362358d1ac2aa19a6fe07d18be5 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Fri, 5 Aug 2016 13:04:19 +0300
Subject: [PATCH 3/5] support schema files from third-party plugins

Allow upgrade process to include schema files from third-party plugins
installed in /usr/share/ipa/schema.d/*.schema.

The directory /usr/shar/eipa/schema.d is owned by the server-common
subpackage and therefore third-party plugins should depend on
freeipa-server-common (ipa-server-common) package in their package
dependencies.

Resolves: https://fedorahosted.org/freeipa/ticket/5864
---
 freeipa.spec.in |  5 -
 install/configure.ac|  1 +
 install/share/Makefile.am   |  1 +
 install/share/schema.d/Makefile.am  | 16 
 install/share/schema.d/README   | 14 ++
 ipaplatform/base/paths.py   |  1 +
 ipaserver/install/dsinstance.py | 15 ++-
 ipaserver/install/server/upgrade.py |  3 +++
 8 files changed, 54 insertions(+), 2 deletions(-)
 create mode 100644 install/share/schema.d/Makefile.am
 create mode 100644 install/share/schema.d/README

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 135e9c9..8acb3fc 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -871,6 +871,8 @@ mkdir -p %{buildroot}%{_sysconfdir}/cron.d
 
 mkdir -p %{buildroot}%{_sysconfdir}/ipa/custodia
 
+mkdir -p %{buildroot}%{_usr}/share/ipa/schema.d
+
 %endif # ONLY_CLIENT
 
 
@@ -1248,7 +1250,8 @@ fi
 %ghost %{_localstatedir}/lib/ipa/pki-ca/publish
 %ghost %{_localstatedir}/named/dyndb-ldap/ipa
 %dir %attr(0700,root,root) %{_sysconfdir}/ipa/custodia
-
+%dir %{_usr}/share/ipa/schema.d
+%attr(0644,root,root) %{_usr}/share/ipa/schema.d/README
 
 %files server-dns
 %defattr(-,root,root,-)
diff --git a/install/configure.ac b/install/configure.ac
index b5f77bf..81f17b9 100644
--- a/install/configure.ac
+++ b/install/configure.ac
@@ -88,6 +88,7 @@ AC_CONFIG_FILES([
 share/advise/Makefile
 share/advise/legacy/Makefile
 share/profiles/Makefile
+share/schema.d/Makefile
 ui/Makefile
 ui/css/Makefile
 ui/src/Makefile
diff --git a/install/share/Makefile.am b/install/share/Makefile.am
index cd1c164..d8845ee 100644
--- a/install/share/Makefile.am
+++ b/install/share/Makefile.am
@@ -3,6 +3,7 @@ NULL =
 SUBDIRS =  \
advise  \
profiles\
+   schema.d\
$(NULL)
 
 appdir = $(IPA_DATA_DIR)
diff --git a/install/share/schema.d/Makefile.am 
b/install/share/schema.d/Makefile.am
new file mode 100644
index 000..0fef87f
--- /dev/null
+++ b/install/share/schema.d/Makefile.am
@@ -0,0 +1,16 @@
+NULL =
+
+SUBDIRS =  \
+   $(NULL)
+
+appdir = $(IPA_DATA_DIR)/schema.d
+app_DATA = README  \
+   $(NULL)
+
+EXTRA_DIST =   \
+   $(app_DATA) \
+   $(NULL)
+
+MAINTAINERCLEANFILES = \
+   *~  \
+   Makefile.in
diff --git a/install/share/schema.d/README b/install/share/schema.d/README
new file mode 100644
index 000..19e3e68
--- /dev/null
+++ b/install/share/schema.d/README
@@ -0,0 +1,14 @@
+This directory is indended to store schema files for 3rd-party plugins.
+
+Each schema file should be named NN-description.ldif where NN is a number 
00..90.
+
+The schema files from this directory are merged together with the core IPA
+schema files during the run of ipa-server-upgrade utility. Therefore, they are
+also installed when upgrade happens within the process of ipa-server-install.
+
+The directory is installed as /usr/share/ipa/schema.d and is owned by a
+freeipa-server-common package. Therefore, a 3rd-party plugin would need to
+depend on the 

Re: [Freeipa-devel] certmonger proxy configuration not possible ?

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Marx, Peter wrote:

I am trying this but it has no effect - as if the environment is not passed to 
the called helper scep-submit.

In /usr/lib/systemd/certmonger.service there is already a link defined to add 
stuff:
[Service]
..
EnvironmentFile=/etc/sysconfig/certmonger

In /etc/sysconfig/certmonger I added my proxy like this:

[Service]
Environment="http_proxy=http://proxyuser:proxypassword@proxyserver:proxyport;

After systemctl daemon-reload and systemctl restart certmonger my
requests still do not get to the proxy.

Commenting out the EnvironmetFile line and adding the Environment line
directly in certmonger.service had the same result.

Can somebody confirm that the proxy setting is visible to the called
scep-submit ?

I've checked certmonger source code and while libcurl can be configured
to use proxy and proxy authentication, certmonger does not configure it
to do so. As result, environmental variables have no influence on the
use of libcurl by certmonger.

It is worth to open a ticket for certmonger to add proxy support.

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

[Freeipa-devel] [PATCH 0214] Support schema files for external plugins

[Alexander Bokovoy]

Hi!

Attached patch is what is needed to allow external plugins for FreeIPA
framework to be functional if they need to extend a schema.

The idea is that we would have a separate directory as
/usr/share/ipa/schema.d and will allow to use schema (*.ldif) files from
it and its subdirectories during install and upgrade stages.

Without the patch only selected schema files from /usr/share/ipa are
used during install and upgrade. This leads to a failure to install IPA
server (or upgrade it) if a new plugin is added. If plugin defines
managed permissions, upgrade tool will generate ACIs which will fail to
be inserted into LDAP store due to references to missing attributes and
object classes.

The patch adds a directory to be installed and a helper utility that
loads files from the directory and adds them to the list of schema files
used during update of dsinstance and upgrade of the server.

With this patch I'm successfully managed to make FleetCommander
integration plugin completely independent of FreeIPA.

--
/ 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] certmonger proxy configuration not possible ?

[Marx, Peter]

I am trying this but it has no effect - as if the environment is not passed to 
the called helper scep-submit.

In /usr/lib/systemd/certmonger.service there is already a link defined to add 
stuff:
[Service]
..
EnvironmentFile=/etc/sysconfig/certmonger

In /etc/sysconfig/certmonger I added my proxy like this:

[Service]
Environment="http_proxy=http://proxyuser:proxypassword@proxyserver:proxyport;

After systemctl daemon-reload and systemctl restart certmonger my requests 
still do not get to the proxy.

Commenting out the EnvironmetFile line and adding the Environment line directly 
in certmonger.service had the same result.

Can somebody confirm that the proxy setting is visible to the called 
scep-submit ? 


-Original Message-
From: Alexander Bokovoy [mailto:aboko...@redhat.com] 
Sent: Thursday, August 04, 2016 6:02 PM
To: Marx, Peter
Cc: Rob Crittenden; 'freeipa-devel@redhat.com'
Subject: Re: [Freeipa-devel] certmonger proxy configuration not possible ?

On Thu, 04 Aug 2016, Marx, Peter wrote:
>I tried it and found out it can't work this way - when issuing a CSR 
>with getcert, the parameters of this request are normally handed over 
>by getcert to the scep-submit helper. I see no way to intercept these 
>parameters  and pass them to the proxy-shellscript. Only the -u 
>paramter is known beforehand, as it is configured in the ca description 
>file or in the proxy shellscript itself.
On systemd-enabled systems certmonger runs as a service. You can affect the 
environment of the service by adding files ending in .conf in 
/etc/systemd/system/certmonger.service.d/

See systemd.service and systemd.unit man pages.

>
>Peter
>
>-Original Message-
>From: Rob Crittenden [mailto:rcrit...@redhat.com]
>Sent: Wednesday, August 03, 2016 3:52 PM
>To: Marx, Peter; 'freeipa-devel@redhat.com'
>Subject: Re: [Freeipa-devel] certmonger proxy configuration not possible ?
>
>Marx, Peter wrote:
>> Hi,
>>
>> i have to access an external PKI server with SCEP protocol through 
>> our corporate proxy.  On command line I can set the proxy and trigger 
>> a CSR with the scep-submit helper successfully.
>
>What are you setting, environment variables I assume?
>
>> But same operation with getcert fails, as there is no proxy 
>> configuration possibility in e.g. certmonger.conf.
>>
>> How can I work around this ?
>
>A quick kludge might be to replace scep-submit with a shell script that 
>exports the proxy config and then calls the real scep-submit.
>
>A perhaps better and more supportable idea would be to add a CA pointing to 
>this new helper, something like:
>
>getcert add-ca -c exampleSCEPca -e \
> "/usr/libexec/certmonger/scep-submit-proxy -u 
> http://ca.example.com/cgi-bin/pkiclient.exe;
>
>So scep-submit-proxy would setup the environment and call scep-submit.
>
>rob
>
>>
>> Peter
>>
>>
>>
>> Knorr-Bremse IT-Services GmbH
>> Sitz: München
>> Geschäftsführer: Helmut Draxler (Vorsitzender), Harald Jessen, Harald 
>> Schneider Registergericht München, HR B 167 268
>>
>> This transmission is intended solely for the addressee and contains 
>> confidential information.
>> If you are not the intended recipient, please immediately inform the 
>> sender and delete the message and any attachments from your system.
>> Furthermore, please do not copy the message or disclose the contents 
>> to anyone unless agreed otherwise. To the extent permitted by law we 
>> shall in no way be liable for any damages, whatever their nature, 
>> arising out of transmission failures, viruses, external influence, delays 
>> and the like.
>>
>>
>
>
>automechanika - 13.09.-17.09.2016 - Messe Frankfurt - Hall 3.0 - Stand 
>G98 + E91 InnoTrans - 20.09.-23.09.2016 - Messe Berlin - Hall 1.2b - 
>Stand 104 + 210 IAA - 22.09.-29.09.2016 - Messe Hannover - Hall 17 - 
>Stand A30 + D131
>
>Knorr-Bremse IT-Services GmbH
>Sitz: Muenchen
>Geschaeftsfuehrer: Helmut Draxler (Vorsitzender), Harald Jessen, Harald 
>Schneider Registergericht Muenchen, HR B 167 268
>
>This transmission is intended solely for the addressee and contains 
>confidential information.
>If you are not the intended recipient, please immediately inform the sender 
>and delete the message and any attachments from your system.
>Furthermore, please do not copy the message or disclose the contents to anyone 
>unless agreed otherwise. To the extent permitted by law we shall in no way be 
>liable for any damages, whatever their nature, arising out of transmission 
>failures, viruses, external influence, delays and the like.
>
>--
>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

--
/ Alexander Bokovoy

automechanika - 13.09.-17.09.2016 - Messe Frankfurt - Hall 3.0 - Stand G98 + E91
InnoTrans - 20.09.-23.09.2016 - Messe Berlin - Hall 1.2b - Stand 104 + 210
IAA - 22.09.-29.09.2016 - Messe Hannover - Hall 17 - Stand A30 + D131

Knorr-Bremse IT-Services 

Re: [Freeipa-devel] [PATCH 0213] support multiple uid values in slapi-nis users map

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Lukas Slebodnik wrote:

On (08/08/16 11:35), Alexander Bokovoy wrote:

On Mon, 08 Aug 2016, Martin Basti wrote:



On 08.08.2016 09:34, Alexander Bokovoy wrote:
> When SSSD resolves AD users on behalf of slapi-nis, it can accept any
> user identifier, including user principal name (UPN) which may be
> different than the canonical user name which SSSD returns.
>
> As result, the entry created by slapi-nis will be using canonical user
> name but the filter for search will refer to the original (aliased)
> name. The search will not match the newly created entry.
>
> The issue is fixed  in slapi-nis-0.56.1 by returning two values for
> 'uid' attribute: the canonical one and the aliased one. This way the
> search will match.
>
> Standard LDAP schema allows multiple values for 'uid' attribute. We
> actually use the same trick for 'cn' attribute in the groups map
> already.
>
> https://fedorahosted.org/freeipa/ticket/6138
>
>
>
>
Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?

No, this is not required. In Fedora we'll submit a combined update --
I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide already
but did not submit a Bodhi request.


How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
 dnf update freeipa-server.

An update file in FreeIPA that is proposed by this patch does not affect
operation of the older slapi-nis deployment once update is applied.

--
/ 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 0213] support multiple uid values in slapi-nis users map

[Lukas Slebodnik]

On (08/08/16 11:35), Alexander Bokovoy wrote:
>On Mon, 08 Aug 2016, Martin Basti wrote:
>> 
>> 
>> On 08.08.2016 09:34, Alexander Bokovoy wrote:
>> > When SSSD resolves AD users on behalf of slapi-nis, it can accept any
>> > user identifier, including user principal name (UPN) which may be
>> > different than the canonical user name which SSSD returns.
>> > 
>> > As result, the entry created by slapi-nis will be using canonical user
>> > name but the filter for search will refer to the original (aliased)
>> > name. The search will not match the newly created entry.
>> > 
>> > The issue is fixed  in slapi-nis-0.56.1 by returning two values for
>> > 'uid' attribute: the canonical one and the aliased one. This way the
>> > search will match.
>> > 
>> > Standard LDAP schema allows multiple values for 'uid' attribute. We
>> > actually use the same trick for 'cn' attribute in the groups map
>> > already.
>> > 
>> > https://fedorahosted.org/freeipa/ticket/6138
>> > 
>> > 
>> > 
>> > 
>> Hello,
>> 
>> should we bump requires to slapi-nis-0.56.1 in freeipa.spec?
>No, this is not required. In Fedora we'll submit a combined update --
>I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide already
>but did not submit a Bodhi request.
>
How is combined updated related to requires to slapi-nis-0.56.1?
It will not prevent tu update freeipa without new slapi-nis.

e.g.
  dnf update freeipa-server.

LS

-- 
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] 0097 Add options to write lightweight CA cert or chain to file

[Jan Cholasta]

On 8.8.2016 09:06, Fraser Tweedale wrote:

On Mon, Aug 08, 2016 at 08:54:05AM +0200, Jan Cholasta wrote:

Hi,

On 8.8.2016 06:34, Fraser Tweedale wrote:

Please review the attached patch with adds --certificate-out and
--certificate-chain-out options to `ca-show' command.

Note that --certificate-chain-out currently writes a bogus file due
to a bug in Dogtag that will be fixed in this week's build.

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


1) The client-side *-out options should be defined on the client side, not
on the server side.


Will option defined on client side be propagated to, and observable
in the ipaserver plugin?  The ipaserver plugin needs to observe that
*-out has been requested and executes additional command(s) on that
basis.


Is there a reason not to *always* return the certs?





2) I don't think there should be additional information included in summary
(and it definitely should not be multi-line). I would rather inform the user
via an error message when unable to write the files.


I was just following the pattern of other commands that write certs,
profile config, etc.  Apart from consistency with other commands I
agree that there is no need to have it.  So I will remove it.


If you think there is an actual value in informing the user about
successfully writing the files, please use ipalib.messages for the job.


3) IMO a better format for the certificate chain than PKCS#7 would be
concatenated PEM, as that's the most commonly used format in IPA (in
installers, there are no cert chains in API commands ATM).


Sure, but the main use case isn't IPA.  Other apps require PKCS #7
or concatenated PEMs, but sometimes they must be concatenated
forward, and othertimes backwards.  There is no one size fits all.


True, which is exactly why I think we should at least be self-consistent 
and use concatenated PEM (and multi-value DER over the wire).



We can add an option to control the format later, but for now,
Dogtag returns a PKCS #7 (PEM or DER) so let's go with that.  Worst
case is an admin has to invoke `openssl pkcs7' and concat the certs
themselves.


AFAIK none of NSS, OpenSSL or p11-kit can use PKCS#7 cert chains 
directly, so I'm afraid the worst case would happen virtually always.






4) Over the wire, the certs should be DER-formatted, as that's the most
common wire format in other API commands.


OK.



5) What is the benefit in having the CA cert and the rest of the chain
separate? For end-entity certs it makes sense to separate the cert from the
CA chain, but for CA certs, you usually want the full chain, no?


If you want to anchor trust directly at a subca (e.g. restrict VPN
login to certs issued by VPN sub-CA) then you often just want the
cert.  The chain option does subsume it, at cost of more work for
administrators with this use case.  I think it makes sense to keep
both options.


Does it? From what you described above, you either want just the sub-CA 
cert, or the full chain including the sub-CA cert, in which case it 
might make more sense to have a single --out option and a --chain flag.




Will cut a new patch tomorrow.

Thanks,
Fraser




--
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 0213] support multiple uid values in slapi-nis users map

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Martin Basti wrote:



On 08.08.2016 09:34, Alexander Bokovoy wrote:

When SSSD resolves AD users on behalf of slapi-nis, it can accept any
user identifier, including user principal name (UPN) which may be
different than the canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using canonical user
name but the filter for search will refer to the original (aliased)
name. The search will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two values for
'uid' attribute: the canonical one and the aliased one. This way the
search will match.

Standard LDAP schema allows multiple values for 'uid' attribute. We
actually use the same trick for 'cn' attribute in the groups map
already.

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





Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?

No, this is not required. In Fedora we'll submit a combined update --
I've built slapi-nis-0.56.1-1 packages for f24, f25, and rawhide already
but did not submit a Bodhi request.

--
/ 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 0003] Test validity of URIs in certificate

[Martin Basti]

On 02.08.2016 14:50, Lenka Doudova wrote:




On 07/29/2016 11:43 AM, Lenka Doudova wrote:




On 07/29/2016 11:41 AM, Lenka Doudova wrote:


On 07/28/2016 01:35 PM, Peter Lacko wrote:

Hops, fixed.

Peter


- Original Message -
From: "Lenka Doudova"
To:freeipa-devel@redhat.com
Sent: Thursday, July 28, 2016 1:32:25 PM
Subject: Re: [Freeipa-devel] [PATCH 0003] Test validity of URIs in  
certificate

Hi,

I cannot find any attached patch :)

Lenka


On 07/28/2016 01:30 PM, Peter Lacko wrote:

Attached you can find a patch adding test for URIs in generated certificate
ipatests/test_xmlrpc/test_cert_plugin.py
Since I'm leaving Red Hat in end of July, I won't be able to modify this patch 
anymore.

Regards,

Peter





Hi,

NACK. Code looks fine and works well on master branch, but patch 
does not apply on 4-3 and 4-2 branches.
Peter left the company but claimed he can fix the patch if 
necessary, I'll communicate it with him or fix it myself.


Lenka



Oh, and forgot this one - PEP8 error:
./ipatests/test_xmlrpc/test_cert_plugin.py:191:80: E501 line too long 
(105 > 79 characters)


Lenka



Hi,

since Peter has quit already, I took it upon myself to do minor fix 
and rebase to the patch.
1) i removed pylint disable comments from the patch, as they were 
unnecessary (this also solved PEP8 error)

2) I rebased the patch to be applicable for ipa-4-3 branch.
Original functionality of the patch remains unchanged.

Both fixed patches attached.

Lenka




Hello,

1)
This is not needed

+global sn
+
+result = api.Command.cert_show(sn, out=unicode(self.certfile))

you need the global statement only for write access. But sn is not assigned in 
this code block.

2)
I prefer to use instance attributes (self.sn) instead of global variables


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 0213] support multiple uid values in slapi-nis users map

[Martin Basti]

On 08.08.2016 09:34, Alexander Bokovoy wrote:

When SSSD resolves AD users on behalf of slapi-nis, it can accept any
user identifier, including user principal name (UPN) which may be
different than the canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using canonical user
name but the filter for search will refer to the original (aliased)
name. The search will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two values for
'uid' attribute: the canonical one and the aliased one. This way the
search will match.

Standard LDAP schema allows multiple values for 'uid' attribute. We
actually use the same trick for 'cn' attribute in the groups map
already.

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





Hello,

should we bump requires to slapi-nis-0.56.1 in freeipa.spec?

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] [PATCH 0213] support multiple uid values in slapi-nis users map

[Alexander Bokovoy]

When SSSD resolves AD users on behalf of slapi-nis, it can accept any
user identifier, including user principal name (UPN) which may be
different than the canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using canonical user
name but the filter for search will refer to the original (aliased)
name. The search will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two values for
'uid' attribute: the canonical one and the aliased one. This way the
search will match.

Standard LDAP schema allows multiple values for 'uid' attribute. We
actually use the same trick for 'cn' attribute in the groups map
already.

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


--
/ Alexander Bokovoy
From 359328c45465c25a2c34629511bf30097b0b8b0a Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy 
Date: Thu, 4 Aug 2016 09:58:50 +0300
Subject: support multiple uid values in schema compatibility tree

When SSSD resolves AD users on behalf of slapi-nis, it can accept any user
identifier, including user principal name (UPN) which may be different than the
canonical user name which SSSD returns.

As result, the entry created by slapi-nis will be using canonical user name but
the filter for search will refer to the original (aliased) name. The search
will not match the newly created entry.

The issue is fixed  in slapi-nis-0.56.1 by returning two values for 'uid'
attribute: the canonical one and the aliased one. This way the search will
match.

Standard LDAP schema allows multiple values for 'uid' attribute.

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

---
 install/updates/10-schema_compat.update | 4 
 1 file changed, 4 insertions(+)

diff --git a/install/updates/10-schema_compat.update 
b/install/updates/10-schema_compat.update
index e4c257d..fbe8703 100644
--- a/install/updates/10-schema_compat.update
+++ b/install/updates/10-schema_compat.update
@@ -87,3 +87,7 @@ add:schema-compat-entry-attribute: 
%ifeq("ipauniqueid","%{ipauniqueid}","objectc
 add:schema-compat-entry-attribute: 
%ifeq("ipauniqueid","%{ipauniqueid}","ipaanchoruuid=:IPA:$DOMAIN:%{ipauniqueid}","")
 add:schema-compat-entry-attribute: ipaanchoruuid=%{ipaanchoruuid}
 add:schema-compat-entry-attribute: 
%ifeq("ipaanchoruuid","%{ipaanchoruuid}","objectclass=ipaOverrideTarget","")
+
+dn: cn=users,cn=Schema Compatibility,cn=plugins,cn=config
+add:schema-compat-entry-attribute: uid=%{uid}
+replace:schema-compat-entry-rdn: uid=%{uid}::uid=%first("%{uid}")
-- 
2.7.4

-- 
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] 0097 Add options to write lightweight CA cert or chain to file

[Alexander Bokovoy]

On Mon, 08 Aug 2016, Fraser Tweedale wrote:

On Mon, Aug 08, 2016 at 08:54:05AM +0200, Jan Cholasta wrote:

Hi,

On 8.8.2016 06:34, Fraser Tweedale wrote:
> Please review the attached patch with adds --certificate-out and
> --certificate-chain-out options to `ca-show' command.
>
> Note that --certificate-chain-out currently writes a bogus file due
> to a bug in Dogtag that will be fixed in this week's build.
>
> https://fedorahosted.org/freeipa/ticket/6178

1) The client-side *-out options should be defined on the client side, not
on the server side.


Will option defined on client side be propagated to, and observable
in the ipaserver plugin?  The ipaserver plugin needs to observe that
*-out has been requested and executes additional command(s) on that
basis.

You define Str() 'out' option on the server side -- this is what
server side will see if --out option would be specified on the client
side. Then on the client side you would override forward() method and
check if 'out' is in options, then do write to the file.


--
/ 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] 0097 Add options to write lightweight CA cert or chain to file

[Fraser Tweedale]

On Mon, Aug 08, 2016 at 08:54:05AM +0200, Jan Cholasta wrote:
> Hi,
> 
> On 8.8.2016 06:34, Fraser Tweedale wrote:
> > Please review the attached patch with adds --certificate-out and
> > --certificate-chain-out options to `ca-show' command.
> > 
> > Note that --certificate-chain-out currently writes a bogus file due
> > to a bug in Dogtag that will be fixed in this week's build.
> > 
> > https://fedorahosted.org/freeipa/ticket/6178
> 
> 1) The client-side *-out options should be defined on the client side, not
> on the server side.
> 
Will option defined on client side be propagated to, and observable
in the ipaserver plugin?  The ipaserver plugin needs to observe that
*-out has been requested and executes additional command(s) on that
basis.

> 
> 2) I don't think there should be additional information included in summary
> (and it definitely should not be multi-line). I would rather inform the user
> via an error message when unable to write the files.
> 
I was just following the pattern of other commands that write certs,
profile config, etc.  Apart from consistency with other commands I
agree that there is no need to have it.  So I will remove it.

> If you think there is an actual value in informing the user about
> successfully writing the files, please use ipalib.messages for the job.
> 
> 
> 3) IMO a better format for the certificate chain than PKCS#7 would be
> concatenated PEM, as that's the most commonly used format in IPA (in
> installers, there are no cert chains in API commands ATM).
> 
Sure, but the main use case isn't IPA.  Other apps require PKCS #7
or concatenated PEMs, but sometimes they must be concatenated
forward, and othertimes backwards.  There is no one size fits all.
We can add an option to control the format later, but for now,
Dogtag returns a PKCS #7 (PEM or DER) so let's go with that.  Worst
case is an admin has to invoke `openssl pkcs7' and concat the certs
themselves.

> 
> 4) Over the wire, the certs should be DER-formatted, as that's the most
> common wire format in other API commands.
> 
OK.

> 
> 5) What is the benefit in having the CA cert and the rest of the chain
> separate? For end-entity certs it makes sense to separate the cert from the
> CA chain, but for CA certs, you usually want the full chain, no?
> 
If you want to anchor trust directly at a subca (e.g. restrict VPN
login to certs issued by VPN sub-CA) then you often just want the
cert.  The chain option does subsume it, at cost of more work for
administrators with this use case.  I think it makes sense to keep
both options.

Will cut a new patch tomorrow.

Thanks,
Fraser

-- 
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] 0097 Add options to write lightweight CA cert or chain to file

[Jan Cholasta]

Hi,

On 8.8.2016 06:34, Fraser Tweedale wrote:

Please review the attached patch with adds --certificate-out and
--certificate-chain-out options to `ca-show' command.

Note that --certificate-chain-out currently writes a bogus file due
to a bug in Dogtag that will be fixed in this week's build.

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


1) The client-side *-out options should be defined on the client side, 
not on the server side.



2) I don't think there should be additional information included in 
summary (and it definitely should not be multi-line). I would rather 
inform the user via an error message when unable to write the files.


If you think there is an actual value in informing the user about 
successfully writing the files, please use ipalib.messages for the job.



3) IMO a better format for the certificate chain than PKCS#7 would be 
concatenated PEM, as that's the most commonly used format in IPA (in 
installers, there are no cert chains in API commands ATM).



4) Over the wire, the certs should be DER-formatted, as that's the most 
common wire format in other API commands.



5) What is the benefit in having the CA cert and the rest of the chain 
separate? For end-entity certs it makes sense to separate the cert from 
the CA chain, but for CA certs, you usually want the full chain, no?




Thanks,
Fraser





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] [PATCH] 0002 New User Role Tests

[Lenka Doudova]

On 07/20/2016 05:31 PM, Peter Lacko wrote:

Sorry for late reply, I was waiting how the discussion with tracker improvement 
will end, but since there's no progress and I'm leaving soon, I'm attaching new 
patch. I also created mapping between old and new tests [1], to make life of 
reviewer easier. Number in first column denotes line number in original file, 
followed by line number in new tests file and its name. Tests that are not 
mentioned in there extend previous coverage.

Regards,
Peter

[1] http://etherpad.corp.redhat.com/Vk3tRLaPSK


Hi,

review notes:

1) code related:

 * leftover PEP8 error:
   ./ipatests/test_xmlrpc/test_role_plugin.py:696:1: W391 blank line at
   end of file
 * in PrivilegeTracker:
 o creating privilege with description does not work properly,
   since there's a hardcoded value in track_create method
 o the check_retrieve method expects 'description' to be returned
   only when privilege is member of a role, but to the extent of my
   knowledge that value is returned always
 * in RoleTracker:
 o global variable 'member_types' is used only inside the class, so
   it should be a class variable rather than global
 o 'check_add_duplicate_member' method uses oneliner to create
   basis of expected value - suggest to use this more in other
   methods that do it 'literally' (e.g. check_add_member)
 o 'update_tracker' method - the main else statement should be
   investigated more, I'm not sure the try-except part is valid (if
   I expect the tracker to delete an attribute, but that attribute
   is not present, it's a pass? Doesn't sound right.)
 o 'update' method is needlessly simple - look at the same method
   in BaseTracker, which contains more method calls. Result of the
   simplicity is that in actual role tests, these other methods
   like 'check_update' etc. are called outside the 'update' method,
   when they can just as well be part of the method and save
   repeating same code over and over. This goes for 'retrieve' and
   'find' methods as well.
 * in role tests (ipatests/test_xmlrpc/test_role_plugin.py):
 o in class TestDeleredRole, there's unused method setUpClass

2) other:

 * I strongly suggest to go through all documentation comments, since
   some of them are vague, or even straight misleading (e.g. in
   RoleTracker, method 'find_tracker_roles': comment says, that it
   performs find command, but nothing like that happens!)
 * similarly as in previous note, please go through all parameter
   descriptions in the documentation - e.g. in RoleTracker, method
   'update_tracker': ":param expected_updates: Dictionary of other
   attributes" - such description is quite unsatisfactory)
 * when doing previous two, there are some typos that could be eliminated
 * some test cases in ipatests/test_xmlrpc/test_role_plugin.py seem
   incorrectly classified, e.g. method 'test_create_invalid' verifying
   attempt to create role with invalid name in TestNonexistentRole
   class that performes operations like role-show, role-delete on
   nonexistent entries
 * for future patches, it might be nice to have separate patches for
   each tracker and the tests

Also, since the author Peter Lacko left the company, fixing this patch 
will be a task for Ganna.


Lenka





- Original Message -
From: "Martin Basti" 
To: "Peter Lacko" 
Cc: freeipa-devel@redhat.com
Sent: Monday, June 6, 2016 12:29:29 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 02.06.2016 16:16, Peter Lacko wrote:

Rebased with updated tests.

Peter

- Original Message -
From: "Martin Basti" 
To: "Peter Lacko" 
Cc: freeipa-devel@redhat.com
Sent: Thursday, June 2, 2016 1:50:06 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests

Your patch doesn't apply anymore on master, there were changes in role
test and patch have to be updated and rebased

Please look at this for new changes in test_role_plugin.py
https://git.fedorahosted.org/cgit/freeipa.git/commit/?id=5f42b42bd4557a669ab5cfcf1af6596f1a2535f1

Martin^2


On 02.06.2016 11:49, Peter Lacko wrote:

Sorry for late response, I wasn't working these days.
Fixed patch is in attachment.

Peter


- Original Message -
From: "Martin Basti" 
To: "Peter Lacko" , freeipa-devel@redhat.com
Sent: Monday, May 9, 2016 1:06:08 PM
Subject: Re: [Freeipa-devel] [PATCH] 0002 New User Role Tests



On 09.05.2016 13:04, Martin Basti wrote:

On 09.05.2016 12:19, Peter Lacko wrote:

+# pylint: disable=unicode-builtin

I'm not doing complete review, just the line above hit my eyes.

unicode() is not in Py3 because all strings there are unicode, thus
you cannot use it directly, you need something like

if six.PY2:
   str = unicode

and use str() everywhere and remove that #pylint line

FYI all enabled pylint checks are there for a good