Re: [Pki-devel] 0001-Provide-CA-EE-Restful-interface-and-test-client.patch
Great stuff, thanks.
I will look into the ones you had questions about.
----- Original Message -----
From: "Endi Sukma Dewata" <edewata(a)redhat.com>
To: pki-devel(a)redhat.com, "John Magne" <jmagne(a)redhat.com>
Sent: Tuesday, May 1, 2012 6:05:28 PM
Subject: Re: [Pki-devel] 0001-Provide-CA-EE-Restful-interface-and-test-client.patch
On 4/30/2012 1:53 PM, John Magne wrote:
Provide CA EE Restful interface and test client.
Tickets #144 and #145
Providing the following:
1. Simple EE restful interface for certificates, printing, listing and searching.
2. Simple EE restful interface for certificate enrollment requests.
3. Simple EE restful interface for profiles and profile properties.
4. Simple Test client to exercise the functionality.
5. Created restful client base class inherited by CARestClient and DRMRestClient.
6. Provide simple restful implementations of new interfaces added.
ToDO: Need some more refactoring to base classes for some of the new classes which
are similar to classes
in the DRM restful area.
ToDO: Actual certificate enrollment code that will be refactored from existing
ProfileSubmitServlet.
Some feedback:
1. The CMSRestClient could be moved into
base/common/src/com/netscape/cms/servlet/csadmin, which can be used by
any clients including the test tools.
2. The CAErrorInterceptor is identical to DRMErrorInterceptor. We can
merge these classes into CMSErrorInterceptor and place it in the same
package as CMSRestClient. The interceptor can be added in the
CMSRestClient, so the subclasses don't need to do that.
3. In CertsResourceService.createSearchFilter() the 'matches' cannot be
more than 1, so the code could be simplified.
4. In CertDAO.getCertChainData() the certsInChain array is allocated
multiple times. It should be possible to use the loop to determine the
length first, then allocate the array just once after that.
5. Also in CertDAO.getCertChainData() after the initialization loop it
looks like the certsInChain may contain a null value if x509cert exists
in mCACerts but not the last element. Is that case possible?
6. The CertSearchData.escapeValueRfc1779() probably can be moved into
LDAPUtil.escapeDN(). We can rename the existing LDAPUtil.escape() into
LDAPUtil.escapeFilter().
7. In CertSearchData.build*Filter() it's not necessary to use "this."
when calling other methods of the same object. It's possible to use the
fields directly too.
8. In CertSearchData.build*Filter() the values should be escaped to
avoid injection attack:
filter.append("(certRecordId>=" + LDAPUtil.escapeFilter(serialFrom) +
")");
9. In CertSearchData.buildAVAFilter() the code does a double escape with
the escape DN method. I think it should use 2 different escape methods,
one for the DN and the other for the filter. So the code would look like
this:
lf.append(LDAPUtil.escapeFilter(LDAPUtil.escapeDN(param)));
Could you try creating some certs with this common name "E*Corp, Inc."
and "E-Corp, Inc." and see if it can find an exact match. The
escapeFilter() should escape the '*'.
10. There are some formatting issues and trailing whitespaces.
Auto-formatting probably will fix this.
--
Endi S. Dewata