On Thu, 13 Aug 2015, Fraser Tweedale wrote:
On Thu, Aug 13, 2015 at 12:01:09PM +0300, Alexander Bokovoy wrote:
On Thu, 13 Aug 2015, Fraser Tweedale wrote:
>On Thu, Aug 13, 2015 at 09:53:35AM +0300, Alexander Bokovoy wrote:
>>On Thu, 13 Aug 2015, Fraser Tweedale wrote:
>>>The attached patch fixes
>>>From 0dd316bf0cbab7b6701bd69f142e82b30bee25b8 Mon Sep 17 00:00:00 2001
>>>From: Fraser Tweedale <ftwee...@redhat.com>
>>>Date: Thu, 13 Aug 2015 02:32:54 -0400
>>>Subject: [PATCH] Prohibit deletion of included profiles
>>>Deletion of included profiles, including the default profile, should
>>>not be allowed.  Detect this case and raise an error.
>>>Also update the included profiles collection to use namedtuple,
>>>making it easier to access the various components.
>>>Fixes: https://fedorahosted.org/freeipa/ticket/5198
>>>ipalib/plugins/certprofile.py | 13 +++++++++++--
>>>ipapython/dogtag.py           |  8 +++++---
>>>2 files changed, 16 insertions(+), 5 deletions(-)
>>>diff --git a/ipalib/plugins/certprofile.py b/ipalib/plugins/certprofile.py
1dd4f403ee4461b83c053eb36019a8896506bb81..03bdd28728dc864adcd7305ddbff34a23405e78f 100644
>>>--- a/ipalib/plugins/certprofile.py
>>>+++ b/ipalib/plugins/certprofile.py
>>>@@ -3,6 +3,7 @@
>>>import re
>>>+from operator import attrgetter
>>>from ipalib import api, Bool, File, Str
>>>from ipalib import output, util
>>>@@ -14,6 +15,7 @@ from ipalib.plugins.baseldap import (
>>>from ipalib.request import context
>>>from ipalib import ngettext
>>>from ipalib.text import _
>>>+from ipapython.dogtag import INCLUDED_PROFILES
>>>from ipapython.version import API_VERSION
>>>from ipalib import errors
>>>@@ -287,9 +289,16 @@ class certprofile_del(LDAPDelete):
>>>    __doc__ = _("Delete a Certificate Profile.")
>>>    msg_summary = _('Deleted profile "%(value)s"')
>>>-    def execute(self, *args, **kwargs):
>>>+    def pre_callback(self, ldap, dn, *keys, **options):
>>>        ca_enabled_check()
>>>-        return super(certprofile_del, self).execute(*args, **kwargs)
>>>+        if keys[0] in map(attrgetter('profile_id'), INCLUDED_PROFILES):
>>>+            raise errors.ValidationError(name='profile_id',
>>>+                error=_("Included profile '%(profile_id)s' cannot be 
>>>+                    % {'profile_id': keys[0]}
>>>+            )
>>>+        return dn
>>I think you also want to protect the included profiles from renaming.
>This is already the case.
I'm also wondering about certprofile-mod changing the profile content
and changing profileID there to point to existing profile. Would this
affect CA operation?

Renaming profile / changing profile-id / pointing it to a different
profile is not possible.

Changing profile content *is* currently possible.  Given that we
have custom profiles now, there is an argument to be made that we
should prevent profile-mod for updating the Dogtag configuration of
predefined profiles.

If we did that, we would probably also want to allow admins to
change which is the default profile, i.e. changing the default to
some custom profile they added.

And if we did that, then perhaps we should let them specify a
different default profile for users vs hosts/services!

How deep does this rabbit hole go? :)
All the above makes sense and should be done in terms of proper
hardening and usability fixes. I don't think it is a bottomless hole,
though, just a normal work we have to do to make certificate profiles
nice and usable :)

/ Alexander Bokovoy

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to