Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert
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
Re: [Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert
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 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
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
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 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
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
[Freeipa-devel] [PATCH 0288] certs: Fix incorrect flag handling in load_cacert
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 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