You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@santuario.apache.org by bu...@apache.org on 2008/05/13 20:25:49 UTC
DO NOT REPLY [Bug 44991] New: Concurrent invocation of KeyInfo.
getX509Certificate() occasionally fails
https://issues.apache.org/bugzilla/show_bug.cgi?id=44991
Summary: Concurrent invocation of KeyInfo.getX509Certificate()
occasionally fails
Product: Security
Version: unspecified
Platform: PC
OS/Version: All
Status: NEW
Severity: critical
Priority: P2
Component: Signature
AssignedTo: security-dev@xml.apache.org
ReportedBy: giedrius.noreikis@gmail.com
When executed concurrently in several threads,
org.apache.xml.security.keys.KeyInfo.getX509Certificate() occasionally returns
null.
The log entries made from the failing thread are:
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.KeyInfo
getX509CertificateFromInternalResolvers
Start getX509CertificateFromInternalResolvers() with 0 resolvers
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.KeyInfo
getX509Certificate
I couldn't find a X509Certificate using the per-KeyInfo key resolvers
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.KeyInfo
getX509CertificateFromStaticResolvers
Start getX509CertificateFromStaticResolvers() with 7 resolvers
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.keyresolver.implementations.X509SKIResolver
engineLookupResolveX509Certificate
Can I resolve X509Data?
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.keyresolver.implementations.X509SKIResolver
engineLookupResolveX509Certificate
I can't
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.keyresolver.implementations.X509SubjectNameResolver
engineLookupResolveX509Certificate
Can I resolve X509Data?
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.keyresolver.implementations.X509SubjectNameResolver
engineLookupResolveX509Certificate
I can't
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.keyresolver.implementations.X509IssuerSerialResolver
engineLookupResolveX509Certificate
Can I resolve X509Data?
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.utils.ElementProxy
<init>
setElement("X509Data", "http://www.w3.org/2000/09/xmldsig#")
--------------------------------------------------
2008-05-13T20:21:50
org.apache.xml.security.keys.KeyInfo
getX509Certificate
I couldn't find a X509Certificate using the system-wide key resolvers
--------------------------------------------------
Possible cause:
KeyInfo.getX509CertificateFromStaticResolvers() operates on
org.apache.xml.security.keys.keyresolver.KeyResolver class: it iterates through
all KeyResolver items, trying to applyCurrentResolver(), and, in case of
success, calls KeyResolver.hit().
When getX509CertificateFromStaticResolvers() in Thread-1 founds a "good"
resolver at iteration, say, i=5, and calls hit(), that resolver is moved at the
beginning of the static KeyResolver._resolverVector list. If Thread-2 at the
same time executes getX509CertificateFromStaticResolvers() at iteration, say,
i=3, it will never see that resolver.
Possible fix:
With the present design, it seems, KeyResolver can not support item() and hit()
methods together, since hit() changes the order of the _resolverVector items.
Either hit() should be removed or a copy of _resolverVector should be made
before accessing it's elements.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44991] Concurrent invocation of
KeyInfo.getX509Certificate() occasionally fails
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44991
--- Comment #5 from Giedrius Noreikis <gi...@gmail.com> 2009-06-27 07:47:20 PST ---
Hi Colm,
I proposed this fix because I had to fix the problem in our production
immediately and I couldn't wait for the Raul's solution to come.
It was more than a year ago. I've tried to recover the history of fixing the
bug:
1. Initially this bug was "fixed" in rev.657592
(http://svn.apache.org/viewvc?view=rev&revision=657592) at 2008-05-18 17:15:25
UTC.
2. At 2008-05-18 20:16:27 UTC Raul posted the comment above, promising another
solution with iterators.
3. I implemented my own fix before 2008-05-18 21:15:15 UTC.
4. The promised fix from Raul followed at 2008-05-26 17:43:10 UTC
(http://svn.apache.org/viewvc?view=rev&revision=660248).
So I guess the bug should fixed by now, though I've never tested that solution.
Anyway, since ResolverIterator is already implemented in KeyResolver, it hardly
makes sense to apply the old patch now. It would be nice to run some
concurrency tests against the current version, but I will have some time for
this only after a month. For the time being, I'd suggest to mark this as fixed.
Giedrius
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44991] Concurrent invocation of
KeyInfo.getX509Certificate() occasionally fails
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44991
coheigea <co...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #6 from coheigea <co...@apache.org> 2009-06-29 03:22:58 PST ---
Marking as fixed as per Giedrius' suggestion.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44991] Concurrent invocation of
KeyInfo.getX509Certificate() occasionally fails
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44991
--- Comment #4 from coheigea <co...@apache.org> 2009-06-24 10:14:49 PST ---
Hi Giedrius,
Could you resubmit your fix as a patch (and tick the patch box)?
I think I'd prefer to keep the Iterator implementation in KeyResolver, for API
backwards compatibility reasons, but this can be easily done.
Colm.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44991] Concurrent invocation of KeyInfo.
getX509Certificate() occasionally fails
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44991
--- Comment #2 from Raul Benito <ra...@r-bg.com> 2008-05-18 12:16:27 PST ---
Sorry the patch only change half of the problem. I'm working in a solution but
it should be in the like of returning a list for iteration.
I will create a single thread unit testing.
Thanks again
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44991] Concurrent invocation of KeyInfo.
getX509Certificate() occasionally fails
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44991
--- Comment #3 from Giedrius Noreikis <gi...@gmail.com> 2008-05-18 13:15:15 PST ---
Created an attachment (id=21978)
--> (https://issues.apache.org/bugzilla/attachment.cgi?id=21978)
KeyResolverVector
Yes, copying just a *reference* to the *same* object changes nothing - returned
list must be a copy of the *object*, which will not be affected by the
subsequent updates.
Reproducing this bug is not easy (nevertheless, it appears in our production
from time to time). I create 100 threads, each sleeps random time before
proceeding, and repeat the procedure 100 times. Almost always it fails at some
iteration.
In case you are interested in my solution, I attach the files (I couldn't wait
any longer, 'cause I had to fix this bug in our system). I decided to introduce
a new class - KeyResolverVector - to encapsulate the shared resource and the
synchronized methods for accessing it. Of course, KeyResolver and KeyInfo had
to be adjusted accordingly.
Regards,
Giedrius
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
DO NOT REPLY [Bug 44991] Concurrent invocation of KeyInfo.
getX509Certificate() occasionally fails
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=44991
--- Comment #1 from Raul Benito <ra...@r-bg.com> 2008-05-18 10:12:45 PST ---
Thanks for the report. I don't found it, but you are right. I forgot to use the
pointer of the old list to make the RCU working.
With this change it should work and shouldn't be more concurrency problems.
This change is already commited to CVS head.
Thanks again
Index: src/org/apache/xml/security/keys/keyresolver/KeyResolver.java
===================================================================
--- src/org/apache/xml/security/keys/keyresolver/KeyResolver.java
(revision 657575)
+++ src/org/apache/xml/security/keys/keyresolver/KeyResolver.java
(working copy)
@@ -123,9 +123,11 @@
Element element, String BaseURI, StorageResolver storage)
throws KeyResolverException {
- for (int i = 0; i < KeyResolver._resolverVector.size(); i++) {
+ // use the old vector to not be hit by updates
+ List resolverVector = KeyResolver._resolverVector;
+ for (int i = 0; i < resolverVector.size(); i++) {
KeyResolver resolver=
- (KeyResolver) KeyResolver._resolverVector.get(i);
+ (KeyResolver) resolverVector.get(i);
if (resolver==null) {
Object exArgs[] = {
@@ -165,10 +167,11 @@
public static final PublicKey getPublicKey(
Element element, String BaseURI, StorageResolver storage)
throws KeyResolverException {
-
- for (int i = 0; i < KeyResolver._resolverVector.size(); i++) {
+
+ List resolverVector = KeyResolver._resolverVector;
+ for (int i = 0; i < resolverVector.size(); i++) {
KeyResolver resolver=
- (KeyResolver) KeyResolver._resolverVector.get(i);
+ (KeyResolver) resolverVector.get(i);
if (resolver==null) {
Object exArgs[] = {
@@ -186,7 +189,7 @@
if (cert!=null) {
if (i!=0) {
//update resolver.
- List
resolverVector=(List)((ArrayList)_resolverVector).clone();
+
resolverVector=(List)((ArrayList)_resolverVector).clone();
Object ob=resolverVector.remove(i);
resolverVector.add(0,ob);
_resolverVector=resolverVector;
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.