[Freeipa-devel] [DESIGN] IPA permission enforcement in Dogtag

[Fraser Tweedale]

On Mon, Feb 06, 2017 at 10:24:31AM +0100, Jan Cholasta wrote:
> On 17.1.2017 08:57, David Kupka wrote:
> > On 13/01/17 08:07, Fraser Tweedale wrote:
> > > Related to design:
> > > http://www.freeipa.org/page/V4/Dogtag_GSS-API_Authentication
> > > 
> > > Currently there are some operations that hit the CA that involve a
> > > number of privileged operations against the CA, but for which there
> > > is only one associated IPA permission.  Deleting a CA is a good
> > > example (but it is one specific case of a more general issue).
> > > Summary of current ca-del behaviour:
> > > 
> > > 1. Disable LWCA in Dogtag (uses RA Agent cert)
> > > 2. Delete LWCA in Dogtag (uses RA Agent cert)
> > > 3. Delete CA entry from IPA (requires "System: Delete CA" permission)
> > > 
> > > So there are two things going on under the hood: a modify operation
> > > (disable CA) and the delete.
> > > 
> > > When we implement proxy authentication to Dogtag, Dogtag will
> > > enforce the IPA permissions on its operations.  Disable will map to
> > > "System: Modify CA" and delete to "System: Delete CA".  So to delete
> > > a CA a user will need *both* permissions.  Which could be
> > > surprising.
> > > 
> > > There are a couple of reasonable approaches to this.
> > > 
> > > 1. Decouple the disable and delete operations.  If CA is not
> > > disabled, the user will be instructed to execute the ca-disable
> > > command separately before they can disable the CA.  This introduces
> > > an additional manual step for operators.
> > > 
> > > 2. Just improve the error reporting.  In my WIP, for a user that has
> > > 'System: Delete CA' permission but not 'System: Modify CA', the
> > > reported failure is a 403 Authorization Error from Dogtag.  We can
> > > add guards to fail more gracefully.
> > > 
> > > I lean towards #2 because I guess the common case will be that users
> > > either get all CA admin permissions, or none, and we don't want to
> > > make more work (in the form of more commands to run) for users in
> > > the common case.
> > > 
> > > I welcome alternative views and suggestions.
> > > 
> > > Thanks,
> > > Fraser
> > > 
> > Hi Fraser,
> > as a user with "System: Delete CA" permission calling "ca-del" command I
> > would be really surprised that I don't have enough privileges to
> > complete the action.
> > 
> > I would expect:
> > a) "Cannot delete active CA, disable it first" error.
> > b) Delete will be completed successfully. All internal and to my sight
> > hidden operations will be allowed just because I'm allowed to perform
> > the delete operation.
> > 
> > I think that b) might lead to strange exceptions in authorization
> > checking and therefore to security issues. So I would prefer decoupling
> > ca-disable and ca-del as you're describing in 1).
> 
> IMO having to disable the CA before deletion is an implementation detail and
> should not be exposed to the user at all. Why do we have to disable the CA
> from IPA in ca-del? I would expect Dogtag to disable it itself internally
> when it's being deleted.
> 
The CA requiring disablement before deletion is a property of how
Dogtag Lightweight CAs are implement.  I don't intend to change this
(besides, it might need to be this way for Common Criteria; a
similar restriction exists for profiles).

We could make it so that in IPA context, delete permission implies
disable permission.  Currently (in Dogtag) permission to
enable/disable is the 'modify' permission.  So to do this without
implying that someone with 'delete' permission as 'modify'
permission, I'd need to add an explicit 'enable/disable ca'
permission.

This is a good idea, but it is more work to add the required ACLs
(which will need to be done during IPA upgrade or installation).
I'll shelve https://github.com/freeipa/freeipa/pull/415 for now, but
keep the patch in my working branch and code it out later, if
there's time before release.  Otherwise we might need to keep it
until there's time for the proper fix, so that things don't break.

Thanks,
Fraser