[Pki-devel] [PATCH] 0048-0049 Lightweight CAs: implement deletion

[Ade Lee]

On Thu, 2015-10-01 at 22:50 +1000, Fraser Tweedale wrote:
> On Wed, Sep 30, 2015 at 03:59:55PM -0400, Ade Lee wrote:
> > On Wed, 2015-09-30 at 11:26 -0700, Christina Fu wrote:
> > See other comments below, but the gist of Christina's comments are:
> > 1.  We need to look up the parent and revoke the subca signing 
> > cert.
> > 2.  We need to restrict Delete to Leaf CAs.  So if you have Host CA 
> > ->
> > sub ca1 -> sub ca2 -> subca 3,
> >      you cannot delete sub CA 1 without deleting subca3 and subca2 
> > (in
> > that order).
> > 3.  The mothballed state Christina refers to is what we now call 
> > the
> > disabled state.  In this case,
> >      we should not be able to issue new certs (including subca 
> > certs). 
> >  We should however still be able to
> >     revoke certs and publish CRLs.
> > 
> Thanks Christina and Ade for the feedback.  The updated patch
> (attached) addresses #1.  #3 is addressed by my patch 0050 (separate
> thread).
> 
> I will tackle revocation-on-delete next.  If you are happy with
> what's in the attached patched we could push it and I'll file a
> ticket and implement revocation in a separate patch, but the call is
> yours.
> 

I think you mean that the attached patch addresses #2 (instead of #1,
which is revocation).  Yes, please address revocation on delete in a
separate patch.

You will likely need to rebase.  I was unable to apply the patch as is.

On question about this patch -- In the iteration over the caMap to
determine if we have a leaf node, does this need to be synchronized?

Once the above is fixed (assuming that it needs to be), ACK.


Cheers,
> Fraser
> 
> > > Hi Fraser,
> > > 
> > > Ade caught me on irc for some feedback.  I have not had chance to
> > > look at your patches, but I did get the gist of the subca delete
> > > issues from him.
> > > Two key suggestions I have:
> > > 1. make sure the action is audited to its parent (audit log
> > > preserved)
> > > 2. make sure revocation is taken cared of of the subca's signing 
> > > cert
> > > (and therefore invalidating all its signed certs)
> > >    - and make sure the root CA is never deleted, so that the crls
> > > could be preserved and referenced to;
> > >    - and note that ocsp will no longer work for the subca that is
> > > deleted, as the signing cert is gone for good
> > > 
> > > Regarding keeping the root CA, we had discussion on possibly 
> > > keeping
> > > it in a "mothballed state"...I'll let Ade add to this.
> > > 
> > > thanks,
> > > Christina
> > > 
> > > On 09/30/2015 07:00 AM, Fraser Tweedale wrote:
> > > > Updated patch attached. Comments inline.
> > > > 
> > > > On Wed, Sep 30, 2015 at 06:35:57PM +1000, Fraser Tweedale 
> > > > wrote:
> > > > > > 3) It would be good to have a "Are you sure?" dialog on the 
> > > > > > CLI
> > > > > > (with
> > > > > > relevant override option).
> > > > > > 
> > > > > Will do.
> > > > > 
> > > > Done.
> > > > 
> > > > > > 5) I have been thinking about ways to restrict delete.  We
> > > > > > should 
> > > > > >    discuss and decide on options.  Some ideas:
> > > > > > 
> > > > > >    a) Add CS.cfg option to disable deletes (for production
> > > > > > say).
> > > > > > 
> > > > > Disagree; don't want more config in flat files.  Having the 
> > > > > knob
> > > > > in
> > > > > the database would be better but I prefer a combination of 
> > > > > other
> > > > > options (see below).
> > > > > 
> > > > > >    b) Add optional field (deletable) to the CA entry.  This 
> > > > > > can
> > > > > > be
> > > > > >       set by the creating admin to be True for test
> > > > > > environments or
> > > > > >       cases where we know the environment will be short 
> > > > > > lived,
> > > > > > or
> > > > > >       False for long lived CAs.  Default could be 
> > > > > > configurable.
> > > > > > 
> > > > > >       CAs could still be deleted, but only by doing 
> > > > > > something
> > > > > >       out-of-band --like modifying the db entry using pki
> > > > > > -server
> > > > > >       commands or similar.
> > > > > > 
> > > > > >    c) Requiring CAs to be disabled before deleting them.
> > > > > > 
> > > > > I'm in favour of this.
> > > > > 
> > > > > >    d) Setting a separate ACL for delete, so that it would 
> > > > > > be
> > > > > > easier
> > > > > >       for admins to set special permissions for delete.
> > > > > > 
> > > > > And in favour of this.
> > > > > 
> > > > > >    ... others?
> > > > > > 
> > > > > I like (c) plus (d) plus perhaps a pkispawn knob that 
> > > > > controls
> > > > > whether the admin-can-delete ACL gets added at the beginning.
> > > > > 
> > > > > Let me know what you think and thanks for your feedback!
> > > > > 
> > > > (c) and (d) are implemented in updated patch.  If you agree 
> > > > with
> > > > (c)
> > > > plus (d) plus pkispawn knob (I guess we'll call that (e)), I'll
> > > > file
> > > > a ticket for (e).
> > > > 
> > I'm OK with (c) and (d) and others appear to be too.  Archiving is
> > difficult because it basically won't work with an HSM.
> > I suppose this is just equivalent to the controls we have in 
> > revoking
> > the host CA signing cert.
> > I don't think (e) is needed.
> >