Re: [Freeipa-devel] [PATCH] 0072..0075 Lightweight CA renewal

2016-06-30 Thread Jan Cholasta 

On 29.6.2016 10:41, Fraser Tweedale wrote:

On Wed, Jun 29, 2016 at 09:30:17AM +0200, Jan Cholasta wrote:

On 29.6.2016 08:55, Jan Cholasta wrote:

On 24.6.2016 08:49, Fraser Tweedale wrote:

On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:

Hi,

On 21.6.2016 08:24, Fraser Tweedale wrote:

The attached patches add lightweight CA renewal.  There are two
substantive aspects:

1. The renew_ca_cert updates the serial number in the lightweight
CA's entry in the Dogtag database.  This causes CA clones to observe
the renewal and update the certs in their own NSSDBs.

2. The ipa-certupdate command adds Certmonger tracking requests for
lightweight CAs (on the renewal master only).

Correct behaviour also depends on my patch 0069 (in-server API for
renew_ca_cert script).


Patch 0072-0074: LGTM

Patch 0075:

1) Lightweight CA certs should be tracked by certmonger on all CA
servers,
not just on the renewal master. The behavior should be the same as
for the
main CA cert, i.e. the actual renewal is done only on the renewal
master,
other CA servers only update their NSS DBs (this is handled in
dogtag-ipa-ca-renew-agent-submit).

This is important because CA renewal master can change at any time, and
without all CA certs being tracked on all CA servers, there is no
guarantee
the renewal would happen.

2) Since CA clones update their NSS DBs on their own,
dogtag-ipa-ca-renew-agent should be updated not to put them in
cn=ca_renewal,cn=ipa,cn=etc.


Thanks for the review, Honza.  Updated patch 0075-2 attached.


Thanks, ACK.

Rebased patch 0072 and pushed to master:
0078e7a9192a940104d8f6621b33d24d814c109b

It would be nice if lightweight CAs known at replica install time were
tracked without having to manually run ipa-certupdate after
ipa-replica-install. Shall I file a ticket for this, or will you be able
to provide a patch before Friday?






Also, the certs should be untracked on server uninstall.







File the ticket, and I'll try to address by Friday anyways :)

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] 0072..0075 Lightweight CA renewal

2016-06-29 Thread Fraser Tweedale 
On Wed, Jun 29, 2016 at 09:30:17AM +0200, Jan Cholasta wrote:
> On 29.6.2016 08:55, Jan Cholasta wrote:
> > On 24.6.2016 08:49, Fraser Tweedale wrote:
> > > On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:
> > > > Hi,
> > > > 
> > > > On 21.6.2016 08:24, Fraser Tweedale wrote:
> > > > > The attached patches add lightweight CA renewal.  There are two
> > > > > substantive aspects:
> > > > > 
> > > > > 1. The renew_ca_cert updates the serial number in the lightweight
> > > > > CA's entry in the Dogtag database.  This causes CA clones to observe
> > > > > the renewal and update the certs in their own NSSDBs.
> > > > > 
> > > > > 2. The ipa-certupdate command adds Certmonger tracking requests for
> > > > > lightweight CAs (on the renewal master only).
> > > > > 
> > > > > Correct behaviour also depends on my patch 0069 (in-server API for
> > > > > renew_ca_cert script).
> > > > 
> > > > Patch 0072-0074: LGTM
> > > > 
> > > > Patch 0075:
> > > > 
> > > > 1) Lightweight CA certs should be tracked by certmonger on all CA
> > > > servers,
> > > > not just on the renewal master. The behavior should be the same as
> > > > for the
> > > > main CA cert, i.e. the actual renewal is done only on the renewal
> > > > master,
> > > > other CA servers only update their NSS DBs (this is handled in
> > > > dogtag-ipa-ca-renew-agent-submit).
> > > > 
> > > > This is important because CA renewal master can change at any time, and
> > > > without all CA certs being tracked on all CA servers, there is no
> > > > guarantee
> > > > the renewal would happen.
> > > > 
> > > > 2) Since CA clones update their NSS DBs on their own,
> > > > dogtag-ipa-ca-renew-agent should be updated not to put them in
> > > > cn=ca_renewal,cn=ipa,cn=etc.
> > > > 
> > > Thanks for the review, Honza.  Updated patch 0075-2 attached.
> > 
> > Thanks, ACK.
> > 
> > Rebased patch 0072 and pushed to master:
> > 0078e7a9192a940104d8f6621b33d24d814c109b
> > 
> > It would be nice if lightweight CAs known at replica install time were
> > tracked without having to manually run ipa-certupdate after
> > ipa-replica-install. Shall I file a ticket for this, or will you be able
> > to provide a patch before Friday?
> 
> Also, the certs should be untracked on server uninstall.
> 
File the ticket, and I'll try to address by Friday anyways :)

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] 0072..0075 Lightweight CA renewal

2016-06-29 Thread Jan Cholasta 

On 29.6.2016 08:55, Jan Cholasta wrote:

On 24.6.2016 08:49, Fraser Tweedale wrote:

On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:

Hi,

On 21.6.2016 08:24, Fraser Tweedale wrote:

The attached patches add lightweight CA renewal.  There are two
substantive aspects:

1. The renew_ca_cert updates the serial number in the lightweight
CA's entry in the Dogtag database.  This causes CA clones to observe
the renewal and update the certs in their own NSSDBs.

2. The ipa-certupdate command adds Certmonger tracking requests for
lightweight CAs (on the renewal master only).

Correct behaviour also depends on my patch 0069 (in-server API for
renew_ca_cert script).


Patch 0072-0074: LGTM

Patch 0075:

1) Lightweight CA certs should be tracked by certmonger on all CA
servers,
not just on the renewal master. The behavior should be the same as
for the
main CA cert, i.e. the actual renewal is done only on the renewal
master,
other CA servers only update their NSS DBs (this is handled in
dogtag-ipa-ca-renew-agent-submit).

This is important because CA renewal master can change at any time, and
without all CA certs being tracked on all CA servers, there is no
guarantee
the renewal would happen.

2) Since CA clones update their NSS DBs on their own,
dogtag-ipa-ca-renew-agent should be updated not to put them in
cn=ca_renewal,cn=ipa,cn=etc.


Thanks for the review, Honza.  Updated patch 0075-2 attached.


Thanks, ACK.

Rebased patch 0072 and pushed to master:
0078e7a9192a940104d8f6621b33d24d814c109b

It would be nice if lightweight CAs known at replica install time were
tracked without having to manually run ipa-certupdate after
ipa-replica-install. Shall I file a ticket for this, or will you be able
to provide a patch before Friday?


Also, the certs should be untracked on server uninstall.

--
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] 0072..0075 Lightweight CA renewal

2016-06-29 Thread Jan Cholasta 

On 24.6.2016 08:49, Fraser Tweedale wrote:

On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:

Hi,

On 21.6.2016 08:24, Fraser Tweedale wrote:

The attached patches add lightweight CA renewal.  There are two
substantive aspects:

1. The renew_ca_cert updates the serial number in the lightweight
CA's entry in the Dogtag database.  This causes CA clones to observe
the renewal and update the certs in their own NSSDBs.

2. The ipa-certupdate command adds Certmonger tracking requests for
lightweight CAs (on the renewal master only).

Correct behaviour also depends on my patch 0069 (in-server API for
renew_ca_cert script).


Patch 0072-0074: LGTM

Patch 0075:

1) Lightweight CA certs should be tracked by certmonger on all CA servers,
not just on the renewal master. The behavior should be the same as for the
main CA cert, i.e. the actual renewal is done only on the renewal master,
other CA servers only update their NSS DBs (this is handled in
dogtag-ipa-ca-renew-agent-submit).

This is important because CA renewal master can change at any time, and
without all CA certs being tracked on all CA servers, there is no guarantee
the renewal would happen.

2) Since CA clones update their NSS DBs on their own,
dogtag-ipa-ca-renew-agent should be updated not to put them in
cn=ca_renewal,cn=ipa,cn=etc.


Thanks for the review, Honza.  Updated patch 0075-2 attached.


Thanks, ACK.

Rebased patch 0072 and pushed to master: 
0078e7a9192a940104d8f6621b33d24d814c109b


It would be nice if lightweight CAs known at replica install time were 
tracked without having to manually run ipa-certupdate after 
ipa-replica-install. Shall I file a ticket for this, or will you be able 
to provide a patch before Friday?


--
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] 0072..0075 Lightweight CA renewal

2016-06-24 Thread Fraser Tweedale 
On Thu, Jun 23, 2016 at 09:51:02AM +0200, Jan Cholasta wrote:
> Hi,
> 
> On 21.6.2016 08:24, Fraser Tweedale wrote:
> > The attached patches add lightweight CA renewal.  There are two
> > substantive aspects:
> > 
> > 1. The renew_ca_cert updates the serial number in the lightweight
> > CA's entry in the Dogtag database.  This causes CA clones to observe
> > the renewal and update the certs in their own NSSDBs.
> > 
> > 2. The ipa-certupdate command adds Certmonger tracking requests for
> > lightweight CAs (on the renewal master only).
> > 
> > Correct behaviour also depends on my patch 0069 (in-server API for
> > renew_ca_cert script).
> 
> Patch 0072-0074: LGTM
> 
> Patch 0075:
> 
> 1) Lightweight CA certs should be tracked by certmonger on all CA servers,
> not just on the renewal master. The behavior should be the same as for the
> main CA cert, i.e. the actual renewal is done only on the renewal master,
> other CA servers only update their NSS DBs (this is handled in
> dogtag-ipa-ca-renew-agent-submit).
> 
> This is important because CA renewal master can change at any time, and
> without all CA certs being tracked on all CA servers, there is no guarantee
> the renewal would happen.
> 
> 2) Since CA clones update their NSS DBs on their own,
> dogtag-ipa-ca-renew-agent should be updated not to put them in
> cn=ca_renewal,cn=ipa,cn=etc.
> 
Thanks for the review, Honza.  Updated patch 0075-2 attached.
From 9256f36d8df206642a51964ae2f40f4905e0c0bc Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Tue, 21 Jun 2016 15:01:41 +1000
Subject: [PATCH] ipa-certupdate: track lightweight CA certificates

Enhance the ipa-certupdate program to add Certmonger tracking
requests for lightweight CA certificates.

Also update the dogtag-ipa-ca-renew-agent-submit to not store or
retrieve lightweight CA certificates, becaues Dogtag clones observe
renewals and update their NSSDBs on their own, and allow the helper
to request non-self-signed certificates.

Part of: https://fedorahosted.org/freeipa/ticket/4559
---
 .../certmonger/dogtag-ipa-ca-renew-agent-submit| 39 +---
 ipaclient/ipa_certupdate.py| 52 --
 2 files changed, 82 insertions(+), 9 deletions(-)

diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit 
b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index 
3f7333c0e0bb6059e8b3791ef5230c7e5663d2eb..7ab3ec15db37894ed443aa16b7edcf85d69c8192
 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -62,6 +62,24 @@ if six.PY3:
 unicode = str
 
 
+IPA_CA_NICKNAME = 'caSigningCert cert-pki-ca'
+
+def get_nickname():
+csr = os.environ.get('CERTMONGER_CSR')
+return pkcs10.get_friendlyname(csr) if csr else None
+
+def is_lightweight_ca():
+nickname = get_nickname() or ''
+return nickname != IPA_CA_NICKNAME and nickname.startswith(IPA_CA_NICKNAME)
+
+def is_renewable():
+cert = os.environ.get('CERTMONGER_CERTIFICATE')
+if not cert:
+return False
+else:
+return x509.is_self_signed(cert) or is_lightweight_ca()
+
+
 @contextlib.contextmanager
 def ldap_connect():
 conn = None
@@ -210,6 +228,11 @@ def store_cert():
 if not cert:
 return (REJECTED, "New certificate requests not supported")
 
+if is_lightweight_ca():
+# Lightweight CAs are updated in Dogtag's NSSDB
+# by Dogtag itself, so do not store it
+return (ISSUED, cert)
+
 dercert = x509.normalize_certificate(cert)
 
 dn = DN(('cn', nickname), ('cn', 'ca_renewal'),
@@ -338,6 +361,12 @@ def retrieve_cert_continuous():
 if old_cert:
 old_cert = x509.normalize_certificate(old_cert)
 
+if is_lightweight_ca():
+# Lightweight CAs are updated in Dogtag's NSSDB
+# by Dogtag itself, so do not try to retrieve it.
+# Everything is fine as is.
+return (ISSUED, os.environ.get('CERTMONGER_CERTIFICATE'))
+
 result = call_handler(retrieve_or_reuse_cert)
 if result[0] != ISSUED:
 return result
@@ -393,13 +422,12 @@ def renew_ca_cert():
 cert = os.environ.get('CERTMONGER_CERTIFICATE')
 if not cert:
 return (REJECTED, "New certificate requests not supported")
-is_self_signed = x509.is_self_signed(cert)
 
 operation = os.environ.get('CERTMONGER_OPERATION')
 if operation == 'SUBMIT':
 state = 'retrieve'
 
-if is_self_signed:
+if is_renewable():
 ca = cainstance.CAInstance(host_name=api.env.host, ldapi=False)
 if ca.is_renewal_master():
 state = 'request'
@@ -419,10 +447,11 @@ def renew_ca_cert():
 
 if state == 'retrieve':
 result = call_handler(retrieve_cert)
-if result[0] == REJECTED and not is_self_signed:
+if result[0] == REJECTED and not is_renewable():
 syslog.syslog(syslog.LOG_ALERT,
-  "IPA CA certificate is

Re: [Freeipa-devel] [PATCH] 0072..0075 Lightweight CA renewal

2016-06-23 Thread Jan Cholasta 

Hi,

On 21.6.2016 08:24, Fraser Tweedale wrote:

The attached patches add lightweight CA renewal.  There are two
substantive aspects:

1. The renew_ca_cert updates the serial number in the lightweight
CA's entry in the Dogtag database.  This causes CA clones to observe
the renewal and update the certs in their own NSSDBs.

2. The ipa-certupdate command adds Certmonger tracking requests for
lightweight CAs (on the renewal master only).

Correct behaviour also depends on my patch 0069 (in-server API for
renew_ca_cert script).


Patch 0072-0074: LGTM

Patch 0075:

1) Lightweight CA certs should be tracked by certmonger on all CA 
servers, not just on the renewal master. The behavior should be the same 
as for the main CA cert, i.e. the actual renewal is done only on the 
renewal master, other CA servers only update their NSS DBs (this is 
handled in dogtag-ipa-ca-renew-agent-submit).


This is important because CA renewal master can change at any time, and 
without all CA certs being tracked on all CA servers, there is no 
guarantee the renewal would happen.


2) Since CA clones update their NSS DBs on their own, 
dogtag-ipa-ca-renew-agent should be updated not to put them in 
cn=ca_renewal,cn=ipa,cn=etc.



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

[Freeipa-devel] [PATCH] 0072..0075 Lightweight CA renewal

2016-06-21 Thread Fraser Tweedale 
The attached patches add lightweight CA renewal.  There are two
substantive aspects:

1. The renew_ca_cert updates the serial number in the lightweight
CA's entry in the Dogtag database.  This causes CA clones to observe
the renewal and update the certs in their own NSSDBs.

2. The ipa-certupdate command adds Certmonger tracking requests for
lightweight CAs (on the renewal master only).

Correct behaviour also depends on my patch 0069 (in-server API for
renew_ca_cert script).

Cheers,
Fraser
From c2333f0dbe0560a67059030e1a04eb96a52c20d8 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Fri, 17 Jun 2016 10:05:49 +1000
Subject: [PATCH 72/75] ipaldap: turn LDAP filter utility functions into class
 methods

The LDAP filter utilities do not use any instance attributes, so
collectively turn them into class methods to promote reuse.

Part of: https://fedorahosted.org/freeipa/ticket/4559
---
 ipapython/ipaldap.py | 35 +++
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 
410ddae2c484f060abf2bbf3e897549a26b0ebc9..67a3c82f03bd89299ce12a35112e9cfda35d6fe6
 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -1153,7 +1153,8 @@ class LDAPClient(object):
 # entry_attrs = {u'firstName': u'Pavel', u'lastName': u'Zuna'}
 # f = ldap2.make_filter(entry_attrs, rules=ldap2.MATCH_ALL)
 
-def combine_filters(self, filters, rules='|'):
+@classmethod
+def combine_filters(cls, filters, rules='|'):
 """
 Combine filters into one for ldap2.find_entries.
 
@@ -1164,9 +1165,9 @@ class LDAPClient(object):
 assert isinstance(filters, (list, tuple))
 
 filters = [f for f in filters if f]
-if filters and rules == self.MATCH_NONE:  # unary operator
-return '(%s%s)' % (self.MATCH_NONE,
-   self.combine_filters(filters, self.MATCH_ANY))
+if filters and rules == cls.MATCH_NONE:  # unary operator
+return '(%s%s)' % (cls.MATCH_NONE,
+   cls.combine_filters(filters, cls.MATCH_ANY))
 
 if len(filters) > 1:
 flt = '(%s' % rules
@@ -1180,8 +1181,9 @@ class LDAPClient(object):
 flt = '%s)' % flt
 return flt
 
+@classmethod
 def make_filter_from_attr(
-self, attr, value, rules='|', exact=True,
+cls, attr, value, rules='|', exact=True,
 leading_wildcard=True, trailing_wildcard=True):
 """
 Make filter for ldap2.find_entries from attribute.
@@ -1198,18 +1200,18 @@ class LDAPClient(object):
 False - forbid trailing filter wildcard when exact=False
 """
 if isinstance(value, (list, tuple)):
-if rules == self.MATCH_NONE:
-make_filter_rules = self.MATCH_ANY
+if rules == cls.MATCH_NONE:
+make_filter_rules = cls.MATCH_ANY
 else:
 make_filter_rules = rules
 flts = [
-self.make_filter_from_attr(
+cls.make_filter_from_attr(
 attr, v, exact=exact,
 leading_wildcard=leading_wildcard,
 trailing_wildcard=trailing_wildcard)
 for v in value
 ]
-return self.combine_filters(flts, rules)
+return cls.combine_filters(flts, rules)
 elif value is not None:
 value = ldap.filter.escape_filter_chars(value_to_utf8(value))
 if not exact:
@@ -1219,13 +1221,14 @@ class LDAPClient(object):
 if trailing_wildcard:
 template = template + '*'
 value = template % value
-if rules == self.MATCH_NONE:
+if rules == cls.MATCH_NONE:
 return '(!(%s=%s))' % (attr, value)
 return '(%s=%s)' % (attr, value)
 return ''
 
+@classmethod
 def make_filter(
-self, entry_attrs, attrs_list=None, rules='|', exact=True,
+cls, entry_attrs, attrs_list=None, rules='|', exact=True,
 leading_wildcard=True, trailing_wildcard=True):
 """
 Make filter for ldap2.find_entries from entry attributes.
@@ -1247,15 +1250,15 @@ class LDAPClient(object):
 ldap2.MATCH_ALL - match entries that match all attributes
 ldap2.MATCH_ANY - match entries that match any of attribute
 """
-if rules == self.MATCH_NONE:
-make_filter_rules = self.MATCH_ANY
+if rules == cls.MATCH_NONE:
+make_filter_rules = cls.MATCH_ANY
 else:
 make_filter_rules = rules
 flts = []
 if attrs_list is None:
 for (k, v) in entry_attrs.items():
 flts.append(
-self.make_filter_from_attr(
+cls.make_filter_from_attr(
 k, v,