Aaand there's a typo in patch 15. Updated version attached.

On 08/09/2016 02:22 PM, Ben Lipton wrote:
Hello,

The attached patches improve upon my last patchset to:

0013: Add support for generating a full script that makes a CSR, rather than just a config, and use that support to automate the full flow from script generation through cert issuance Usage note: the UI for this could probably use work. I currently have the --helper-args param that allows additional data to be passed to the helper. Commonly this would be something like: Certutil: --helper-args '-d /path/to/nss/db' (precreated with certutil -N -d /path/to/nss/db) Openssl: --helper-args 'd /path/to/keyfile' (precreated with openssl genrsa -out /path/to/keyfile)
See the commit message for a full command line.

0014: Allow the feature to be used by non-admin users

0015: Improve error handling by reporting a nice message if the mapping rules are broken, or if the data required to generate the subject DN is missing

These improvements may make it easier to test the other patches.

Thanks,
Ben



From c220b49c2c3a76b58b153dc614d3af216c5cf30d Mon Sep 17 00:00:00 2001
From: Ben Lipton <blip...@redhat.com>
Date: Mon, 1 Aug 2016 10:20:16 -0400
Subject: [PATCH] Improve error handling for certificate mapping

All calls to jinja2 will now raise an IPA error type if rendering does
not succeed, so that broken rules will not generate InternalErrors.
Additionally, if the rendering does not generate a subject DN (for
example if the wrong certificate profile for a principal is used), an
exception is raised, as the CSR creation will not succeed.

https://fedorahosted.org/freeipa/ticket/4899
---
 ipalib/errors.py                 |  9 ++++++++
 ipaserver/plugins/certmapping.py | 45 +++++++++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/ipalib/errors.py b/ipalib/errors.py
index 7b4f15dd60ee80719195ba1b9b85d075b10bdf4f..ad46fc351ab7a0c0c3fd3dc4ba99de6f6b4cb0fa 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -1698,6 +1698,15 @@ class CertificateFormatError(CertificateError):
     format = _('Certificate format error: %(error)s')
 
 
+class CertificateMappingError(CertificateError):
+    """
+    **4035** Raised when a valid cert request config can not be generated
+    """
+
+    errno = 4303
+    format = _('%(reason)s')
+
+
 class MutuallyExclusiveError(ExecutionError):
     """
     **4303** Raised when an operation would result in setting two attributes which are mutually exlusive.
diff --git a/ipaserver/plugins/certmapping.py b/ipaserver/plugins/certmapping.py
index 1d87d1f90de6cc8cb330176df49209dd85581170..36cdd1f2a19ceb5dd223d95ce34eaedc8fe778d7 100644
--- a/ipaserver/plugins/certmapping.py
+++ b/ipaserver/plugins/certmapping.py
@@ -431,11 +431,22 @@ class Formatter(object):
     def _format(self, base_template_name, base_template_params, render_data):
         base_template = self.jinja2.get_template(
             base_template_name, globals=self.passthrough_globals)
-        combined_template_source = base_template.render(**base_template_params)
+
+        try:
+            combined_template_source = base_template.render(**base_template_params)
+        except jinja2.UndefinedError:
+            raise errors.CertificateMappingError(reason=_(
+                'Template error when formatting certificate data'))
+
         self.backend.debug(
             'Formatting with template: %s' % combined_template_source)
         combined_template = self.jinja2.from_string(combined_template_source)
-        script = combined_template.render(**render_data)
+
+        try:
+            script = combined_template.render(**render_data)
+        except jinja2.UndefinedError:
+            raise errors.CertificateMappingError(reason=_(
+                'Template error when formatting certificate data'))
         return dict(script=script)
 
     def _wrap_rule(self, rule, rule_type):
@@ -450,8 +461,12 @@ class Formatter(object):
         self.backend.debug('Syntax rule template: %s' % syntax_rule)
         template = self.jinja2.from_string(
             syntax_rule, globals=self.passthrough_globals)
-        prepared_template = self._wrap_rule(
-            template.render(datarules=data_rules), 'syntax')
+        try:
+            rendered = template.render(datarules=data_rules)
+        except jinja2.UndefinedError:
+            raise errors.CertificateMappingError(reason=_(
+                'Template error when formatting certificate data'))
+        prepared_template = self._wrap_rule(rendered, 'syntax')
         return prepared_template
 
 
@@ -472,6 +487,12 @@ class OpenSSLFormatter(Formatter):
         rendered = self._format(
             'openssl_base.tmpl',
             {'parameters': parameters, 'extensions': extensions}, render_data)
+
+        if not 'distinguished_name =' in rendered['script']:
+            raise errors.CertificateMappingError(reason=_(
+                'Certificate subject could not be generated. You may need to'
+                ' use a different certificate profile for this principal.'))
+
         return rendered
 
     def prepare_syntax_rule(self, syntax_rule, data_rules):
@@ -479,9 +500,13 @@ class OpenSSLFormatter(Formatter):
         self.backend.debug('Syntax rule template: %s' % syntax_rule)
         template = self.jinja2.from_string(
             syntax_rule, globals=self.passthrough_globals)
-        is_extension = getattr(template.module, 'extension', False)
-        prepared_template = self._wrap_rule(
-            template.render(datarules=data_rules), 'syntax')
+        try:
+            is_extension = getattr(template.module, 'extension', False)
+            rendered = template.render(datarules=data_rules)
+        except jinja2.UndefinedError:
+            raise errors.CertificateMappingError(reason=_(
+                'Template error when formatting certificate data'))
+        prepared_template = self._wrap_rule(rendered, 'syntax')
         return self.SyntaxRule(prepared_template, is_extension)
 
 
@@ -489,6 +514,12 @@ class CertutilFormatter(Formatter):
     def format(self, syntax_rules, render_data):
         rendered = self._format(
             'certutil_base.tmpl', {'options': syntax_rules}, render_data)
+
+        if not ' -s ' in rendered['script']:
+            raise errors.CertificateMappingError(reason=_(
+                'Certificate subject could not be generated. You may need to'
+                ' use a different certificate profile for this principal.'))
+
         return rendered
 
 
-- 
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