On 06/09/2016 04:32 PM, Rob Crittenden wrote:
Fraser Tweedale wrote:
On Thu, Jun 09, 2016 at 03:07:34PM +0200, Martin Basti wrote:
On 09.06.2016 15:03, Martin Basti wrote:
On 09.06.2016 15:02, Stanislav Laznicka wrote:
On 06/09/2016 02:51 PM, Rob Crittenden wrote:
Stanislav Laznicka wrote:
Hello,

Please see the attached patch of
https://fedorahosted.org/freeipa/ticket/5797.

Standa

Just wondering out loud but should usercertificate be excluded
from the output if it is unparsable? Is there any value in
showing that a bogus value is in there?

rob
I think it is a good pointer that something has gone wrong with the
certificate. Another way would be to print 'Invalid certificate'
instead of it similar to what Apache LDAP Browser does.
We can return a warning message that something with certificates is
broken.

Martin^2

And you should log it at error log level, because it is error

Is the data from LDAP actually invalid?  It should not be possible
to store data that is not a syntactically valid X.509 cert in the
userCertificate attribute (if it is, we should file a ticket against
389).

Is there a full traceback for the original error of #5797?  What is
the datum that is the immediate cause of the error and what happens
to it between the database and the function that throws?

Could it be a python3 bytes/str problem originating in
x509.normalize_certificate?

Cheers,
Fraser


A cert can get in several different ways. IPA sure tries hard not to allow bad certs but I guess they can happen:

$ ldapmodify -Y GSSAPI
SASL/GSSAPI authentication started
SASL username: ad...@greyoak.com
SASL SSF: 56
SASL data security layer installed.
dn: krbprincipalname=cert/slithy.greyoak....@greyoak.com,cn=services,cn=accounts,dc=greyoak,dc=com
changetype: modify
add: usercertificate
usercertificate: foo

modifying entry "krbprincipalname=cert/slithy.greyoak....@greyoak.com,cn=services,cn=accounts,dc=greyoak,dc=com"
< .. snip .. >
That is exactly how I was reproducing this issue.

Added is the patch that adds error message and logs properly.
From 6913cb98da07b235b571d0772889df9a2caff1e8 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Thu, 9 Jun 2016 13:13:24 +0200
Subject: [PATCH] host/service-show/find shouldn't fail on invalid certificate

host/service-show/find methods would have failed if the first
certificate they had in userCertificate attribute were invalid.
Expected behavior is that they just show the rest of the reqested
attributes.

https://fedorahosted.org/freeipa/ticket/5797
---
 ipalib/messages.py           | 10 ++++++++++
 ipaserver/plugins/host.py    | 31 +++++++++++++++++++++++++++++--
 ipaserver/plugins/service.py | 34 +++++++++++++++++++++++++++++++---
 3 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index e863bdd495b55921c9e487794f5c9573a6166038..2c02f8b912f5e8253048f63c0b60a5483b29772b 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -395,6 +395,16 @@ class DNSForwardPolicyConflictWithEmptyZone(PublicMessage):
     )
 
 
+class CertificateInvalid(PublicMessage):
+    """
+    ***13022 Failed to parse a certificate
+    """
+    errno = 13022
+    type = "error"
+    format = _("%(subject)s: Invalid certificate. "
+               "%(reason)s")
+
+
 def iter_messages(variables, base):
     """Return a tuple with all subclasses
     """
diff --git a/ipaserver/plugins/host.py b/ipaserver/plugins/host.py
index e59e0fa93c9fc0b9c6fccc36421d3489678a0eb2..befa4d8bbb602594cfaa388cf9615d1f62b6a028 100644
--- a/ipaserver/plugins/host.py
+++ b/ipaserver/plugins/host.py
@@ -1023,7 +1023,21 @@ class host_find(LDAPSearch):
         if options.get('pkey_only', False):
             return truncated
         for entry_attrs in entries:
-            set_certificate_attrs(entry_attrs)
+            hostname = entry_attrs['fqdn']
+            if isinstance(hostname, (tuple, list)):
+                hostname = hostname[0]
+            try:
+                set_certificate_attrs(entry_attrs)
+            except errors.CertificateFormatError as e:
+                self.add_message(
+                    messages.CertificateInvalid(
+                        subject=hostname,
+                        reason=e,
+                    )
+                )
+                self.log.error("Invalid certificate: {err}".format(err=e))
+                del(entry_attrs['usercertificate'])
+
             set_kerberos_attrs(entry_attrs, options)
             rename_ipaallowedtoperform_from_ldap(entry_attrs, options)
             self.obj.suppress_netgroup_memberof(ldap, entry_attrs)
@@ -1066,7 +1080,20 @@ class host_show(LDAPRetrieve):
             # fetched anywhere.
             entry_attrs['has_keytab'] = False
 
-        set_certificate_attrs(entry_attrs)
+        hostname = entry_attrs['fqdn']
+        if isinstance(hostname, (tuple, list)):
+            hostname = hostname[0]
+        try:
+            set_certificate_attrs(entry_attrs)
+        except errors.CertificateFormatError as e:
+            self.add_message(
+                messages.CertificateInvalid(
+                    subject=hostname,
+                    reason=e,
+                )
+            )
+            del(entry_attrs['usercertificate'])
+
         set_kerberos_attrs(entry_attrs, options)
         rename_ipaallowedtoperform_from_ldap(entry_attrs, options)
 
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 7b8f2a7aa8711bc8bf6f2e42c5794c8cf358f252..24031eb429c1946f2ec730683f46c9cef35910ed 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -21,7 +21,7 @@
 
 import six
 
-from ipalib import api, errors
+from ipalib import api, errors, messages
 from ipalib import Bytes, StrEnum, Bool, Str, Flag
 from ipalib.plugable import Registry
 from .baseldap import (
@@ -698,7 +698,21 @@ class service_find(LDAPSearch):
             return truncated
         for entry_attrs in entries:
             self.obj.get_password_attributes(ldap, entry_attrs.dn, entry_attrs)
-            set_certificate_attrs(entry_attrs)
+            principal = entry_attrs['krbprincipalname']
+            if isinstance(principal, (tuple, list)):
+                principal = principal[0]
+            try:
+                set_certificate_attrs(entry_attrs)
+            except errors.CertificateFormatError as e:
+                self.add_message(
+                    messages.CertificateInvalid(
+                        subject=principal,
+                        reason=e
+                    )
+                )
+                self.log.error("Invalid certificate: {err}".format(err=e))
+                del(entry_attrs['usercertificate'])
+
             set_kerberos_attrs(entry_attrs, options)
             rename_ipaallowedtoperform_from_ldap(entry_attrs, options)
         return truncated
@@ -721,7 +735,21 @@ class service_show(LDAPRetrieve):
         assert isinstance(dn, DN)
         self.obj.get_password_attributes(ldap, dn, entry_attrs)
 
-        set_certificate_attrs(entry_attrs)
+        principal = entry_attrs['krbprincipalname']
+        if isinstance(principal, (tuple, list)):
+            principal = principal[0]
+        try:
+            set_certificate_attrs(entry_attrs)
+        except errors.CertificateFormatError as e:
+            self.add_message(
+                messages.CertificateInvalid(
+                    subject=principal,
+                    reason=e,
+                )
+            )
+            self.log.error("Invalid certificate: {err}".format(err=e))
+            del(entry_attrs['usercertificate'])
+
         set_kerberos_attrs(entry_attrs, options)
         rename_ipaallowedtoperform_from_ldap(entry_attrs, options)
 
-- 
2.5.5

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

Reply via email to