Re: [Freeipa-devel] Certificate Identity Mapping

2017-01-08 Thread Jan Cholasta

On 6.1.2017 10:48, Sumit Bose wrote:

On Fri, Jan 06, 2017 at 08:40:31AM +0100, Jan Cholasta wrote:

On 5.1.2017 13:15, Sumit Bose wrote:

On Mon, Jan 02, 2017 at 08:06:04AM +0100, Jan Cholasta wrote:

On 19.12.2016 12:13, Sumit Bose wrote:

On Mon, Dec 19, 2016 at 10:02:58AM +0100, Jan Cholasta wrote:

I agree with *almost* everything Sumit said. See my inline comments below.

On 16.12.2016 11:53, Sumit Bose wrote:

On Tue, Dec 06, 2016 at 04:39:10PM +0100, Florence Blanc-Renaud wrote:

Hi,

I have started a feature description for the Certificate Identity Mapping at
the following location:
http://www.freeipa.org/page/V4/Certificate_Identity_Mapping

This is a first step, focusing on the interface we would like to provide. It
still contains open questions, some of which are linked to the corresponding
design on SSSD side:
https://fedorahosted.org/sssd/wiki/DesignDocs/MatchingAndMappingCertificates
https://fedorahosted.org/sssd/wiki/DesignDocs/SmartcardsAndMultipleIdentities

Comments, concerns and suggestions are welcome. Thanks!


Hi Flo,

thank you very much for setting up the page.

My comments are mostly about the commands.

certmappingconfig-mod:

* --enable=Boolean: if this option is 'False' SSSD will basically show
  the current behavior and just look up the certificates directly. But I
  wonder if the option is needed at all because not adding any mapping
  rules would have the same effect.

  What is the scope here, only the IPA domain, or all trusted domains as
  well? If it is for trusted domains as well will the certmappingrule-*
  commands and user-{add/remove}-certmapping return an error?

  So, in general I see an overlap with the mapping rules and I think it
  would be clearer to drop this option and do the lookups according to
  the mapping rules.

* --prompt-username=Boolean: the description implies that this option is
  synonymous to 1:1 mapping, but it is not. On Linux authentication in
  most cases use a user name either by directly asking (e.g. /bin/login)
  or using the current user name (e.g. sudo). So, according to its name
  it would only control if gdm is allowed to ask for an (optional) user
  name.

  If the option is renamed to e.g. --force-1-to-1-mapping to really
  enforce a 1:1 mapping then it would make sense to derived to gdm
  behavior. I.e. if 1:1 mapping is enforce it makes no sense for gdm to
  ask for a user name and if it is not enforced then it makes sense to
  offer and optional user name input field.

* --enable-username-mismatch=Boolean: I think this option can be
  dropped. My test so far show that if a non-matching hint is given on a
  Windows client authentication fails.

* --alternate-attribute=STRING: I think this option isn't needed as
  well. For IPA server-side we should decide on an attribute name and
  add it to the schema for user objects. On the client side the
  attribute name can be taken from the mapping rule.A


certmappingrule.*:

* ISSUERDN: it looks like you want to use issuerName here. In
  certificateRecord it it used with LDAP ordering and I would prefer
  LDAP ordering at all points where we have a choice. Unfortunately in the
  issuer-subject mapping AD dictates X.500 ordering.


LDAP ordering should indeed be preferred, as it is used everywhere else in
IPA. We can convert to/from X.500 ordering where necessary, when possible.



* DOMAINDN: does this refer to the nsslapd-certmap-basedn attribute in
  the example? My intention in the SSSD design-page was to specify the
  domain (as in DNS domain/IPA domain/trusted domain) where the matching
  user should be searched. Different domains might certificates from
  different issuers and some domains might not even use certificates.
  With this information SSSD does not have to search any domain trusted
  by IPA from a given certificate, but look only at domains listed here
  (the attribute should be a multi-value one).

  There are objects in the LDAP tree for each trusted domain which are
  used by SSSD so using a DN syntax would be valid here.


We use domain names rather than DNs to refer to domains everywhere else in
the framework. I don't think this place should be an exception.


I'm fine with domain names as well. In fact I didn't thought of using
DNs for this before I read DOMAINDN on the design page.





* LDAPSEARCHFILTER: I think a separate option is not need. LDAP search
  filters should just be a special kind of mapping rules. I can image in
  syntax like: . I
  think the difficult part with the LDAP filters will to define sensible
  templates.


I'm not sure I understand. Could you please elaborate a little bit?


A LDAP search filter which would cover the AD behavior would look like:

(|(altSecurityIdentities=%A%B)(userPrincipalName=%C)(samAccountName=%D))

where

%A: must be replaced with the issuer of the certificate in X.500 order
%B: must be replaced with the subject of the certificate in X.500 order

it would be possible 

Re: [Freeipa-devel] [RFC] Matching and Mapping Certificates

2017-01-08 Thread Jan Cholasta

On 6.1.2017 10:30, Sumit Bose wrote:

On Fri, Jan 06, 2017 at 08:50:14AM +0100, Jan Cholasta wrote:

On 5.1.2017 10:39, Sumit Bose wrote:

On Mon, Jan 02, 2017 at 09:18:47AM +0100, Jan Cholasta wrote:

On 18.10.2016 07:34, Jan Cholasta wrote:

On 17.10.2016 16:50, Rob Crittenden wrote:

Jan Cholasta wrote:

Hi,

On 13.10.2016 18:52, Sumit Bose wrote:

= Issuer specific matching =
Although the MIT Kerberos rules allow to select the issuer of a
certificate there are use cases where a more specific selection is
needed. E.g. if there are some default matching rules for all issuers
and some other issuer specific rules where the default rules should
not apply. To make this possible with the above scheme the default
rules must have an  clause which matches all but the issuer
with the specific rules. Writing regular-expressions to not match a
specific string or a list of strings is at least error-prone if not
impossible.

To make it easier to define issuer specific rules and default rules at
the same time and optional issuer string can be added to the rule to
indicate that for the given issuer only those rules should be
considered. Given the use-case I think it is acceptable to require
that the full issuer must be specified here in LDAP order (see below)
and case-sensitive matching is used.


This could also be solved by adding priority to rules - if two rules
match, the one with higher priority (the issuer specific rule) is
preferred over the one with lower priority (the default rule). IMO this
is better than an optional issuer string as it offers greater
flexibility.


The use cases I've seen haven't had to do with priority, though that
would be a nice enhancement, but with only allowing certificates issued
by a specific CA to be allowed (this is pretty common in web servers).
Being able to say "only do the matching on certificates issued by foo"
is valuable.


Sure, I'm not suggesting that matching by issuer should be removed, only
that rule precedence should not be determined by the issuer field setting.



Bump. Sumit, what is your opinion on this?


I'm fine with an optional(?) priority as well. Since priorities are
already used in the pwpolicies this should be already known to the
experienced admin. I guess we just have stick with "A lower value
indicates a higher priority" to not confuse users. That's why I think
that the priority should be optional here and a missing value indicates
the lowest priority (default rules).


In pwpolicy and sudorule, the priority is required and has to be unique.
Maybe we should do this for certificate mapping rules as well?


I think there is no requirement that only a single rule should match
hence I think the priority here must not be unique.


Is there a requirement that it must be possible for multiple rules to 
match? If not, we could begin with a simpler (for admin) solution with 
unique priorities and lift the restriction later when/if it becomes 
necessary. But I don't have a strong opinion on this.








Are you thinking of using the CoS scheme here as well would a priority
attribute be sufficient because we do not want to reference internal
objects in the mapping rules?


I'm not sure how CoS would be helpful here, I think a simple interger
attribute would be perfectly sufficient.


I agree.

bye,
Sumit





bye,
Sumit



--
Jan Cholasta



--
Jan Cholasta



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


[Freeipa-devel] [freeipa PR#377][opened] dogtaginstance: track server certificate with our renew agent

2017-01-08 Thread HonzaCholasta
   URL: https://github.com/freeipa/freeipa/pull/377
Author: HonzaCholasta
 Title: #377: dogtaginstance: track server certificate with our renew agent
Action: opened

PR body:
"""
This patchset is intended to make @simo5's life easier when changing the RA 
agent certificate location in #314.

**renew agent: handle non-replicated certificates**

In addition to replicated certificates (Dogtag certificates, RA
certificate), handle non-replicated certificates in
dogtag-ipa-ca-renew-agent as well.

**dogtaginstance: track server certificate with our renew agent**

Track Dogtag's server certificate with dogtag-ipa-ca-renew-agent instead of
dogtag-ipa-renew-agent.

**cainstance: do not configure renewal guard**

Do not configure renewal guard for dogtag-ipa-renew-agent, as it is not
used in IPA anymore.

https://fedorahosted.org/freeipa/ticket/5959
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/377/head:pr377
git checkout pr377
From 65d5593e9f8ed014d2a022c8dc25c1fca8a3d622 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 6 Jan 2017 10:45:38 +0100
Subject: [PATCH 1/3] renew agent: handle non-replicated certificates

In addition to replicated certificates (Dogtag certificates, RA
certificate), handle non-replicated certificates in
dogtag-ipa-ca-renew-agent as well.

https://fedorahosted.org/freeipa/ticket/5959
---
 .../certmonger/dogtag-ipa-ca-renew-agent-submit| 25 ++
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index 2e137ad..cb8f93b 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -108,6 +108,15 @@ def is_renewable():
 return x509.is_self_signed(cert) or is_lightweight_ca()
 
 
+def is_replicated():
+return not get_nickname()
+
+
+def is_renewal_master():
+ca = cainstance.CAInstance(host_name=api.env.host)
+return ca.is_renewal_master()
+
+
 @contextlib.contextmanager
 def ldap_connect():
 conn = None
@@ -447,10 +456,8 @@ def renew_ca_cert():
 if operation == 'SUBMIT':
 state = 'retrieve'
 
-if is_renewable():
-ca = cainstance.CAInstance(host_name=api.env.host)
-if ca.is_renewal_master():
-state = 'request'
+if is_renewable() and is_renewal_master():
+state = 'request'
 elif operation == 'POLL':
 cookie = os.environ.get('CERTMONGER_CA_COOKIE')
 if not cookie:
@@ -506,14 +513,14 @@ def main():
 certs.renewal_lock.acquire()
 try:
 profile = os.environ.get('CERTMONGER_CA_PROFILE')
-if profile:
-handler = handlers.get(profile, request_and_store_cert)
-else:
-ca = cainstance.CAInstance(host_name=api.env.host)
-if ca.is_renewal_master():
+if is_replicated():
+if profile or is_renewal_master():
 handler = request_and_store_cert
 else:
 handler = retrieve_cert_continuous
+else:
+handler = request_cert
+handler = handlers.get(profile, handler)
 
 res = call_handler(handler)
 for item in res[1:]:

From 8e079d1e1eb88cac5cfcc421735077ee8f178dd3 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Fri, 6 Jan 2017 10:47:02 +0100
Subject: [PATCH 2/3] dogtaginstance: track server certificate with our renew
 agent

Track Dogtag's server certificate with dogtag-ipa-ca-renew-agent instead of
dogtag-ipa-renew-agent.

https://fedorahosted.org/freeipa/ticket/5959
---
 ipaserver/install/dogtaginstance.py | 2 +-
 ipaserver/install/server/upgrade.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/dogtaginstance.py b/ipaserver/install/dogtaginstance.py
index c3c470d..4c02d73 100644
--- a/ipaserver/install/dogtaginstance.py
+++ b/ipaserver/install/dogtaginstance.py
@@ -325,7 +325,7 @@ def track_servercert(self):
 pin = self.__get_pin()
 try:
 certmonger.dogtag_start_tracking(
-ca='dogtag-ipa-renew-agent',
+ca='dogtag-ipa-ca-renew-agent',
 nickname=self.server_cert_name,
 pin=pin,
 pinfile=None,
diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py
index 5d8e596..10f2e3d 100644
--- a/ipaserver/install/server/upgrade.py
+++ b/ipaserver/install/server/upgrade.py
@@ -917,7 +917,7 @@ def certificate_renewal_update(ca, ds, http):
 dirsrv_dir = dsinstance.config_dirname(serverid)
 
 # bump version when requests is changed
-version = 5
+version = 6
 requests = (
 (
 paths.PKI_TOMCAT_ALIAS_DIR,
@@ -962,7 +962,7 @@ def certificate_renewal_update(ca, ds, http):
 (
 

[Freeipa-devel] [freeipa PR#181][comment] Tests : User Tracker creation of user with minimal values

2017-01-08 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/181
Title: #181: Tests : User Tracker creation of user with minimal values

stlaz commented:
"""
Thank you for the changes!
"""

See the full comment at 
https://github.com/freeipa/freeipa/pull/181#issuecomment-271222754
-- 
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] [freeipa PR#181][+ack] Tests : User Tracker creation of user with minimal values

2017-01-08 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/181
Title: #181: Tests : User Tracker creation of user with minimal values

Label: +ack
-- 
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] [freeipa PR#210][+ack] Tests: Stage User Tracker implementation

2017-01-08 Thread stlaz
  URL: https://github.com/freeipa/freeipa/pull/210
Title: #210: Tests: Stage User Tracker implementation

Label: +ack
-- 
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