[prev in list] [next in list] [prev in thread] [next in thread] 

List:       myfaces-dev
Subject:    [jira] [Comment Edited] (TRINIDAD-2567) Trinidad secret generation is not thread-safe
From:       "Kyle Stiemann (Jira)" <dev () myfaces ! apache ! org>
Date:       2021-02-11 20:41:01
Message-ID: JIRA.13358057.1613014039000.604563.1613076061089 () Atlassian ! JIRA
[Download RAW message or body]


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

Kyle Stiemann edited comment on TRINIDAD-2567 at 2/11/21, 8:40 PM:
-------------------------------------------------------------------

Thanks [~tandraschko], I'm not super worried about this getting fixed as the \
workaround was trivial for me. I really just wanted to document the issue so that if \
I ever run into it again (or if someone else does), the problem (and workaround) is \
googleable.


was (Author: stiemannkj1@gmail.com):
Thanks [~tandraschko], I'm not super worried about this getting fixed as the \
workaround was trivial for me. I really just wanted to document the issue so that if \
I ever run into it again (or if someone else does), the problem (and workaround) is \
in google.

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


[prev in list] [next in list] [prev in thread] [next in thread]