Community
Participate
Working Groups
The correct chained certificates are rejected in the Client. In 264858, additional check will be done in DataStoreTrustManager. This additional check is OK for self-signed certificate but the check rejects the valid chained certificates erroneously. And I think that this additional check is not needed to support the request claimed by 264858.
Created attachment 170733 [details] Patch for verification against the chained certificates in the client
Created attachment 170839 [details] patch updated with copyright
Dave, you need to iplog+ the ORIGINAL contribution, and not yours.
I must admit that I do not understand the change. I also do not understand how that would relate to bug 264858 (or did you mean to reference something different?) Noriaki or Dave, can you explain the change please.
Comment on attachment 170839 [details] patch updated with copyright The other things (beside an explanation of what's the code supposed to do) I require before giving a PMC +1 on this are: 1. Justification why this is important 2. A test case description. What behavior do end users see with the bug, and how does the fix resolve that. Ideally, some steps that an arbitrary person in the Community can perform to verify that the fix has been made properly. I hope you do not perceive this as overly bureaucratic... I'm trying to understand the impact, the risk and how to mitigate the risk by testing at this point.
(In reply to comment #4) > I must admit that I do not understand the change. I also do not understand how > that would relate to bug 264858 (or did you mean to reference something > different?) > > Noriaki or Dave, can you explain the change please. As a background, here is the Javadoc describing what the checkTrusted method is supposed to do: http://java.sun.com/j2se/1.4.2/docs/api/javax/net/ssl/X509TrustManager.html "Given the partial or complete certificate chain provided by the peer, build a certificate path to a trusted root and return if it can be validated and is trusted for client/server SSL authentication based on the authentication type." In bug 264858, the following was added before the call to verify but it is unneeded and restrictive: if (cert.getSubjectDN().equals(tcert.getIssuerDN())) {
(In reply to comment #5) > (From update of attachment 170839 [details]) > The other things (beside an explanation of what's the code supposed to do) I > require before giving a PMC +1 on this are: > > 1. Justification why this is important > > 2. A test case description. What behavior do end users see with the bug, and > how does the fix resolve that. Ideally, some steps that an arbitrary person in > the Community can perform to verify that the fix has been made properly. > > I hope you do not perceive this as overly bureaucratic... I'm trying to > understand the impact, the risk and how to mitigate the risk by testing at this > point. I'll let Noriaki answer the justification and describe the conditions of the customer problem.
This is a problem when the client logs on the server by a pair of userid and password under SSL connection. In bug 264858, the following check was added: if (cert.getSubjectDN().equals(tcert.getIssuerDN())) { This check means that the server certificate must be a self-signed certificate. But almost all users use server certificate signed by internal Certificate Authority or public Certificate Authority (e.g. Verisign). The following are the traces in this point under my test environments: cert.getSubjectDN()=CN=Yamato Server, OU=YSL, O=IBMJ, C=JP tcert.getIssuerDN()=CN=IBM JAPAN CA, OU=Local Certificate Authority, O=IBMJ, C=JP That is, My server certificate ("Yamato server") is signed by our local CA, but this correct server certificate is rejected by the above additional check before verifying the certificate. You can recreate this problem by using a server certificate signed by another CA. And I haven't seen any IBM customers that use self-signed server certificate.
+1 based on the technical description, and the fact that this is clearly a regression introduced by the fix for bug 264858. The explanation is logical, the fix marginal, and just restores the code to a state as it was before the fix for bug 264858. The explanation around self-signed certificate helped me understand the impact of removing the if() clause. I have committed and released the change for the I20100603-0700 I-build since the fix is small and understandable, and getting an actual build verified is of larger value here than waiting for Kevin to give his +1 (Kevin, please do so after-the-fact after reviewing the patch). Noriaki/Dave, please verify that the fix does what you want with I20100603-0700. Thanks for the Fix!
Yes, I confirmed that the fix works. Legal Message: I, {Noriaki Takatsu}, declare that I developed the attached code from scratch, without referencing any 3rd party materials except material licensed under the EPL. I am authorized by my employer to make this contribution under the EPL.
