Re: [Pki-devel] [PATCH] DRM Transport Key Rotation

On 09/27/2013 01:45 PM, Andrew Wnuk wrote: > On 09/27/2013 10:11 AM, Ade Lee wrote: >> Just a few comments/ questions so I can understand the patch better. >> >> 1. In CAEnrollProfile, you update the request queue only if the >> transport cert is invalid. Why do we need to do this? Or why do we not >> need to do this in all cases here? > DRM rejects archival request when invalid transport keys are used by CA. > CA marks corresponding enrollment request as rejected providing clear > error message at the end of approval process. >> >> 2. In EnrollProfile.java, you get the transport cert from >> ca.connector.KRA.transportCert. Is it possible to have more than one CA >> connected? Is that parameter always the correct one to use? > Multiple CAs can archive keys using single DRM, but one CA cannot > archive keys using multiple DRMs. >> >> 3. In EnrollmentService.java, you read the transport cert attribute in >> the request, and throw an exception of it is not present (basically >> tcert == null). This will presumably occur if you receive an escrow >> request from an older CA, right? How are we handling this case? > Transport certificate verification returns null only for invalid > transport certificate. > DRM will use current transport key In case of CA not providing > transport certificate through the connector. > I just noticed that attached patch is not the final one. I'll resend > it shortly including (4). >> 4. Incidentally, >> transportCert != null && transportCert.length() > 0 >> can be replaced with ! StringUtils.isEmpty(transportCert) >> Same thing in a couple other places. I see 3 occurrences in 2 files. None of the two file has dependency on apache packages at this moment. Use of StringUtils requires dependency on apache package, so I see no reason to add unnecessary dependency on the foreign package just to save 16 characters

> I did not see any documentation specifying such requirement. >> >> 5. Why do you return true in KRAService.java (serviceRequest) instead of >> false? > This way CA has a chance to return nice message reporting use of > invalid transport certificate to agent approving enrollment request. >> >> Ade >> >> >> On Wed, 2013-09-25 at 16:59 -0700, Andrew Wnuk wrote: >>> This patch provides basic support for DRM transport key rotation >>> described >>> in http://pki.fedoraproject.org/wiki/DRM_Transport_Key_Rotation >>> >>> This patch provides implementation for tickets: >>> - 729 - CA to include transport certificate when submitting >>> archival request to DRM >>> - 730 - DRM to detect presence of transport certificate >>> attribute >>> in submitted archival >>> request and validate transport certificate against DRM's >>> transport key list >>> - 731 - DRM to provide handling for alternative transport key >>> based on detected >>> and validated transport certificate arriving as a >>> part of >>> extended archival request >>> >>> >>> >>> _______________________________________________ >>> Pki-devel mailing list >>> Pki-devel(a)redhat.com >>> https://www.redhat.com/mailman/listinfo/pki-devel >> > > _______________________________________________ > Pki-devel mailing list > Pki-devel(a)redhat.com > https://www.redhat.com/mailman/listinfo/pki-devel _______________________________________________ Pki-devel mailing list Pki-devel(a)redhat.com https://www.redhat.com/mailman/listinfo/pki-devel