[Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert

2014-12-02 Thread Tomas Babej
Hi,

For CA certificates that are not certificates of IPA CA, we incorrectly
set the trust flags to ,,, regardless what the actual trust_flags
parameter was passed.

Make the load_cacert method respect trust_flags and make C,, default
set of trust flags.

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

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 


From dacea08e7f33451788f464907385f5ac88f1db4e Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 2 Dec 2014 13:13:51 +0100
Subject: [PATCH] certs: Fix incorrect flag handling in load_cacert

For CA certificates that are not certificates of IPA CA, we incorrectly
set the trust flags to ,,, regardless what the actual trust_flags
parameter was passed.

Make the load_cacert method respect trust_flags and make C,, default
set of trust flags.

https://fedorahosted.org/freeipa/ticket/4779
---
 ipaserver/install/certs.py | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 5399a0fa566c6f7df81a9d1e347f6ac99e5188c9..5a37acb2d2dbbd3193e59643add4c63f297ae2fe 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -238,7 +238,7 @@ class CertDB(object):
  -k, self.passwd_fname])
 self.set_perms(self.pk12_fname)
 
-def load_cacert(self, cacert_fname, trust_flags='C,,'):
+def load_cacert(self, cacert_fname, trust_flags=None):
 
 Load all the certificates from a given file. It is assumed that
 this file creates CA certificates.
@@ -255,11 +255,9 @@ class CertDB(object):
 (rdn, subject_dn) = get_cert_nickname(cert)
 if subject_dn == ca_dn:
 nick = get_ca_nickname(self.realm)
-tf = trust_flags
 else:
 nick = str(subject_dn)
-tf = ',,'
-self.nssdb.add_cert(cert, nick, tf, pem=True)
+self.nssdb.add_cert(cert, nick, trust_flags or C,,, pem=True)
 except RuntimeError:
 break
 
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert

2014-12-02 Thread Jan Cholasta

Hi,

Dne 2.12.2014 v 13:16 Tomas Babej napsal(a):

Hi,

For CA certificates that are not certificates of IPA CA, we incorrectly
set the trust flags to ,,, regardless what the actual trust_flags
parameter was passed.

Make the load_cacert method respect trust_flags and make C,, default
set of trust flags.


For unknown CA certificates, you must keep the default ,, and 
explicitly override it where necessary. We don't want to trust *any* CA 
certificate to issue server certs.




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


Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert

2014-12-02 Thread Tomas Babej

On 12/02/2014 01:45 PM, Jan Cholasta wrote:
 Hi,

 Dne 2.12.2014 v 13:16 Tomas Babej napsal(a):
 Hi,

 For CA certificates that are not certificates of IPA CA, we incorrectly
 set the trust flags to ,,, regardless what the actual trust_flags
 parameter was passed.

 Make the load_cacert method respect trust_flags and make C,, default
 set of trust flags.

 For unknown CA certificates, you must keep the default ,, and
 explicitly override it where necessary. We don't want to trust *any*
 CA certificate to issue server certs.


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

 Honza

Updated patch attached.

However, this boils down to the same, so there is really no functional
difference between the two versions of the patches in the current code
base. All places where load_cacert is called, the trust flags are
explicitly overriden.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

From 55b5f82445c9e0ce45a8c8587fcb7d5c6c5c07b0 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 2 Dec 2014 13:13:51 +0100
Subject: [PATCH] certs: Fix incorrect flag handling in load_cacert

For CA certificates that are not certificates of IPA CA, we incorrectly
set the trust flags to ,,, regardless what the actual trust_flags
parameter was passed.

Make the load_cacert method respect trust_flags and make C,, default
set of trust flags.

https://fedorahosted.org/freeipa/ticket/4779
---
 ipaserver/install/certs.py  | 6 +++---
 ipaserver/install/dsinstance.py | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 5399a0fa566c6f7df81a9d1e347f6ac99e5188c9..6c1537b9c1aff58c56c1d10ada2dfa5deba631ca 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -238,7 +238,7 @@ class CertDB(object):
  -k, self.passwd_fname])
 self.set_perms(self.pk12_fname)
 
-def load_cacert(self, cacert_fname, trust_flags='C,,'):
+def load_cacert(self, cacert_fname, trust_flags=None):
 
 Load all the certificates from a given file. It is assumed that
 this file creates CA certificates.
@@ -255,10 +255,10 @@ class CertDB(object):
 (rdn, subject_dn) = get_cert_nickname(cert)
 if subject_dn == ca_dn:
 nick = get_ca_nickname(self.realm)
-tf = trust_flags
+tf = trust_flags or 'C,,'
 else:
 nick = str(subject_dn)
-tf = ',,'
+tf = trust_flags or ',,'
 self.nssdb.add_cert(cert, nick, tf, pem=True)
 except RuntimeError:
 break
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 06c13c21dd3a5ea1e15c0a797a48fd6af02c1bdf..2ca09e7e32cd423ff90c41ad6309fcf0dd099a82 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -840,7 +840,7 @@ class DsInstance(service.Service):
 certdb.cacert_name = cacert_name
 status = True
 try:
-certdb.load_cacert(cacert_fname)
+certdb.load_cacert(cacert_fname, trust_flags=C,,)
 except ipautil.CalledProcessError, e:
 root_logger.critical(Error importing CA cert file named [%s]: %s %
  (cacert_fname, str(e)))
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert

2014-12-02 Thread Jan Cholasta

Dne 2.12.2014 v 13:55 Tomas Babej napsal(a):


On 12/02/2014 01:45 PM, Jan Cholasta wrote:

Hi,

Dne 2.12.2014 v 13:16 Tomas Babej napsal(a):

Hi,

For CA certificates that are not certificates of IPA CA, we incorrectly
set the trust flags to ,,, regardless what the actual trust_flags
parameter was passed.

Make the load_cacert method respect trust_flags and make C,, default
set of trust flags.


For unknown CA certificates, you must keep the default ,, and
explicitly override it where necessary. We don't want to trust *any*
CA certificate to issue server certs.



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


Honza


Updated patch attached.

However, this boils down to the same, so there is really no functional
difference between the two versions of the patches in the current code
base. All places where load_cacert is called, the trust flags are
explicitly overriden.



OK, then we don't need a default value at all.

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert

2014-12-02 Thread Tomas Babej

On 12/02/2014 02:02 PM, Jan Cholasta wrote:
 Dne 2.12.2014 v 13:55 Tomas Babej napsal(a):

 On 12/02/2014 01:45 PM, Jan Cholasta wrote:
 Hi,

 Dne 2.12.2014 v 13:16 Tomas Babej napsal(a):
 Hi,

 For CA certificates that are not certificates of IPA CA, we
 incorrectly
 set the trust flags to ,,, regardless what the actual trust_flags
 parameter was passed.

 Make the load_cacert method respect trust_flags and make C,, default
 set of trust flags.

 For unknown CA certificates, you must keep the default ,, and
 explicitly override it where necessary. We don't want to trust *any*
 CA certificate to issue server certs.


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

 Honza

 Updated patch attached.

 However, this boils down to the same, so there is really no functional
 difference between the two versions of the patches in the current code
 base. All places where load_cacert is called, the trust flags are
 explicitly overriden.


 OK, then we don't need a default value at all.


Updated patch makes trust_flags a required argument of load_cacert.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

From a087bda498e10b1e923f2882522b42fa7d8f8239 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Tue, 2 Dec 2014 13:13:51 +0100
Subject: [PATCH] certs: Fix incorrect flag handling in load_cacert

For CA certificates that are not certificates of IPA CA, we incorrectly
set the trust flags to ,,, regardless what the actual trust_flags
parameter was passed.

Make the load_cacert method respect trust_flags and make it a required
argument.

https://fedorahosted.org/freeipa/ticket/4779
---
 ipaserver/install/certs.py  | 6 ++
 ipaserver/install/dsinstance.py | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/certs.py b/ipaserver/install/certs.py
index 5399a0fa566c6f7df81a9d1e347f6ac99e5188c9..7292cbbe3574f57d32daa6f1e310669486fa5eff 100644
--- a/ipaserver/install/certs.py
+++ b/ipaserver/install/certs.py
@@ -238,7 +238,7 @@ class CertDB(object):
  -k, self.passwd_fname])
 self.set_perms(self.pk12_fname)
 
-def load_cacert(self, cacert_fname, trust_flags='C,,'):
+def load_cacert(self, cacert_fname, trust_flags):
 
 Load all the certificates from a given file. It is assumed that
 this file creates CA certificates.
@@ -255,11 +255,9 @@ class CertDB(object):
 (rdn, subject_dn) = get_cert_nickname(cert)
 if subject_dn == ca_dn:
 nick = get_ca_nickname(self.realm)
-tf = trust_flags
 else:
 nick = str(subject_dn)
-tf = ',,'
-self.nssdb.add_cert(cert, nick, tf, pem=True)
+self.nssdb.add_cert(cert, nick, trust_flags, pem=True)
 except RuntimeError:
 break
 
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 06c13c21dd3a5ea1e15c0a797a48fd6af02c1bdf..66267f4cdde548266b15594e3640bf8247c64859 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -840,7 +840,7 @@ class DsInstance(service.Service):
 certdb.cacert_name = cacert_name
 status = True
 try:
-certdb.load_cacert(cacert_fname)
+certdb.load_cacert(cacert_fname, 'C,,')
 except ipautil.CalledProcessError, e:
 root_logger.critical(Error importing CA cert file named [%s]: %s %
  (cacert_fname, str(e)))
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert

2014-12-02 Thread Jan Cholasta

Dne 2.12.2014 v 14:09 Tomas Babej napsal(a):


On 12/02/2014 02:02 PM, Jan Cholasta wrote:

Dne 2.12.2014 v 13:55 Tomas Babej napsal(a):


On 12/02/2014 01:45 PM, Jan Cholasta wrote:

Hi,

Dne 2.12.2014 v 13:16 Tomas Babej napsal(a):

Hi,

For CA certificates that are not certificates of IPA CA, we
incorrectly
set the trust flags to ,,, regardless what the actual trust_flags
parameter was passed.

Make the load_cacert method respect trust_flags and make C,, default
set of trust flags.


For unknown CA certificates, you must keep the default ,, and
explicitly override it where necessary. We don't want to trust *any*
CA certificate to issue server certs.



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


Honza


Updated patch attached.

However, this boils down to the same, so there is really no functional
difference between the two versions of the patches in the current code
base. All places where load_cacert is called, the trust flags are
explicitly overriden.



OK, then we don't need a default value at all.



Updated patch makes trust_flags a required argument of load_cacert.



Thanks, ACK!

Pushed to:
master: faec4ef9de431a1b72423be8ce6cea28a7221531
ipa-4-1: db4ac4774523c1d41a606b1c0297e9eeae13ebd6

Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel