eclipse-ee4j / mojarra

Mojarra, a Jakarta Faces implementation
Other
162 stars 110 forks source link

ViewState encryption: ByteArrayGuard is not thread safe / Does Not Take Into Account Session Failover #2557

Closed ren-zhijun-oracle closed 10 years ago

ren-zhijun-oracle commented 12 years ago

[Symptoms]

ByteArrayGuard class randomly outputs "ERROR: MAC did not verify!" messages to the error console. This only occurs when client side viewstate encryption is enabled and that multiple clients are running against the server.

[Identified problem]

ByteArrayGuard's instances of javax.crypto.Mac are used in a way that is not thread-safe.

ByteArrayGuard creates 2 instances of the Mac object and recycles them through the whole application lifecycle. During encryption and decryption, Macs get updated and calculated. If multiple threads run any of these 2 operations simultaneously on a (shared) Mac instance, the calculated Mac can be corrupted.

[Steps to reproduce]

1)Enable client viewstate encryption: -Enable viewstate encryption (http://docs.oracle.com/cd/E19575-01/819-3669/bnaxo/index.html) -Set state saving method to "client" 2)Run an intensive multi-user load test (e.g. using JMeter/LoadRunner). Each user must perform actions leading to: -Make the client store the viewstate encrypted by the server -Make the server process the encrypted viewstate stored by the client In my case, the error message appeared randomly, for about 5% of requests.

[Proposed solution]

Mac instanciation and initialization can be moved to the encrypt/decrypt procedure. Various threads will therefore not share Mac instances. I implemented this solution and tested it. It does not lead to notable performance drawbacks. See attachments.

Affected Versions

[2.0, 2.1, 2.1.20]

ren-zhijun-oracle commented 6 years ago
ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented Reported by apolypse

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented @manfredriem said: If the encryption key would be stored in the user session then it could be used to encrypt/decrypt any request coming from the client. Note if the session does not contain a key yet it should generate one.

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented apolypse said: Despite the similarities between the current issue and the cluster issue, those are completely different to me. The current issue is invariant to the scope in which the encryption key is stored. Neither is it a cluster/multi-server issue. It stricly is a multi-thread issue.

The scope of the encryption key might be an issue to be discussed, but I don't think it is relevant to this thread. If this non-relatedness is unclear, I'll be glad to detail my thoughts.

ren-zhijun-oracle commented 11 years ago

@javaserverfaces Commented rogerk said: We do acknowledge the potential thread issue - however, the fail over scenario is also valid. We talked about storing the secret key in the session (if it does not already exist there) (see ByteArrayGuard setupKeyAndMac method that generates the secret key).

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: Only the thread safeness will be addressed by this issue. For the cluster-awareness a separate enhancement issue has been filed.

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @edburns said: r=edburns. Looks good.

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented @manfredriem said: Applied to 2.2 branch,

svn commit -m "Fixes https://java.net/jira/browse/JAVASERVERFACES-2553, r=edburns, do not use ivars for Mac instances." Sending jsf-ri/src/main/java/com/sun/faces/renderkit/ByteArrayGuard.java Transmitting file data . Committed revision 12678.

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented File: ByteArrayGuard.java.patch Attached By: apolypse

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented File: ByteArrayGuard_fixed.java Attached By: apolypse

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented File: changebundle.txt Attached By: @manfredriem

ren-zhijun-oracle commented 12 years ago

@javaserverfaces Commented Issue-Links: is related to JAVASERVERFACES-1013 JAVASERVERFACES-3083

ren-zhijun-oracle commented 7 years ago

@javaserverfaces Commented This issue was imported from java.net JIRA JAVASERVERFACES-2553

ren-zhijun-oracle commented 10 years ago

@javaserverfaces Commented Marked as fixed on Wednesday, November 20th 2013, 12:26:50 pm