[Freeipa-devel] [PATCH] 0039 Prohibit deletion of included profiles

[Alexander Bokovoy]

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
>> >>>https://fedorahosted.org/freeipa/ticket/5198
>> >>>
>> >>>Thanks,
>> >>>Fraser
>> >>
>> >>>From 0dd316bf0cbab7b6701bd69f142e82b30bee25b8 Mon Sep 17 00:00:00 2001
>> >>>From: Fraser Tweedale <ftweedal at 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
>> >>>index 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 deleted")
>> >>>+                    % {'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