[Freeipa-devel] V4/Sub-CAs review

Tue Jun 7 04:30:11 UTC 2016

On Mon, Jun 06, 2016 at 08:29:16AM +0200, Jan Cholasta wrote:
> On 1.6.2016 06:49, Fraser Tweedale wrote:
> > On Mon, May 23, 2016 at 10:02:44AM +0200, Jan Cholasta wrote:
> > > > > > > 2) <http://www.freeipa.org/page/V4/Sub-CAs#Sub-CA_plugin>
> > > > > > > 
> > > > > > > It should be mentioned here that the primary CA is also handled by this
> > > > > > > plugin.
> > > > > > > 
> > > > > > > I would like to propose two additional fields:
> > > > > > > 
> > > > > > >   * subject (required) - subject name of the CA, to be able to look up
> > > > > > > sub-CA that issued a certificate from its issuer name.
> > > > > > > 
> > > > > > >   * issuer_ca (optional) - name of the sub-CA which issued certificate for
> > > > > > > this CA, to have information about the sub-CA hierarchy. If there is no
> > > > > > > sub-CA entry for the issuer, it would be unset.
> > > > > > > 
> > > > > > These data exist in the Dogtag database.  Adding them to IPA might
> > > > > > be useful for avoiding round trips so it is probably worth doing,
> > > > > > but are you aware of use cases that would absolutely require them?
> > > > > 
> > > > > As for subject, we are adding the ability to look up information about
> > > > > arbitrary certificates to cert-{show,find} as part of
> > > > > <https://fedorahosted.org/freeipa/ticket/5381>. Part of this information
> > > > > should be whether the certificate was issued by our CA and what CA it was,
> > > > > so that the web UI can present an appropriate "revoke certificate" button
> > > > > for the certificate.
> > > > > 
> > > > > As for issuer CA, I believe we need it to fix automatic CA certificate
> > > > > renewal. The current renewal code uses virtual "profiles" to handle CA
> > > > > certificate renewal, but that turned out to be an issue, especially with
> > > > > externally signed CA certificates:
> > > > > <https://bugzilla.redhat.com/show_bug.cgi?id=1322963>. Instead it could use
> > > > > the issuer CA information from LDAP to figure out what needs to be done.
> > > > > (Note that during the renewal, Dogtag is offline.)
> > > > > 
> > > > > Also, both the attributes should be included for compatibility with external
> > > > > CAs. At this point, I think it's only a matter of time when support for them
> > > > > will be added (there were already several requests for such a feature), and
> > > > > I would very much prefer to have to maintain only a single code path for the
> > > > > generic stuff (which includes both of the attributes), instead of one for
> > > > > Dogtag and one for external CAs.
> > > > > 
> > > > OK, I'll add issuer DN and subject DN attributes to the ipaCa
> > > > objectClass.
> > > 
> > > Just to be clear, what I meant is for the issuer attribute to contain the DN
> > > of the CA entry in LDAP, not the issuer DN itself.
> > > 
> > I see; thanks for the clarification.
> 
> Actually, the more I think of this, I think the attribute should contain the
> issuer DN rather than the CA entry DN. That way it could be mandatory, thus
> simplifying the related logic, and it should make adding external CAs easier
> in the future.
> 
No problem; the latest patchset adds the issuer DN and subject DN
attributes.  I'll update the design page shortly.

> > I'm going to publish the first
> > version of the patchset soon - it will not have this implemented
> > yet, but I think it's more important for me to get patches out for
> > review ASAP, and add this aspect in a subsequent patchset.
> 
> OK, but please make sure it is done before 4.4, adding new mandatory
> attributes is a backward incompatible change, and we most certainly don't
> want to deal with that after the release.
> 
> > 
> > > > > > > """
> > > > > > > The Python script shall be installed at /usr/libexec/pki-ipa-retrieve-key
> > > > > > > and shall be executed as pkiuser.
> > > > > > > """
> > > > > > > 
> > > > > > > Could you please use a subdirectory? Like /usr/libexec/pki (if the script is
> > > > > > > going to be distributed with Dogtag) or /usr/libexec/ipa (if the script is
> > > > > > > going to be distributed with IPA).
> > > > > > > 
> > > > > > What is the rationale - is it a packaging guideline or just common
> > > > > > sense?
> > > > > 
> > > > > I'm not sure if it's an actual guideline, but IMHO it's definitely common
> > > > > sense - I don't think littering the global namespace (i.e. /usr/libexec) is
> > > > > ever preferable to keeping your stuff in your own namespace.
> > > > > 
> > > > I'll drop the script in a subdir.  While I'm at it, I think I will
> > > > move it to the IPA codebase, to improve locality of the Python code.
> > > > e.g. if CustodiaClient API or any other IPA Python API changes, the
> > > > code in pki repo will be too easily missed.
> > > 
> > > OK, makes sense.
> > > 
> > Latest version of patch 0054 installs the helper program under
> > /usr/libexec/ipa.
> 
> OK.
> 
> > 
> > > > > Please don't use ipaUniqueId as the RDN unless absolutely necessary. Not
> > > > > only it makes debugging harder (because you can't tell which object is which
> > > > > just by looking at the DN), it also requires the framework to do an extra
> > > > > LDAP search every time the DN needs to be translated to primary key.
> > > > > 
> > > > If cn is used in RDN, will changing cn (which then will be a modrdn
> > > > operation) correctly update the references from CA ACLs?
> > > 
> > > Yes, the referint DS plugin takes care of that.
> > > 
> > cn it is; no more ipaUniqueId.
> 
> OK.
> 
> > 
> > > > 
> > > > > "host-authority" does not strike me as something familiar, and the "host"
> > > > > bit is kind of confusing, since it is not at all related to IPA hosts. Could
> > > > > we use something more obvious ("default", "root", ...)?
> > > > > 
> > > > We shouldn't use "root" because it might not be a root CA.
> > > > 
> > > > We probably shouldn't use "default" because we might later want to
> > > > allow different default CAs for different profiles or principal
> > > > types.
> > > 
> > > What about "main"?
> > > 
> > > > 
> > > > Perhaps simply "IPA CA", "ipa", or some variation on that theme?
> > > 
> > > Something like this might be the best, as we currently refer to this CA as
> > > "IPA CA".
> > > 
> > I went with 'cn=ipa' and 'description=IPA CA'.
> 
> OK.
> 
> > 
> > > > 
> > > > As discussed above, there will now also be attributes for issuer DN
> > > > and subject DN.
> > > > 
> > > > > Why does a Dogtag lightweight CA need to be created before the LDAP entry? I
> > > > > assume it's because deleting an LDAP entry is easier than deleting a Dogtag
> > > > > lightweight CA in case something goes wrong, in which case I think ipaCaId
> > > > > should be required and use a placeholder value in the initial LDAP entry.
> > > > > 
> > > > The IPA object is created first to ensure that the user has the
> > > > permissions to do so.  Apart from that it is not important which
> > > > happens first.  I can check permissions with can_add() but defer IPA
> > > > object creation until after the Dogtag LWCA has been created.  In
> > > > fact this is a cleaner approach.
> > > 
> > > Yes, I agree.
> > > 
> > Everything works fine with can_add(); design updated.
> 
> OK.
> 
> > 
> > > > ca-del results in deletion of the signing key from Dogtag's NSSDB,
> > > > and deletion of the Dogtag lightweight authority object.  On IPA
> > > > side it removes the ipaCa object, removes references from CA ACLs,
> > > > etc.  ca-del should be a command.
> > > 
> > > OK, but should the certificates issued by the CA stay valid? I would think
> > > not - when authorization is based on group membership and the group is
> > > deleted, it is no longer possible to authorize - when authorization is based
> > > on the CA, I assume it should work analogically and authorization should
> > > stop working when the CA is deleted. Or am I missing something?
> > > 
> > It makes sense to revoke the CA certificate when the CA is deleted.
> > There is a Dogtag ticket: https://fedorahosted.org/pki/ticket/1638.
> > 
> > After that is implemented, I don't think extra behaviour is needed
> > in IPA - but we should clearly document what happens with revocation
> > when a CA is deleted.
> > 
> > Until it is implemented, revocation can be performed manually.
> > 
> > (I'll add the preceding commentary to the design page.)
> 
> OK.
> 
> > 
> > > > 
> > > > We can add ca-disable and ca-enable - these behaviours are
> > > > implemented in Dogtag already.  I prefer the more fine-grained
> > > > control that CA ACLs give you, but I suppose people will want
> > > > wholesale enable/disable as well?  Or is it premature to assume so?
> > > 
> > > I think it can be added later. The advantage over CA ACLs is that you don't
> > > have to go through potentially many ACLs to enable/disable a single CA.
> > > 
> > Agreed.
> > 
> > Thanks,
> > Fraser
> > 
> 
> 
> -- 
> Jan Cholasta