Re: [Freeipa-devel] [PATCH] 0057..0058 Fix caIPAserviceCert regression

2016-05-19 Thread Jan Cholasta 

On 19.5.2016 13:03, Fraser Tweedale wrote:

On Thu, May 19, 2016 at 09:52:08AM +0200, Jan Cholasta wrote:

On 19.5.2016 01:31, Fraser Tweedale wrote:

On Wed, May 18, 2016 at 03:17:37PM +0200, Jan Cholasta wrote:

Hi,

On 18.5.2016 08:09, Fraser Tweedale wrote:

Rebased version of 0057 attached, along with new patch 0058 that
detects when the Dogtag version of caIPAserviceCert has been
erroneously imported and repairs the profile.


How to reproduce the issue? So far I had no luck with
freeipa-server-4.2.4-1.fc23.x86_64.

Honza


`ipa-replica-install --setup-ca` with an affected version (including
4.2.4-1) will cause the issue.


Thanks.

The fix seems to work, although if you install an unfixed replica against a
fixed server, it will still break the profile. I'm not sure how much of a
problem this could be in the real world, though.


Thank you for testing.  If un-fixed replica re-breaks the profile,
the next server upgrade on any fixed replica will re-fix the
profile.  So assume customer eventually upgrades all replicas beyond
the broken versions, it will converge on a fixed profile.


Right.




For the record, this is a diff between the relevant parts of a test
certificate issued before and after the fix:

@@ -1,19 +1,19 @@
 Validity
 Not Before: 
 Not After : 
-Subject: O=IPA, OU=pki-ipa,
CN=vm-244.abc.idm.lab.eng.brq.redhat.com
+Subject: O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM,
CN=vm-244.abc.idm.lab.eng.brq.redhat.com
 Subject Public Key Info:
 Public Key Algorithm: rsaEncryption
 Public-Key: (2048 bit)
@@ -36,48 +36,60 @@

keyid:89:8C:12:22:E7:D4:C5:87:06:26:5E:AE:C7:73:B5:4B:82:93:CE:1E

 Authority Information Access:
-OCSP -
URI:http://vm-244.abc.idm.lab.eng.brq.redhat.com:80/ca/ocsp
+OCSP -
URI:http://ipa-ca.abc.idm.lab.eng.brq.redhat.com/ca/ocsp

 X509v3 Key Usage: critical
 Digital Signature, Non Repudiation, Key Encipherment, Data
Encipherment
 X509v3 Extended Key Usage:
 TLS Web Server Authentication, TLS Web Client
Authentication
+X509v3 CRL Distribution Points:
+
+Full Name:
+ URI:http://ipa-ca.abc.idm.lab.eng.brq.redhat.com/ipa/crl/MasterCRL.bin
+CRL Issuer:
+  DirName: O = ipaca, CN = Certificate Authority
+
+X509v3 Subject Key Identifier:
+5E:A4:14:31:8E:71:00:4E:2A:71:D6:49:E4:1D:09:EC:10:DF:5D:B1
 Signature Algorithm: sha256WithRSAEncryption
  


Note that SAN, if requested, would also not appear with the broken
profile.


The ticket and bugzilla mention only OCSP URI being wrong. It should be
stated somewhere visible that the subject name and CRL distribution points
are also affected.

BTW the CRL issuer field is wrong. In my case, the issuer name of the CRL is
"CN=Certificate Authority,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM", not
"CN=Certificate Authority,O=ipaca". Also, according to RFC 5280, section
4.2.1.13, the CRL issuer field must be ommited if the CRL issuer is the same
as the certificate issuer.


It's been like that (i.e. wrong) forever.  A proper fix, in light of
sub-CAs work, will require a fair bit of work in Dogtag, i.e. a new
profile component that implements the following heuristic:

- Determine whether issuer has CRL(s) configured; if not, omit CRLDP
- Determine whether issuer issues CRLs itself, or delegates.
  Construct CRLDP extension accordingly.

Not gonna happen for v4.4 ;)


OK. ACK then.

Pushed to:
master: 356f262fb7320345fd5f787c383912b9a2d77314
ipa-4-2: f116e51ce3d495758ff71f685b78d4848ce6708c
ipa-4-3: e9672b1a2b191a1622f18a57a2751e4db3e9e39d

--
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] 0057..0058 Fix caIPAserviceCert regression

2016-05-19 Thread Fraser Tweedale 
On Thu, May 19, 2016 at 09:52:08AM +0200, Jan Cholasta wrote:
> On 19.5.2016 01:31, Fraser Tweedale wrote:
> > On Wed, May 18, 2016 at 03:17:37PM +0200, Jan Cholasta wrote:
> > > Hi,
> > > 
> > > On 18.5.2016 08:09, Fraser Tweedale wrote:
> > > > Rebased version of 0057 attached, along with new patch 0058 that
> > > > detects when the Dogtag version of caIPAserviceCert has been
> > > > erroneously imported and repairs the profile.
> > > 
> > > How to reproduce the issue? So far I had no luck with
> > > freeipa-server-4.2.4-1.fc23.x86_64.
> > > 
> > > Honza
> > > 
> > `ipa-replica-install --setup-ca` with an affected version (including
> > 4.2.4-1) will cause the issue.
> 
> Thanks.
> 
> The fix seems to work, although if you install an unfixed replica against a
> fixed server, it will still break the profile. I'm not sure how much of a
> problem this could be in the real world, though.
> 
Thank you for testing.  If un-fixed replica re-breaks the profile,
the next server upgrade on any fixed replica will re-fix the
profile.  So assume customer eventually upgrades all replicas beyond
the broken versions, it will converge on a fixed profile.

> For the record, this is a diff between the relevant parts of a test
> certificate issued before and after the fix:
> 
> @@ -1,19 +1,19 @@
>  Validity
>  Not Before: 
>  Not After : 
> -Subject: O=IPA, OU=pki-ipa,
> CN=vm-244.abc.idm.lab.eng.brq.redhat.com
> +Subject: O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM,
> CN=vm-244.abc.idm.lab.eng.brq.redhat.com
>  Subject Public Key Info:
>  Public Key Algorithm: rsaEncryption
>  Public-Key: (2048 bit)
> @@ -36,48 +36,60 @@
> 
> keyid:89:8C:12:22:E7:D4:C5:87:06:26:5E:AE:C7:73:B5:4B:82:93:CE:1E
> 
>  Authority Information Access:
> -OCSP -
> URI:http://vm-244.abc.idm.lab.eng.brq.redhat.com:80/ca/ocsp
> +OCSP -
> URI:http://ipa-ca.abc.idm.lab.eng.brq.redhat.com/ca/ocsp
> 
>  X509v3 Key Usage: critical
>  Digital Signature, Non Repudiation, Key Encipherment, Data
> Encipherment
>  X509v3 Extended Key Usage:
>  TLS Web Server Authentication, TLS Web Client
> Authentication
> +X509v3 CRL Distribution Points:
> +
> +Full Name:
> + URI:http://ipa-ca.abc.idm.lab.eng.brq.redhat.com/ipa/crl/MasterCRL.bin
> +CRL Issuer:
> +  DirName: O = ipaca, CN = Certificate Authority
> +
> +X509v3 Subject Key Identifier:
> +5E:A4:14:31:8E:71:00:4E:2A:71:D6:49:E4:1D:09:EC:10:DF:5D:B1
>  Signature Algorithm: sha256WithRSAEncryption
>   
> 
Note that SAN, if requested, would also not appear with the broken
profile.

> The ticket and bugzilla mention only OCSP URI being wrong. It should be
> stated somewhere visible that the subject name and CRL distribution points
> are also affected.
> 
> BTW the CRL issuer field is wrong. In my case, the issuer name of the CRL is
> "CN=Certificate Authority,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM", not
> "CN=Certificate Authority,O=ipaca". Also, according to RFC 5280, section
> 4.2.1.13, the CRL issuer field must be ommited if the CRL issuer is the same
> as the certificate issuer.
> 
It's been like that (i.e. wrong) forever.  A proper fix, in light of
sub-CAs work, will require a fair bit of work in Dogtag, i.e. a new
profile component that implements the following heuristic:

- Determine whether issuer has CRL(s) configured; if not, omit CRLDP
- Determine whether issuer issues CRLs itself, or delegates.
  Construct CRLDP extension accordingly.

Not gonna happen for v4.4 ;)

Cheers,
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] 0057..0058 Fix caIPAserviceCert regression

2016-05-19 Thread Jan Cholasta 

On 19.5.2016 01:31, Fraser Tweedale wrote:

On Wed, May 18, 2016 at 03:17:37PM +0200, Jan Cholasta wrote:

Hi,

On 18.5.2016 08:09, Fraser Tweedale wrote:

Rebased version of 0057 attached, along with new patch 0058 that
detects when the Dogtag version of caIPAserviceCert has been
erroneously imported and repairs the profile.


How to reproduce the issue? So far I had no luck with
freeipa-server-4.2.4-1.fc23.x86_64.

Honza


`ipa-replica-install --setup-ca` with an affected version (including
4.2.4-1) will cause the issue.


Thanks.

The fix seems to work, although if you install an unfixed replica 
against a fixed server, it will still break the profile. I'm not sure 
how much of a problem this could be in the real world, though.


For the record, this is a diff between the relevant parts of a test 
certificate issued before and after the fix:


@@ -1,19 +1,19 @@
 Validity
 Not Before: 
 Not After : 
-Subject: O=IPA, OU=pki-ipa, 
CN=vm-244.abc.idm.lab.eng.brq.redhat.com
+Subject: O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM, 
CN=vm-244.abc.idm.lab.eng.brq.redhat.com

 Subject Public Key Info:
 Public Key Algorithm: rsaEncryption
 Public-Key: (2048 bit)
@@ -36,48 +36,60 @@

keyid:89:8C:12:22:E7:D4:C5:87:06:26:5E:AE:C7:73:B5:4B:82:93:CE:1E

 Authority Information Access:
-OCSP - 
URI:http://vm-244.abc.idm.lab.eng.brq.redhat.com:80/ca/ocsp
+OCSP - 
URI:http://ipa-ca.abc.idm.lab.eng.brq.redhat.com/ca/ocsp


 X509v3 Key Usage: critical
 Digital Signature, Non Repudiation, Key Encipherment, 
Data Encipherment

 X509v3 Extended Key Usage:
 TLS Web Server Authentication, TLS Web Client 
Authentication

+X509v3 CRL Distribution Points:
+
+Full Name:
+ 
URI:http://ipa-ca.abc.idm.lab.eng.brq.redhat.com/ipa/crl/MasterCRL.bin

+CRL Issuer:
+  DirName: O = ipaca, CN = Certificate Authority
+
+X509v3 Subject Key Identifier:
+5E:A4:14:31:8E:71:00:4E:2A:71:D6:49:E4:1D:09:EC:10:DF:5D:B1
 Signature Algorithm: sha256WithRSAEncryption
  

The ticket and bugzilla mention only OCSP URI being wrong. It should be 
stated somewhere visible that the subject name and CRL distribution 
points are also affected.


BTW the CRL issuer field is wrong. In my case, the issuer name of the 
CRL is "CN=Certificate Authority,O=ABC.IDM.LAB.ENG.BRQ.REDHAT.COM", not 
"CN=Certificate Authority,O=ipaca". Also, according to RFC 5280, section 
4.2.1.13, the CRL issuer field must be ommited if the CRL issuer is the 
same as the certificate issuer.


--
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] 0057..0058 Fix caIPAserviceCert regression

2016-05-18 Thread Fraser Tweedale 
On Wed, May 18, 2016 at 03:17:37PM +0200, Jan Cholasta wrote:
> Hi,
> 
> On 18.5.2016 08:09, Fraser Tweedale wrote:
> > Rebased version of 0057 attached, along with new patch 0058 that
> > detects when the Dogtag version of caIPAserviceCert has been
> > erroneously imported and repairs the profile.
> 
> How to reproduce the issue? So far I had no luck with
> freeipa-server-4.2.4-1.fc23.x86_64.
> 
> Honza
> 
`ipa-replica-install --setup-ca` with an affected version (including
4.2.4-1) will cause the issue.

-- 
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] 0057..0058 Fix caIPAserviceCert regression

2016-05-18 Thread Jan Cholasta 

Hi,

On 18.5.2016 08:09, Fraser Tweedale wrote:

Rebased version of 0057 attached, along with new patch 0058 that
detects when the Dogtag version of caIPAserviceCert has been
erroneously imported and repairs the profile.


How to reproduce the issue? So far I had no luck with 
freeipa-server-4.2.4-1.fc23.x86_64.


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] 0057..0058 Fix caIPAserviceCert regression

2016-05-18 Thread Fraser Tweedale 
Rebased version of 0057 attached, along with new patch 0058 that
detects when the Dogtag version of caIPAserviceCert has been
erroneously imported and repairs the profile.

Thanks,
Fraser
From 9994a98a0cd3bcf6b4af72708c7ac1cfdfdd5440 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 11 May 2016 16:13:51 +1000
Subject: [PATCH] Prevent replica install from overwriting cert profiles

An earlier change that unconditionally triggers import of file-based
profiles to LDAP during server or replica install results in
replicas overwriting FreeIPA-managed profiles with profiles of the
same name shipped with Dogtag. ('caIPAserviceCert' is the affected
profile).

Avoid this situation by never overwriting existing profiles during
the LDAP import.

Fixes: https://fedorahosted.org/freeipa/ticket/5881
---
 ipaserver/install/cainstance.py | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 
3ca4fa8d373ebc3375a9fc75b59969292f0198f0..7e68b832831c3487c7bda466ba04d1a3eb51e780
 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1764,7 +1764,9 @@ def import_included_profiles():
 conn.add_entry(entry)
 profile_data = ipautil.template_file(
 '/usr/share/ipa/profiles/{}.cfg'.format(profile_id), sub_dict)
-_create_dogtag_profile(profile_id, profile_data)
+
+# Create the profile, replacing any existing profile of same name
+_create_dogtag_profile(profile_id, profile_data, overwrite=True)
 root_logger.info("Imported profile '%s'", profile_id)
 
 api.Backend.ra_certprofile.override_port = None
@@ -1816,12 +1818,17 @@ def migrate_profiles_to_ldap(dogtag_constants):
 profile_data += '\n'
 profile_data += 'profileId={}\n'.format(profile_id)
 profile_data += 'classId={}\n'.format(class_id)
-_create_dogtag_profile(profile_id, profile_data)
+
+# Import the profile, but do not replace it if it already exists.
+# This prevents replicas from replacing IPA-managed profiles with
+# Dogtag default profiles of same name.
+#
+_create_dogtag_profile(profile_id, profile_data, overwrite=False)
 
 api.Backend.ra_certprofile.override_port = None
 
 
-def _create_dogtag_profile(profile_id, profile_data):
+def _create_dogtag_profile(profile_id, profile_data, overwrite):
 with api.Backend.ra_certprofile as profile_api:
 # import the profile
 try:
@@ -1832,9 +1839,8 @@ def _create_dogtag_profile(profile_id, profile_data):
 root_logger.debug("Error migrating '{}': {}".format(
 profile_id, e))
 
-# conflicting profile; replace it if we are
-# installing IPA, but keep it for upgrades
-if api.env.context == 'installer':
+# profile already exists
+if overwrite:
 try:
 profile_api.disable_profile(profile_id)
 except errors.RemoteRetrieveError:
-- 
2.5.5

From 9524513a6e7c1e9da7d0a20dfec1d1566158cef0 Mon Sep 17 00:00:00 2001
From: Fraser Tweedale 
Date: Wed, 11 May 2016 16:13:51 +1000
Subject: [PATCH] Prevent replica install from overwriting cert profiles

An earlier change that unconditionally triggers import of file-based
profiles to LDAP during server or replica install results in
replicas overwriting FreeIPA-managed profiles with profiles of the
same name shipped with Dogtag. ('caIPAserviceCert' is the affected
profile).

Avoid this situation by never overwriting existing profiles during
the LDAP import.

Fixes: https://fedorahosted.org/freeipa/ticket/5881
---
 ipaserver/install/cainstance.py | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 
10bc2afc416737e89ffa7255e50bec96eb86fcce..274694012d5afc8690c4d69356d5ae56ae0a44e1
 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1665,7 +1665,9 @@ def import_included_profiles():
 conn.add_entry(entry)
 profile_data = ipautil.template_file(
 '/usr/share/ipa/profiles/{}.cfg'.format(profile_id), sub_dict)
-_create_dogtag_profile(profile_id, profile_data)
+
+# Create the profile, replacing any existing profile of same name
+_create_dogtag_profile(profile_id, profile_data, overwrite=True)
 root_logger.info("Imported profile '%s'", profile_id)
 
 api.Backend.ra_certprofile.override_port = None
@@ -1717,12 +1719,17 @@ def migrate_profiles_to_ldap():
 profile_data += '\n'
 profile_data += 'profileId={}\n'.format(profile_id)
 profile_data += 'classId={}\n'.format(class_id)
-