[ 
https://issues.apache.org/jira/browse/TRINIDAD-2567?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17283183#comment-17283183
 ] 

Kyle Stiemann commented on TRINIDAD-2567:
-----------------------------------------

This bug is particularly nasty when testing a Trinidad application since tests 
often send multiple requests rapidly or in parallel. Worse yet, if you attempt 
to debug the test or the Trinidad application, you will likely slow down the 
requests enough that the first request will cause the key to be generated and 
cached before any other request is sent/received. So the issue effectively does 
not show up when debugging. Similarly, if you run a single test, you will not 
see the issue as the key will be generated and cached correctly.

> Trinidad secret generation is not thread-safe
> ---------------------------------------------
>
>                 Key: TRINIDAD-2567
>                 URL: https://issues.apache.org/jira/browse/TRINIDAD-2567
>             Project: MyFaces Trinidad
>          Issue Type: Bug
>          Components: Components, Facelets, Infrastructure, Plugins
>    Affects Versions: 2.2.1-core
>            Reporter: Kyle Stiemann
>            Priority: Minor
>
> Sending multiple requests in rapid succession to a Trinidad application that 
> has just started will cause multiple different secret keys to be generated. 
> If multiple {{POST}} s are sent, all but 1 will fail with a 
> {{ViewExpiredException}}. Trinidad generates the secret keys in 
> {{StateUtils}} somewhat like this:
> {code}
> private static SecretKey getSecret(ExternalContext ctx) {
>   SecretKey secretKey = (SecretKey) 
> ctx.getApplicationMap().get(INIT_SECRET_KEY_CACHE);
>   if (secretKey == null) {
>     secretKey = 
> createSecretKey(KeyGenerator.getInstance(getAlgorithm(ctx)).generateKey().getEncoded());
>     ctx.getApplicationMap().put(INIT_SECRET_KEY_CACHE, secretKey);
>   }
>   return secretKey;
> }
> {code}
> {{FormRenderer}} calls {{ViewHandler.writeState()}} which calls the 
> {{StateUtils.getSecret()}} method on each request. If more than 1 request 
> calls {{getSecret()}} before the secret key is set in 
> {{INIT_SECRET_KEY_CACHE}}, each call to {{getSecret()}} has the chance to see 
> a {{null}} value for {{INIT_SECRET_KEY_CACHE}}, generate a new secret key, 
> and replace any existing secret in {{INIT_SECRET_KEY_CACHE}}. Any view state 
> that was generated using the discarded secrets will be unusable and cause a 
> {{ViewExpiredException}}.
> h2. Workarounds
> The simplest workaround is to set values for the secret keys as 
> {{init-param}} s: 
> https://cwiki.apache.org/confluence/display/MYFACES2/Secure+Your+Application. 
> For example, in the {{web.xml}} (*note that the provided values are examples 
> and should not be used in a production application*):
> {code:xml}
> <context-param>
>     <param-name>org.apache.myfaces.SECRET</param-name>
>     <!-- Decodes to TEST_KEY -->
>     <param-value>VEVTVF9LRVk=</param-value>
> </context-param>
> <context-param>
>     <param-name>org.apache.myfaces.MAC_SECRET</param-name>
>     <!-- Decodes to TRINIDAD_TEST_MAC_SECRET -->
>     <param-value>VFJJTklEQURfVEVTVF9NQUNfU0VDUkVU</param-value>
> </context-param>
> {code}
> h2. Potential Fixes
> # Save 1 generated key in the application scope using either 
> {{Map.putIfAbsent()}} or some other kind of synchronization. Use only the key 
> from the application scope to generate the {{SecretKey}} object. Even if 
> secret object caching is disabled, only 1 key would be used to generate the 
> secret object, so the application would still function.
> # Use {{Map.putIfAbsent()}} to ensure only 1 secret is ever cached in the 
> application. If secret caching is disabled, the application would still not 
> function (which is the same as the existing behavior).
> h2. Steps to Reproduce:
> # Create 1 WAR with a simple {{ping.xhtml}} endpoint:
> {code:xml}
> <html
>   xmlns:h="http://java.sun.com/jsf/html";
>   xmlns="http://www.w3.org/1999/xhtml";>
>     <h:head/>
>     <h:body>
>         <code>pong</code>
>     </h:body>
> </html>
> {code}
> # Create another Trinidad WAR with the following view and bean:
> *{{hello.xhtml}}:*
> {code:xml}
> <?xml version="1.0" encoding="UTF-8"?>
> <tr:document
>   xmlns:tr="http://myfaces.apache.org/trinidad";
>   title="hello">
>     <tr:form id="form">
>         <tr:inputText id="name" label="Please enter your name:" 
> required="true" value="#{helloBean.name}" />
>         <tr:commandButton id="submitName" text="Submit" /><br />
>         <tr:outputText value="Hello #{helloBean.name}!" />
>     </tr:form>
> </tr:document>
> {code}
> *{{HelloBean.java}}:*
> {code:java}
> @ManagedBean
> @RequestScoped
> public final class HelloBean {
>   private String name;
>   public String getName() {
>     return name;
>   }
>   public void setName(String name) {
>     this.name = name;
>   }
> }
> {code}
> # Start up an app server like Tomcat with both WARs deployed.
> # Using a script:
> ## {{GET}} the {{ping.xhtml}} endpoint to cause the app server to initialize.
> ## {{GET}} the {{hello.xhtml}} endpoint to obtain the view state and session 
> id.
> ## Use the view state and session id to {{POST}} a name to the 
> {{hello.xhtml}} form.
> ## Repeat the {{GET}} and {{POST}} 5 times in rapid succession with different 
> sessions.
> Here's an example {{bash}} script which uses {{curl}} to execute the above 
> steps: 
> {code:sh}
> #!/bin/bash
> sendPost() {
>   ENCODED_VIEW_STATE="$(curl -s --cookie-jar /tmp/cookie-jar-$1 --cookie 
> /tmp/cookie-jar-$1 \
>     'http://localhost:8080/trinidad-2.2/faces/hello.xhtml' | \
>     tr -d '\n' | sed 
> 's/.*name="javax.faces.ViewState".*value="\([^"][^"]*\)".*/\1/' | \
>     sed -e 's|/|\%2F|g' -e 's/+/%2B/g' -e 's/=/%3D/g')"
>   curl --cookie-jar /tmp/cookie-jar-$1 --cookie /tmp/cookie-jar-$1 \
>     -d 
> "javax.faces.ViewState=$ENCODED_VIEW_STATE&org.apache.myfaces.trinidad.faces.FORM=form&source=submitName&name=Test"
>  \
>     -X POST 'http://localhost:8080/trinidad-2.2/faces/hello.xhtml'
> }
> rm /tmp/cookie-jar*; until curl -s 
> 'http://localhost:8080/other-app/ping.xhtml'; do :; done &&
> for i in {1..5}; do sendPost $i & done
> {code}
> h3. Result:
> If the bug still exists, 4 of the 5 {{POST}} s will fail with a 
> {{ViewExpiredException}}:
> {code:html}
> <!doctype html>
> <html lang="en">
> <head><title>HTTP Status 500 – Internal Server Error</title>
>     <style type="text/css">body {font-family: Tahoma, Arial, sans-serif;} h1, 
> h2, h3, b {color: white; background-color: #525D76; } h1 { font-size: 22px; } 
> h2 { font-size: 16px; } h3 { font-size: 14px; } p { font-size: 12px; } a { 
> color: black; } .line { height: 1px; background-color: #525D76; border: none; 
> }</style>
> </head>
> <body><h1>HTTP Status 500 – Internal Server Error</h1>
> <hr class="line"/>
> <p><b>Type</b> Exception Report</p>
> <p><b>Message</b> viewId:&#47;hello.xhtml - View &#47;hello.xhtml could not 
> be restored.</p>
> <p><b>Description</b> The server encountered an unexpected condition that 
> prevented it from fulfilling the request.</p>
> <p><b>Exception</b></p>
> <pre>javax.servlet.ServletException: viewId:&#47;hello.xhtml - View 
> &#47;hello.xhtml could not be restored.
>         javax.faces.webapp.FacesServlet.service(FacesServlet.java:671)
>         
> org.apache.myfaces.trinidadinternal.webapp.TrinidadFilterImpl._doFilterImpl(TrinidadFilterImpl.java:350)
>         
> org.apache.myfaces.trinidadinternal.webapp.TrinidadFilterImpl.doFilter(TrinidadFilterImpl.java:226)
>         
> org.apache.myfaces.trinidad.webapp.TrinidadFilter.doFilter(TrinidadFilter.java:92)
>         org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
> </pre>
> <p><b>Root Cause</b></p>
> <pre>javax.faces.application.ViewExpiredException: viewId:&#47;hello.xhtml - 
> View &#47;hello.xhtml could not be restored.
>         
> com.sun.faces.lifecycle.RestoreViewPhase.execute(RestoreViewPhase.java:212)
>         com.sun.faces.lifecycle.Phase.doPhase(Phase.java:101)
>         
> com.sun.faces.lifecycle.RestoreViewPhase.doPhase(RestoreViewPhase.java:123)
>         com.sun.faces.lifecycle.LifecycleImpl.execute(LifecycleImpl.java:198)
>         javax.faces.webapp.FacesServlet.service(FacesServlet.java:658)
>         
> org.apache.myfaces.trinidadinternal.webapp.TrinidadFilterImpl._doFilterImpl(TrinidadFilterImpl.java:350)
>         
> org.apache.myfaces.trinidadinternal.webapp.TrinidadFilterImpl.doFilter(TrinidadFilterImpl.java:226)
>         
> org.apache.myfaces.trinidad.webapp.TrinidadFilter.doFilter(TrinidadFilter.java:92)
>         org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
> </pre>
> <p><b>Note</b> The full stack trace of the root cause is available in the 
> server logs.</p>
> <hr class="line"/>
> <h3>Apache Tomcat/9.0.39</h3></body>
> </html>
> {code}
> If the bug is fixed, all 5 requests will succeed and show "Hello Test!" due 
> to a successful {{form}} {{POST}}.
> h2. Related
> If the developer disables secret caching without specifying a key, all 
> {{POST}} requests will fail with similar results.
> {code:xml}
> <context-param>
>     <param-name>org.apache.myfaces.SECRET.CACHE</param-name>
>     <param-value>false</param-value>
> </context-param>
> <context-param>
>     <param-name>org.apache.myfaces.MAC_SECRET.CACHE</param-name>
>     <param-value>false</param-value>
> </context-param>
> {code}
> I'm unsure whether the secret cache was ever intended to be disabled when 
> there is no key set, but it may be worth fixing that issue alongside this one.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to