Thanks for the detailed review Ade! Comments inline. On Tue, Aug 25, 2015 at 02:40:11PM -0400, Ade Lee wrote:

Some initial comments and questions. Will have more after playing with it and testing. Looks pretty good so far.

1. what is org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true and why is it needed?

caRef / handle components are separated by "/". The Java client code translates e.g. "sub1/sub2" into the path "https://.../subca/sub1%3Fsub2", which Tomcat does not like (immediately rejects). The above config allows this. I contemplated changing the separator character - I am still open to it - but I made this smaller change for now. From ongoing IRC discussions there are other situations that might warrant changing the separator char.

2. In CAService.java: issueX509Cert() - if ca is null, this throws an ECAException - which, if I understand the code correctly, ends up being some kind of BaseException. So, if someone requests a cert and provides a bogus caref, it will be reported as a server error instead of a installation error?

Installation error? Did you mean client/enrolment error? Anyhow, I'll have to check the behaviour here, but I thought bogus carefs were resulting in the appropriate error (404); but I may have missed some cases.

3. CertificateAuthority.java -- subCAHier --> rename to subCAHierarchy.

OK.

-- In the constructor, it would be useful to have example inputs in the method documentation so that we can see what data is expected to be in each of the fields. Its hard to review/understand/maintain otherwise.

OK.

-- you have extracted initCetDatabase() and initCRLDB(), also extract the replica repo initialization code into initReplicaRepo() or similar.

Fair enough.

-- loadSubCAs - could use some description in method comments about the structure we expect to construct here. An example config would help understanding.

OK.

-- createSubCA -- returns ECAException() in many different cases ie. if the CA exists, if the parent does not exist, if there is an error in CA creation. This makes it difficult to separate out the things that are likely client request issues vs. server issues. More likely, we want different exceptions that can be caught by the caller and handled appropriately.

No problemo.

-- what about uniqueness checks for the issuer DN?

This was on the TODO list. Now that we have live info on all sub-CAs it will be straightforward and I will include it in the next patchset.

4. SubCAResource.java -- path should be subcas ? not subca ..

OK.

-- acls needed of course

Yep; next patchset.

5. CAEnrollProfile.java --> could not reach CA -- that ends up being a ProfileException -- which maps to a server error? Is this a case where we need a bad request exception instead?

Yes, user requesting nonexistant CA should be a 404. Will confirm / fix for next patchset.

6. subca.schema -- why is it specified in a separate file? (and also in schema.ldif)

FreeIPA will use this file to update the DS schema. Similar thing was done with schema for LDAP-based profiles. Necessary workaround until Dogtag has its own schema update mechanism.

-- the subca is uniquely defined by only one MAY attribute for nickname (which is in printable string format)? Is that sufficient and should that be a required attribute? Do you need to store the issuerDN for uniqueness checks?

Well, it is uniquely defined ("distinguished" ^_^) by its DN. I see no harm in adding issuerDN to the object though; it may turn out to be useful for other things.

7. SubCAService.java -- lets remove the commented out audit messages and functions

They are there to remind me to put audit messages in :)

-- createSubCA() -- should be some checks here on the data -- rather than passing through to the lower level. Bad data should return BadRequestException, including for cases where we have existing issuerDN or caRef.

OK

8. SubCACreateCLI -- this code would be confusing:

if (cmdArgs.length < positionalArgNames.length) { System.err.println("Error: No " + positionalArgNames[cmdArgs.length] + " specified.");

Just specify "Insufficient params .."

OK, thanks.

9. It would be good to have a DN check here on client side -- this can be added in a separate patch.

Makes sense.

10. Should users be able to define the nickname? I would say "yes" because it might make it easier to notify services like custodia for instance to distribute keys.

Not sure about that. Custodia will just replicate any new subCA keys it finds (by monitoring / polling the directory). If we choose a sensible name that indicates which CA they key is associated with, I'm not sure what benefit it would have to allow users to choose the nickname - in fact, I see it as a potential source of user confusion / errors. This is not a "no" - it's certainly doable; just want to make sure there's a good use case.

11. Should we also have the option to define the token in which the key is generated and stored? ( I think yes - in case for instance, your HSM has limited keys). Where do the subCA keys get generated by default -- in the software or hardware token?

Currently in softtoken. Need to think about this; I'll put an item on TODO list so it doesn't get forgotten.

12. To help clarify this, please describe what would be created if one were to add a new subCA using the CLI at reference caRef

Specifically, a) what database entry is created? b) what is the nickname for the key/cert? c) Where is the repo (ie. the suffix) for this ca's certs? d) Exactly which resources belong to the subCA only?

In brief (I will add detailed commentary in next patchset): a) An entry is created at ``ou=ca1,ou=subCAs,ou=ca,{rootSuffix}`` (prepend more ou=foo for nested sub-CAs), with objectClass ``subCA``. b) Nickname is the signing key nickname with the caRef appended, except slashes are replaced with ",". e.g for subCA "puppet/master" the nickname is "caSigningCert cert-pki-ca puppet,master" c) All certs live in the one repo, and the one repo is used for assigning serial numbers. d) SigningUnit (and associated key in NSSDB), and the sub-CA's LDAP entry. That *should* be all - other resources should be shared with top-level CA. (Side-note: each subca *temporarily* holds an LDAP conn factory and connection during initialisation and when creating a subca, but destroys them when done)

MISSING ITEMS: These can be in a separate patch, but if so, we need tickets to track them:

1. acls 2. auditing 3. key parameters 4. migration scripts (i.e. how to add db entries) 5. tests - we need unit/functional tests to show the data that is expected and to do basic error checking in the client. These are very important. Eventually, we would like tests to be a required part of any check-in.

acls and (probably) audit will be in upcoming revision of this patch. I'll file tickets for other bits now. Thanks! Fraser

Ade

On Mon, 2015-08-24 at 17:27 +1000, Fraser Tweedale wrote:

...

Hi team,

The latest sub-CAs patches are attached. It has been a while since the last patchset (that was posted here, anyway) and there have been some significant changes, outlined below. (The patchset version skipped a couple numbers due to versions distributed privately that I felt were not stable enough to warrant posting to pki-devel.)

Major changes:

- The Java client and CLI were extracted to a separate patch (0044). - An LDAP entry for each sub-CA is written to database. - Database searched and sub-CAs are initialised at startup - Key nickname is store in / read from LDAP entry - Sub-CA "list" API call, client method and CLI was added - More resources are shared between top-level CA and sub-CAs - Suprious task threads and LDAP connections hunted down :)

Dependencies:

- Patch 0026-5 probably depends on 0045[1] for a clean merge. - Patch 0044-3 depends on my patch 0046[2].

[1] https://www.redhat.com/archives/pki-devel/2015-August/msg00072.html [2] https://www.redhat.com/archives/pki-devel/2015-August/msg00073.html

Cheers, Fraser

On Wed, Aug 26, 2015 at 02:09:22AM -0400, Ade Lee wrote:

Some more thoughts.

First off, there are things that should be included with the CAData for each subsystem.

1. CA signing cert (or more likely a reference to the ca cert)

Since we are using a shared cert repo, I think serial suffices for our purposes. Issuer + serial for completeness (probably as separate attibutes, for simplicity).

2. pkcs7 chain (again this is something that could/should likely be a reference)

Yes, references and construct on demand.

3. status - ie. enabled/disabled.

Agreed. I'll implement this in next patchset. Should initial status be enabled or disabled? (After hearing some Common Criteria stories, my hunch says "disabled", but would like other opinions).

4. Description -- user defined description.

Ok.

For the motivation for (30 status, we do need to consider what we want do when someone wants to delete or decommission a subCA. If we want to keep the subCA record -- and we almost certainly need to -- we should probably add some kind of field to enable/disable.

Second, we definitely need code to be able to issue a cert using the subcas from the REST interface. This will allow us to actually test that the subCA is working.

Third, if we do stick with the current REST API, I think we should consider using something like:

POST /subcas/ca1/ca2 {"id": "ca3"} to generate a subca /ca1/ca2/ca3

Fourth, though, we need to consider carefully whether or not it makes sense to identify a subca through 1) an identifier that contains slashes 2) an identifier that is defined by the user instead of the server. We have currently no controls as to what names are chosen, and no clear rationale for what should be chosen.

Well, it is the same with profiles - user chooses the name, not us.

Much as I dislike uuids, there is a good reasons openstack uses them everywhere.

I like the idea that the identifier encodes the "path" but it is not essential; we just need a way to say which CA (identified by e.g. UUID, or unique "shortname") to create a sub-CA under. What do other projects do with their APIs for hierarchical objects? Is the API itself nested, or flat with data containing references to parent? Does OpenStack have wisdom for us? Thanks, Fraser

We can discuss over IRC, but I can see many issues here.

Ade

On Tue, 2015-08-25 at 14:40 -0400, Ade Lee wrote:

...

Some initial comments and questions. Will have more after playing with it and testing. Looks pretty good so far.

1. what is org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true and why is it needed?

2. In CAService.java: issueX509Cert() - if ca is null, this throws an ECAException - which, if I understand the code correctly, ends up being some kind of BaseException. So, if someone requests a cert and provides a bogus caref, it will be reported as a server error instead of a installation error?

3. CertificateAuthority.java -- subCAHier --> rename to subCAHierarchy.

-- In the constructor, it would be useful to have example inputs in the method documentation so that we can see what data is expected to be in each of the fields. Its hard to review/understand/maintain otherwise.

-- you have extracted initCetDatabase() and initCRLDB(), also extract the replica repo initialization code into initReplicaRepo() or similar.

-- loadSubCAs - could use some description in method comments about the structure we expect to construct here. An example config would help understanding.

-- createSubCA -- returns ECAException() in many different cases ie. if the CA exists, if the parent does not exist, if there is an error in CA creation. This makes it difficult to separate out the things that are likely client request issues vs. server issues. More likely, we want different exceptions that can be caught by the caller and handled appropriately. -- what about uniqueness checks for the issuer DN?

4. SubCAResource.java -- path should be subcas ? not subca .. -- acls needed of course

5. CAEnrollProfile.java --> could not reach CA -- that ends up being a ProfileException -- which maps to a server error? Is this a case where we need a bad request exception instead?

6. subca.schema -- why is it specified in a separate file? (and also in schema.ldif) -- the subca is uniquely defined by only one MAY attribute for nickname (which is in printable string format)? Is that sufficient and should that be a required attribute? Do you need to store the issuerDN for uniqueness checks?

7. SubCAService.java -- lets remove the commented out audit messages and functions -- createSubCA() -- should be some checks here on the data -- rather than passing through to the lower level. Bad data should return BadRequestException, including for cases where we have existing issuerDN or caRef.

8. SubCACreateCLI -- this code would be confusing:

if (cmdArgs.length < positionalArgNames.length) { System.err.println("Error: No " + positionalArgNames[cmdArgs.length] + " specified.");

Just specify "Insufficient params .."

9. It would be good to have a DN check here on client side -- this can be added in a separate patch.

10. Should users be able to define the nickname? I would say "yes" because it might make it easier to notify services like custodia for instance to distribute keys.

11. Should we also have the option to define the token in which the key is generated and stored? ( I think yes - in case for instance, your HSM has limited keys). Where do the subCA keys get generated by default -- in the software or hardware token?

12. To help clarify this, please describe what would be created if one were to add a new subCA using the CLI at reference caRef

Specifically, a) what database entry is created? b) what is the nickname for the key/cert? c) Where is the repo (ie. the suffix) for this ca's certs? d) Exactly which resources belong to the subCA only?

8. SubCACreateCLI -- this code would be confusing:

if (cmdArgs.length < positionalArgNames.length) { System.err.println("Error: No " + positionalArgNames[cmdArgs.length] + " specified.");

Just specify "Insufficient params .."

9. It would be good to have a DN check here on client side -- this can be added in a separate patch.

10. Should users be able to define the nickname? I would say "yes" because it might make it easier to notify services like custodia for instance to distribute keys.

11. Should we also have the option to define the token in which the key is generated and stored? ( I think yes - in case for instance, your HSM has limited keys). Where do the subCA keys get generated by default -- in the software or hardware token?

12. To help clarify this, please describe what would be created if one were to add a new subCA using the CLI at reference caRef

Specifically, a) what database entry is created? b) what is the nickname for the key/cert? c) Where is the repo (ie. the suffix) for this ca's certs? d) Exactly which resources belong to the subCA only?

MISSING ITEMS: These can be in a separate patch, but if so, we need tickets to track them:

1. acls 2. auditing 3. key parameters 4. migration scripts (i.e. how to add db entries) 5. tests - we need unit/functional tests to show the data that is expected and to do basic error checking in the client. These are very important. Eventually, we would like tests to be a required part of any check-in.

Ade

On Mon, 2015-08-24 at 17:27 +1000, Fraser Tweedale wrote:

...

Hi team,

The latest sub-CAs patches are attached. It has been a while since the last patchset (that was posted here, anyway) and there have been some significant changes, outlined below. (The patchset version skipped a couple numbers due to versions distributed privately that I felt were not stable enough to warrant posting to pki-devel.)

Major changes:

- The Java client and CLI were extracted to a separate patch (0044). - An LDAP entry for each sub-CA is written to database. - Database searched and sub-CAs are initialised at startup - Key nickname is store in / read from LDAP entry - Sub-CA "list" API call, client method and CLI was added - More resources are shared between top-level CA and sub-CAs - Suprious task threads and LDAP connections hunted down :)

Dependencies:

- Patch 0026-5 probably depends on 0045[1] for a clean merge. - Patch 0044-3 depends on my patch 0046[2].

[1] https://www.redhat.com/archives/pki-devel/2015-August/msg00072.html [2] https://www.redhat.com/archives/pki-devel/2015-August/msg00073.html

Cheers, Fraser

_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel

Some more comments to clarify my concerns on the use of hierarchical urls as the identifier for subcas. This is the "usual" way (ie. for example OpenStack) REST interfaces are defined: POST /subcas -- creates a subca and returns the reference /subcas/{uuid} POST /subcas/ -- POST includes "parent_ref"=http://../subcas/{uuid}" ; returns a subca that has the relevant parent GET /subcas -- list all subcas GET /subcas/{uuid} - get details on subca. GET /subcas/{uuid}/subcas -- list subcas for subca {uuid} An alternate formulation could also be: POST /subcas/{uuid}/subcas -- creates subca with subca {uuid} as parent. I don't like this as much because its a little less standard. The resulting resource would be /subcas/{uuid2} rather than /subcas/{uuid}/subcas/{uuid2} Now, I hate uuids. I mean whats not to love about "2a549393-0710-444b -8aa5-84cf0f85ea79" ? It just rolls off the tongue. Everyone hates uuids but thats actually an advantage, rather than a weakness in that no one possibly would want to change the URL. Handles on the other hand - are subject to all kinds of bike shedding. Do I use something like "ca1", "ca2"? Thats no better than a UUID in terms of semantic content. How about org references? "foo_it_dept", "bar_domain"? Uh oh, what if I decide later I didn't like my handle? What if there is an org rename? Or a reorg? Remember, certs can potentially live a long time. And what if I'm really perverse and use the handle "subcas" -- aargh! now I've broken one of the possible paths above. Or I might end up breaking a path we'll add in future. No - the only way to keep this straight is to have the resource identifier be something unique that no one will want to change -- like /subcas/{uuid} - and have some kind of alias system for that subca that can be changed at will. That mapping could be kept in IPA or Barbican database for instance, rather that in dogtag itself. We can also do what we did for secrets and allow the user to define a "client nickname" as part of the subca record. This is a field that can later be searched for or changed. As for hierachical-ness, flatter is easier to understand and maintain. Ade On Wed, 2015-08-26 at 02:09 -0400, Ade Lee wrote:

Some more thoughts.

First off, there are things that should be included with the CAData for each subsystem.

1. CA signing cert (or more likely a reference to the ca cert) 2. pkcs7 chain (again this is something that could/should likely be a reference) 3. status - ie. enabled/disabled. 4. Description -- user defined description.

For the motivation for (30 status, we do need to consider what we want do when someone wants to delete or decommission a subCA. If we want to keep the subCA record -- and we almost certainly need to -- we should probably add some kind of field to enable/disable.

Second, we definitely need code to be able to issue a cert using the subcas from the REST interface. This will allow us to actually test that the subCA is working.

Third, if we do stick with the current REST API, I think we should consider using something like:

POST /subcas/ca1/ca2 {"id": "ca3"} to generate a subca /ca1/ca2/ca3

Fourth, though, we need to consider carefully whether or not it makes sense to identify a subca through 1) an identifier that contains slashes 2) an identifier that is defined by the user instead of the server. We have currently no controls as to what names are chosen, and no clear rationale for what should be chosen.

Much as I dislike uuids, there is a good reasons openstack uses them everywhere.

We can discuss over IRC, but I can see many issues here.

Ade

On Tue, 2015-08-25 at 14:40 -0400, Ade Lee wrote:

...

Some initial comments and questions. Will have more after playing with it and testing. Looks pretty good so far.

1. what is org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true and why is it needed?

2. In CAService.java: issueX509Cert() - if ca is null, this throws an ECAException - which, if I understand the code correctly, ends up being some kind of BaseException. So, if someone requests a cert and provides a bogus caref, it will be reported as a server error instead of a installation error?

3. CertificateAuthority.java -- subCAHier --> rename to subCAHierarchy.

-- In the constructor, it would be useful to have example inputs in the method documentation so that we can see what data is expected to be in each of the fields. Its hard to review/understand/maintain otherwise.

-- you have extracted initCetDatabase() and initCRLDB(), also extract the replica repo initialization code into initReplicaRepo() or similar.

-- loadSubCAs - could use some description in method comments about the structure we expect to construct here. An example config would help understanding.

-- createSubCA -- returns ECAException() in many different cases ie. if the CA exists, if the parent does not exist, if there is an error in CA creation. This makes it difficult to separate out the things that are likely client request issues vs. server issues. More likely, we want different exceptions that can be caught by the caller and handled appropriately. -- what about uniqueness checks for the issuer DN?

4. SubCAResource.java -- path should be subcas ? not subca .. -- acls needed of course

5. CAEnrollProfile.java --> could not reach CA -- that ends up being a ProfileException -- which maps to a server error? Is this a case where we need a bad request exception instead?

6. subca.schema -- why is it specified in a separate file? (and also in schema.ldif) -- the subca is uniquely defined by only one MAY attribute for nickname (which is in printable string format)? Is that sufficient and should that be a required attribute? Do you need to store the issuerDN for uniqueness checks?

7. SubCAService.java -- lets remove the commented out audit messages and functions -- createSubCA() -- should be some checks here on the data -- rather than passing through to the lower level. Bad data should return BadRequestException, including for cases where we have existing issuerDN or caRef.

8. SubCACreateCLI -- this code would be confusing:

if (cmdArgs.length < positionalArgNames.length) { System.err.println("Error: No " + positionalArgNames[cmdArgs.length] + " specified.");

Just specify "Insufficient params .."

9. It would be good to have a DN check here on client side -- this can be added in a separate patch.

10. Should users be able to define the nickname? I would say "yes" because it might make it easier to notify services like custodia for instance to distribute keys.

11. Should we also have the option to define the token in which the key is generated and stored? ( I think yes - in case for instance, your HSM has limited keys). Where do the subCA keys get generated by default -- in the software or hardware token?

12. To help clarify this, please describe what would be created if one were to add a new subCA using the CLI at reference caRef

Specifically, a) what database entry is created? b) what is the nickname for the key/cert? c) Where is the repo (ie. the suffix) for this ca's certs? d) Exactly which resources belong to the subCA only?

8. SubCACreateCLI -- this code would be confusing:

if (cmdArgs.length < positionalArgNames.length) { System.err.println("Error: No " + positionalArgNames[cmdArgs.length] + " specified.");

Just specify "Insufficient params .."

9. It would be good to have a DN check here on client side -- this can be added in a separate patch.

10. Should users be able to define the nickname? I would say "yes" because it might make it easier to notify services like custodia for instance to distribute keys.

11. Should we also have the option to define the token in which the key is generated and stored? ( I think yes - in case for instance, your HSM has limited keys). Where do the subCA keys get generated by default -- in the software or hardware token?

12. To help clarify this, please describe what would be created if one were to add a new subCA using the CLI at reference caRef

Specifically, a) what database entry is created? b) what is the nickname for the key/cert? c) Where is the repo (ie. the suffix) for this ca's certs? d) Exactly which resources belong to the subCA only?

MISSING ITEMS: These can be in a separate patch, but if so, we need tickets to track them:

1. acls 2. auditing 3. key parameters 4. migration scripts (i.e. how to add db entries) 5. tests - we need unit/functional tests to show the data that is expected and to do basic error checking in the client. These are very important. Eventually, we would like tests to be a required part of any check-in.

Ade

On Mon, 2015-08-24 at 17:27 +1000, Fraser Tweedale wrote:

...

Hi team,

The latest sub-CAs patches are attached. It has been a while since the last patchset (that was posted here, anyway) and there have been some significant changes, outlined below. (The patchset version skipped a couple numbers due to versions distributed privately that I felt were not stable enough to warrant posting to pki-devel.)

Major changes:

- The Java client and CLI were extracted to a separate patch (0044). - An LDAP entry for each sub-CA is written to database. - Database searched and sub-CAs are initialised at startup - Key nickname is store in / read from LDAP entry - Sub-CA "list" API call, client method and CLI was added - More resources are shared between top-level CA and sub-CAs - Suprious task threads and LDAP connections hunted down :)

Dependencies:

- Patch 0026-5 probably depends on 0045[1] for a clean merge. - Patch 0044-3 depends on my patch 0046[2].

[1] https://www.redhat.com/archives/pki-devel/2015-August/msg00072.ht ml [2] https://www.redhat.com/archives/pki-devel/2015-August/msg00073.ht ml

Cheers, Fraser

_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel

_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel

On Thu, 2015-08-27 at 17:22 +1000, Fraser Tweedale wrote:

On Wed, Aug 26, 2015 at 11:09:24AM -0400, Ade Lee wrote:

...

Some more comments to clarify my concerns on the use of hierarchical urls as the identifier for subcas.

This is the "usual" way (ie. for example OpenStack) REST interfaces are defined:

POST /subcas -- creates a subca and returns the reference /subcas/{uuid}

POST /subcas/ -- POST includes "parent_ref"=http://../subcas/{uuid}" ; returns a subca that has the relevant parent

Full URL reference mandatory here? Preferable? Much-of-a-muchness? Would a plain UUID suffice?

If URL is the way to go here, is it necessary to ensure that the host:port refers to "this" server, or can we just ignore the front matter and go straight to looking up the UUID?

I used full URL reference here because of my experience with OpenStack, where systems can be federated across multiple domains. Using URLs for all references is de rigueur there. I do not expect us to be federated in the same way, and we tend to use plain ids everywhere in the API now. Lets use a plain ID.

...

GET /subcas -- list all subcas

GET /subcas/{uuid} - get details on subca.

GET /subcas/{uuid}/subcas -- list subcas for subca {uuid}

An alternate formulation could also be:

POST /subcas/{uuid}/subcas -- creates subca with subca {uuid} as parent.

I don't like this as much because its a little less standard. The resulting resource would be /subcas/{uuid2} rather than /subcas/{uuid}/subcas/{uuid2}

Lets not use the above alternative formulation. It is as suggested non -standard and therefore less intuitive.

...

Now, I hate uuids. I mean whats not to love about "2a549393-0710 -444b -8aa5-84cf0f85ea79" ? It just rolls off the tongue.

Everyone hates uuids but thats actually an advantage, rather than a weakness in that no one possibly would want to change the URL.

Great wisdom!

...

Handles on the other hand - are subject to all kinds of bike shedding. Do I use something like "ca1", "ca2"? Thats no better than a UUID in terms of semantic content. How about org references? "foo_it_dept", "bar_domain"? Uh oh, what if I decide later I didn't like my handle? What if there is an org rename? Or a reorg? Remember, certs can potentially live a long time.

And what if I'm really perverse and use the handle "subcas" -- aargh! now I've broken one of the possible paths above. Or I might end up breaking a path we'll add in future.

No - the only way to keep this straight is to have the resource identifier be something unique that no one will want to change -- like /subcas/{uuid} - and have some kind of alias system for that subca that can be changed at will.

That mapping could be kept in IPA or Barbican database for instance, rather that in dogtag itself.

Yep. Unless you need it for feel strongly otherwise, we can avoid implementing the alias indirection in initial feature. Certainly it is not needed for IPA use case.

Agreed. Aliases can come later (if ever).

...

We can also do what we did for secrets and allow the user to define a "client nickname" as part of the subca record. This is a field that can later be searched for or changed.

As for hierachical-ness, flatter is easier to understand and maintain.

Fair enough. I will implement the scheme proposed above. Thank you for your input! Hope to have a new patchset tomorrow or early next week.

Excellent. Also, while we want to keep the hierarchy flat, we do want to keep track of which CA/subCA is the parent CA in the CA record. This should probably be the plain ID of the parent CA. On the other hand, we want to return this to the client as a fully formed URL. Its the HATEOAS thing to do. Looking forward to the patches. Ade

...

Ade

On Wed, 2015-08-26 at 02:09 -0400, Ade Lee wrote:

...

Some more thoughts.

First off, there are things that should be included with the CAData for each subsystem.

1. CA signing cert (or more likely a reference to the ca cert) 2. pkcs7 chain (again this is something that could/should likely be a reference) 3. status - ie. enabled/disabled. 4. Description -- user defined description.

For the motivation for (30 status, we do need to consider what we want do when someone wants to delete or decommission a subCA. If we want to keep the subCA record -- and we almost certainly need to -- we should probably add some kind of field to enable/disable.

Second, we definitely need code to be able to issue a cert using the subcas from the REST interface. This will allow us to actually test that the subCA is working.

Third, if we do stick with the current REST API, I think we should consider using something like:

POST /subcas/ca1/ca2 {"id": "ca3"} to generate a subca /ca1/ca2/ca3

Fourth, though, we need to consider carefully whether or not it makes sense to identify a subca through 1) an identifier that contains slashes 2) an identifier that is defined by the user instead of the server. We have currently no controls as to what names are chosen, and no clear rationale for what should be chosen.

Much as I dislike uuids, there is a good reasons openstack uses them everywhere.

We can discuss over IRC, but I can see many issues here.

Ade

On Tue, 2015-08-25 at 14:40 -0400, Ade Lee wrote:

...

Some initial comments and questions. Will have more after playing with it and testing. Looks pretty good so far.

1. what is org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true and why is it needed?

2. In CAService.java: issueX509Cert() - if ca is null, this throws an ECAException - which, if I understand the code correctly, ends up being some kind of BaseException. So, if someone requests a cert and provides a bogus caref, it will be reported as a server error instead of a installation error?

3. CertificateAuthority.java -- subCAHier --> rename to subCAHierarchy.

-- In the constructor, it would be useful to have example inputs in the method documentation so that we can see what data is expected to be in each of the fields. Its hard to review/understand/maintain otherwise.

-- you have extracted initCetDatabase() and initCRLDB(), also extract the replica repo initialization code into initReplicaRepo() or similar.

-- loadSubCAs - could use some description in method comments about the structure we expect to construct here. An example config would help understanding.

-- createSubCA -- returns ECAException() in many different cases ie. if the CA exists, if the parent does not exist, if there is an error in CA creation. This makes it difficult to separate out the things that are likely client request issues vs. server issues. More likely, we want different exceptions that can be caught by the caller and handled appropriately. -- what about uniqueness checks for the issuer DN?

4. SubCAResource.java -- path should be subcas ? not subca .. -- acls needed of course

5. CAEnrollProfile.java --> could not reach CA -- that ends up being a ProfileException -- which maps to a server error? Is this a case where we need a bad request exception instead?

6. subca.schema -- why is it specified in a separate file? (and also in schema.ldif) -- the subca is uniquely defined by only one MAY attribute for nickname (which is in printable string format)? Is that sufficient and should that be a required attribute? Do you need to store the issuerDN for uniqueness checks?

7. SubCAService.java -- lets remove the commented out audit messages and functions -- createSubCA() -- should be some checks here on the data -- rather than passing through to the lower level. Bad data should return BadRequestException, including for cases where we have existing issuerDN or caRef.

8. SubCACreateCLI -- this code would be confusing:

if (cmdArgs.length < positionalArgNames.length) { System.err.println("Error: No " + positionalArgNames[cmdArgs.length] + " specified.");

Just specify "Insufficient params .."

9. It would be good to have a DN check here on client side -- this can be added in a separate patch.

10. Should users be able to define the nickname? I would say "yes" because it might make it easier to notify services like custodia for instance to distribute keys.

11. Should we also have the option to define the token in which the key is generated and stored? ( I think yes - in case for instance, your HSM has limited keys). Where do the subCA keys get generated by default -- in the software or hardware token?

12. To help clarify this, please describe what would be created if one were to add a new subCA using the CLI at reference caRef

Specifically, a) what database entry is created? b) what is the nickname for the key/cert? c) Where is the repo (ie. the suffix) for this ca's certs? d) Exactly which resources belong to the subCA only?

8. SubCACreateCLI -- this code would be confusing:

if (cmdArgs.length < positionalArgNames.length) { System.err.println("Error: No " + positionalArgNames[cmdArgs.length] + " specified.");

Just specify "Insufficient params .."

9. It would be good to have a DN check here on client side -- this can be added in a separate patch.

10. Should users be able to define the nickname? I would say "yes" because it might make it easier to notify services like custodia for instance to distribute keys.

11. Should we also have the option to define the token in which the key is generated and stored? ( I think yes - in case for instance, your HSM has limited keys). Where do the subCA keys get generated by default -- in the software or hardware token?

12. To help clarify this, please describe what would be created if one were to add a new subCA using the CLI at reference caRef

Specifically, a) what database entry is created? b) what is the nickname for the key/cert? c) Where is the repo (ie. the suffix) for this ca's certs? d) Exactly which resources belong to the subCA only?

MISSING ITEMS: These can be in a separate patch, but if so, we need tickets to track them:

1. acls 2. auditing 3. key parameters 4. migration scripts (i.e. how to add db entries) 5. tests - we need unit/functional tests to show the data that is expected and to do basic error checking in the client. These are very important. Eventually, we would like tests to be a required part of any check-in.

Ade

On Mon, 2015-08-24 at 17:27 +1000, Fraser Tweedale wrote:

...

Hi team,

The latest sub-CAs patches are attached. It has been a while since the last patchset (that was posted here, anyway) and there have been some significant changes, outlined below. (The patchset version skipped a couple numbers due to versions distributed privately that I felt were not stable enough to warrant posting to pki -devel.)

Major changes:

- The Java client and CLI were extracted to a separate patch (0044). - An LDAP entry for each sub-CA is written to database. - Database searched and sub-CAs are initialised at startup - Key nickname is store in / read from LDAP entry - Sub-CA "list" API call, client method and CLI was added - More resources are shared between top-level CA and sub-CAs - Suprious task threads and LDAP connections hunted down :)

Dependencies:

- Patch 0026-5 probably depends on 0045[1] for a clean merge. - Patch 0044-3 depends on my patch 0046[2].

[1] https://www.redhat.com/archives/pki-devel/2015-August/msg0007 2.ht ml [2] https://www.redhat.com/archives/pki-devel/2015-August/msg0007 3.ht ml

Cheers, Fraser

_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel

_______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel

On Thu, Aug 27, 2015 at 10:54:14AM -0400, Ade Lee wrote:

One other thing to consider.

Endi suggested that it might be nice to have the UUID generated by the database. One way to do this would be make use of the 389's DNA plugin. As we do not expect too many of these subCAs to be created, this might work well.

Too bad it appears 389ds does not support the 'entryUUID' attribute defined in https://tools.ietf.org/html/rfc4530. Seems that it would be a good fit.

Using DNA would ensure that all replicas would generate different IDs, and would prevent us having to make a database query to confirm that an ID has no longer been used.

If we generate version 4 (pseudorandom) UUID, is the likelihood that different replicas could generate the same UUID even worth worrying about?

On the other hand, this might be harder to set up than simply using java.util.UUID, and may require configuration changes on the DS.

For the first iteration, to unblock me so that I can start writing clients and testing, we can use something simple. In a following iteration, we can look into DNA.

In fact, you can hide these details in a UUID Generator interface which we can switch out later.

I agree on using java.util.UUID for first cut; interface makes good sense.

Ade

On Thu, 2015-08-27 at 10:22 -0400, Ade Lee wrote:

...

On Thu, 2015-08-27 at 17:22 +1000, Fraser Tweedale wrote:

...

On Wed, Aug 26, 2015 at 11:09:24AM -0400, Ade Lee wrote:

...

Some more comments to clarify my concerns on the use of hierarchical urls as the identifier for subcas.

This is the "usual" way (ie. for example OpenStack) REST interfaces are defined:

POST /subcas -- creates a subca and returns the reference /subcas/{uuid}

POST /subcas/ -- POST includes "parent_ref"=http://../subcas/{uuid}" ; returns a subca that has the relevant parent

Full URL reference mandatory here? Preferable? Much-of-a -muchness? Would a plain UUID suffice?

...

If URL is the way to go here, is it necessary to ensure that the host:port refers to "this" server, or can we just ignore the front matter and go straight to looking up the UUID?

I used full URL reference here because of my experience with OpenStack, where systems can be federated across multiple domains. Using URLs for all references is de rigueur there. I do not expect us to be federated in the same way, and we tend to use plain ids everywhere in the API now. Lets use a plain ID.

...

...

GET /subcas -- list all subcas

GET /subcas/{uuid} - get details on subca.

GET /subcas/{uuid}/subcas -- list subcas for subca {uuid}

An alternate formulation could also be:

POST /subcas/{uuid}/subcas -- creates subca with subca {uuid} as parent.

I don't like this as much because its a little less standard. The resulting resource would be /subcas/{uuid2} rather than /subcas/{uuid}/subcas/{uuid2}

Lets not use the above alternative formulation. It is as suggested non -standard and therefore less intuitive.

...

...

Now, I hate uuids. I mean whats not to love about "2a549393-0710 -444b -8aa5-84cf0f85ea79" ? It just rolls off the tongue.

Everyone hates uuids but thats actually an advantage, rather than a weakness in that no one possibly would want to change the URL.

Great wisdom!

...

Handles on the other hand - are subject to all kinds of bike shedding. Do I use something like "ca1", "ca2"? Thats no better than a UUID in terms of semantic content. How about org references? "foo_it_dept", "bar_domain"? Uh oh, what if I decide later I didn't like my handle? What if there is an org rename? Or a reorg? Remember, certs can potentially live a long time.

And what if I'm really perverse and use the handle "subcas" -- aargh! now I've broken one of the possible paths above. Or I might end up breaking a path we'll add in future.

No - the only way to keep this straight is to have the resource identifier be something unique that no one will want to change -- like /subcas/{uuid} - and have some kind of alias system for that subca that can be changed at will.

That mapping could be kept in IPA or Barbican database for instance, rather that in dogtag itself.

Yep. Unless you need it for feel strongly otherwise, we can avoid implementing the alias indirection in initial feature. Certainly it is not needed for IPA use case.

Agreed. Aliases can come later (if ever).

...

...

We can also do what we did for secrets and allow the user to define a "client nickname" as part of the subca record. This is a field that can later be searched for or changed.

As for hierachical-ness, flatter is easier to understand and maintain.

Fair enough. I will implement the scheme proposed above. Thank you for your input! Hope to have a new patchset tomorrow or early next week.

Excellent. Also, while we want to keep the hierarchy flat, we do want to keep track of which CA/subCA is the parent CA in the CA record. This should probably be the plain ID of the parent CA. On the other hand, we want to return this to the client as a fully formed URL. Its the HATEOAS thing to do.

Looking forward to the patches.

Ade

...

...

Ade

On Wed, 2015-08-26 at 02:09 -0400, Ade Lee wrote:

...

Some more thoughts.

First off, there are things that should be included with the CAData for each subsystem.

1. CA signing cert (or more likely a reference to the ca cert) 2. pkcs7 chain (again this is something that could/should likely be a reference) 3. status - ie. enabled/disabled. 4. Description -- user defined description.

For the motivation for (30 status, we do need to consider what we want do when someone wants to delete or decommission a subCA. If we want to keep the subCA record -- and we almost certainly need to -- we should probably add some kind of field to enable/disable.

Second, we definitely need code to be able to issue a cert using the subcas from the REST interface. This will allow us to actually test that the subCA is working.

Third, if we do stick with the current REST API, I think we should consider using something like:

POST /subcas/ca1/ca2 {"id": "ca3"} to generate a subca /ca1/ca2/ca3

Fourth, though, we need to consider carefully whether or not it makes sense to identify a subca through 1) an identifier that contains slashes 2) an identifier that is defined by the user instead of the server. We have currently no controls as to what names are chosen, and no clear rationale for what should be chosen.

Much as I dislike uuids, there is a good reasons openstack uses them everywhere.

We can discuss over IRC, but I can see many issues here.

Ade

On Tue, 2015-08-25 at 14:40 -0400, Ade Lee wrote:

...

Some initial comments and questions. Will have more after playing with it and testing. Looks pretty good so far.

1. what is org.apache.tomcat.util.buf.UDecoder.ALLOW_ENCODED_SLASH=true and why is it needed?

2. In CAService.java: issueX509Cert() - if ca is null, this throws an ECAException - which, if I understand the code correctly, ends up being some kind of BaseException. So, if someone requests a cert and provides a bogus caref, it will be reported as a server error instead of a installation error?

3. CertificateAuthority.java -- subCAHier --> rename to subCAHierarchy.

-- In the constructor, it would be useful to have example inputs in the method documentation so that we can see what data is expected to be in each of the fields. Its hard to review/understand/maintain otherwise.

-- you have extracted initCetDatabase() and initCRLDB(), also extract the replica repo initialization code into initReplicaRepo() or similar.

-- loadSubCAs - could use some description in method comments about the structure we expect to construct here. An example config would help understanding.

-- createSubCA -- returns ECAException() in many different cases ie. if the CA exists, if the parent does not exist, if there is an error in CA creation. This makes it difficult to separate out the things that are likely client request issues vs. server issues. More likely, we want different exceptions that can be caught by the caller and handled appropriately. -- what about uniqueness checks for the issuer DN?

4. SubCAResource.java -- path should be subcas ? not subca .. -- acls needed of course

5. CAEnrollProfile.java --> could not reach CA -- that ends up being a ProfileException