Issue Details (XML | Word | Printable)

Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Manfred Riem
Reporter: Stephane Poirier
Votes: 1
Watchers: 1

If you were logged in you would be able to see more operations.

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

Created: 18/Oct/12 07:05 PM   Updated: 08/Jan/14 10:19 PM   Resolved: 20/Nov/13 08:26 PM
Component/s: state saving
Affects Version/s: 2.0, 2.1, 2.1.20
Fix Version/s: 2.2.5

Time Tracking:
Original Estimate: 1 day
Original Estimate - 1 day
Remaining Estimate: 1 day
Remaining Estimate - 1 day
Time Spent: Not Specified
Time Spent - Not Specified

File Attachments: 1. Text File (2 kB) 18/Oct/12 07:05 PM - Stephane Poirier
2. Java Source File (9 kB) 18/Oct/12 07:05 PM - Stephane Poirier
3. Text File changebundle.txt (3 kB) 19/Nov/13 10:02 PM - Manfred Riem

Issue Links:

Participants: Ed Burns, Manfred Riem, rogerk and Stephane Poirier

 Description  « Hide


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 (
-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.

Manfred Riem added a comment - 24/Oct/12 08:18 PM

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.

Stephane Poirier added a comment - 24/Oct/12 09:03 PM - edited

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.

rogerk added a comment - 11/Jan/13 03:46 PM

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).

Manfred Riem added a comment - 19/Nov/13 09:36 PM

Only the thread safeness will be addressed by this issue. For the cluster-awareness a separate enhancement issue has been filed.

Ed Burns added a comment - 20/Nov/13 05:09 PM

r=edburns. Looks good.

Manfred Riem added a comment - 20/Nov/13 08:26 PM

Applied to 2.2 branch,

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