Handle binascii.Error from base64.b64decode()

In Python 3, the base64.b64decode function raises binascii.Error (a ValueError
subclass) when it finds incorrect padding. In Python 2 it raises TypeError.

Callers should usually handle ValueError; unless they are specifically
concerned with handling base64 padding issues).

In some cases, callers should handle ValueError:
- ipalib.pkcs10 (get_friendlyname, load_certificate_request): callers should
  handle ValueError
- ipalib.x509 (load_certificate*, get_*): callers should handle ValueError

In other cases ValueError is handled:
- ipalib.parameters
- ipapython.ssh
- ipalib.rpc (json_decode_binary - callers already expect ValueError)
- ipaserver.install.ldapupdate

Elsewhere no error handling is done, because values come from trusted
sources, or are pre-validated:
- vault plugin
- ipaserver.install.cainstance
- ipaserver.install.certs
- ipaserver.install.ipa_otptoken_import

Reviewed-By: Tomas Babej <tbabej@redhat.com>

7 files changed, 12 insertions, 11 deletions

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index ef8814eeb..dadd87d6a 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -1383,7 +1383,7 @@ class Bytes(Data):
         if isinstance(value, unicode):
             try:
                 value = base64.b64decode(value)
-            except TypeError as e:
+            except (TypeError, ValueError) as e:
                 raise Base64DecodeError(reason=str(e))
         return super(Bytes, self)._convert_scalar(value, index)
 
diff --git a/ipalib/plugins/cert.py b/ipalib/plugins/cert.py
index e4593200e..b4ea2feae 100644
--- a/ipalib/plugins/cert.py
+++ b/ipalib/plugins/cert.py
@@ -21,6 +21,8 @@
 
 import os
 import time
+import binascii
+
 from ipalib import Command, Str, Int, Bytes, Flag, File
 from ipalib import api
 from ipalib import errors
@@ -156,7 +158,7 @@ def validate_csr(ugettext, csr):
             return
     try:
         request = pkcs10.load_certificate_request(csr)
-    except TypeError as e:
+    except (TypeError, binascii.Error) as e:
         raise errors.Base64DecodeError(reason=str(e))
     except Exception as e:
         raise errors.CertificateOperationError(error=_('Failure decoding Certificate Signing Request: %s') % e)
@@ -368,7 +370,7 @@ class cert_request(VirtualCommand):
             subject = pkcs10.get_subject(csr)
             extensions = pkcs10.get_extensions(csr)
             subjectaltname = pkcs10.get_subjectaltname(csr) or ()
-        except (NSPRError, PyAsn1Error) as e:
+        except (NSPRError, PyAsn1Error, ValueError) as e:
             raise errors.CertificateOperationError(
                 error=_("Failure decoding Certificate Signing Request: %s") % e)
 
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 446e2886e..94d2cb4e9 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -210,7 +210,7 @@ class RedHatTaskNamespace(BaseTaskNamespace):
                 issuer = x509.get_der_issuer(cert, x509.DER)
                 serial_number = x509.get_der_serial_number(cert, x509.DER)
                 public_key_info = x509.get_der_public_key_info(cert, x509.DER)
-            except (NSPRError, PyAsn1Error) as e:
+            except (NSPRError, PyAsn1Error, ValueError) as e:
                 root_logger.warning(
                     "Failed to decode certificate \"%s\": %s", nickname, e)
                 continue
diff --git a/ipapython/ssh.py b/ipapython/ssh.py
index 02f577e8b..a625c422c 100644
--- a/ipapython/ssh.py
+++ b/ipapython/ssh.py
@@ -102,7 +102,7 @@ class SSHPublicKey(object):
 
         try:
             key = base64.b64decode(key)
-        except (TypeError, binascii.Error):
+        except (TypeError, ValueError):
             return False
 
         return self._parse_raw(key)
diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py
index a2242fd3d..66cba891f 100644
--- a/ipaserver/install/ipa_cacert_manage.py
+++ b/ipaserver/install/ipa_cacert_manage.py
@@ -334,7 +334,7 @@ class CACertManage(admintool.AdminTool):
             except IOError as e:
                 raise admintool.ScriptError(
                     "Can't open \"%s\": %s" % (cert_filename, e))
-            except (TypeError, NSPRError) as e:
+            except (TypeError, NSPRError, ValueError) as e:
                 raise admintool.ScriptError("Not a valid certificate: %s" % e)
             subject = nss_cert.subject
             cert = nss_cert.der_data
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index fd02bdc02..86c011047 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -423,7 +423,7 @@ class LDAPUpdate:
                     for i, v in enumerate(value):
                         try:
                             value[i] = base64.b64decode(v)
-                        except TypeError as e:
+                        except (TypeError, ValueError) as e:
                             raise BadSyntax(
                                 "Base64 encoded value %s on line %s:%d: %s is "
                                 "incorrect (%s)" % (v, data_source_name,
diff --git a/ipatests/test_pkcs10/test_pkcs10.py b/ipatests/test_pkcs10/test_pkcs10.py
index 86314dfe5..a36d1883e 100644
--- a/ipatests/test_pkcs10/test_pkcs10.py
+++ b/ipatests/test_pkcs10/test_pkcs10.py
@@ -23,10 +23,9 @@ Test the `pkcs10.py` module.
 # FIXME: Pylint errors
 # pylint: disable=no-member
 
-import os
-import sys
+import binascii
+
 import nose
-from ipatests.util import raises, PluginTester
 from ipalib import pkcs10
 from ipapython import ipautil
 import nss.nss as nss
@@ -122,5 +121,5 @@ class test_update(object):
         csr = self.read_file("test4.csr")
         try:
             request = pkcs10.load_certificate_request(csr)
-        except TypeError as typeerr:
+        except (TypeError, binascii.Error) as typeerr:
             assert(str(typeerr) == 'Incorrect padding')