[jira] [Comment Edited] (ROL-2058) No salt renewal on POST request

["Greg Huber (JIRA)" <ji...@apache.org>]

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

Greg Huber edited comment on ROL-2058 at 3/20/15 1:54 PM:
----------------------------------------------------------

I would say you need the no-opp setSalt(..) for completeness (messages are there for a reason).   The timeout is personal I guess. 

I never really like the way the salt csrf works here for struts UI as its leads to problems of saving (restart the server and the salt cache is gone).  I prefer to use an interceptor instead of the filter, and then return back to the action rather than the general exception page on any salt issue.  It is then possible to resubmit the page with a new salt value.

<interceptor-ref name="UIActionSaltInterceptor" >
                    <param name="excludeMethods">*</param>
                    <param name="includeMethods">save</param>
</interceptor-ref>
{code:java}
public class UIActionSaltInterceptor extends MethodFilterInterceptor {

	private static final Logger log = LoggerFactory
			.getLogger(UIActionSaltInterceptor.class);

	private String inputResultName = Action.INPUT;

	/**
	 * Set the <code>inputResultName</code> (result name to be returned when
	 * action fails the salt check). Default to {@link Action#INPUT}
	 * 
	 * struts.xml interceptor parameter:
	 * 
	 * <param name="inputResultName">input</param>
	 * 
	 * @param inputResultName
	 *            what result name to use when there is a salt error.
	 */
	public void setInputResultName(String inputResultName) {
		this.inputResultName = inputResultName;
	}

	/**
	 * Intercept {@link ActionInvocation} and returns a
	 * <code>inputResultName</code> when action fails the salt check.
	 * 
	 * @return String result name
	 */
	@Override
	protected String doIntercept(ActionInvocation invocation) throws Exception {
		Object action = invocation.getAction();

		if (action instanceof UIAction) {

			UIAction theAction = (UIAction) action;

			final ActionContext context = invocation.getInvocationContext();
			HttpServletRequest request = (HttpServletRequest) context
					.get(ServletActionContext.HTTP_REQUEST);

			// Check post
			if (("POST").equals(request.getMethod())) {

				SaltCache saltCache = SaltCache.getInstance();
				if (saltCache.isCacheEnabled()) {

					String salt = (String) request.getParameter("salt");

					if (salt == null || saltCache.get(salt) == null
							|| saltCache.get(salt).equals(false)) {

						if (log.isDebugEnabled())
							log.debug("Failed salt check on action "
									+ theAction
									+ ", returning result name 'input'");

						// Indicate the error to the user
						theAction.addError("error.permissions.deniedSalt");

						return inputResultName;

					}

					// Cleanup
					saltCache.remove(salt);
				}
			}
		}

		return invocation.invoke();
	}
} 
{code:java}


was (Author: gregh99):
I would say you need the no-opp setSalt(..) for completeness (messages are there for a reason).   The timeout is personal I guess. 

I never really like the way the salt csrf works here for struts UI as its leads to problems of saving (restart the server and the salt cache is gone).  I prefer to use an interceptor instead of the filter, and then return back to the action rather than the general exception page on any salt issue.  It is then possible to resubmit the page with a new salt value.

<interceptor-ref name="UIActionSaltInterceptor" >
                    <param name="excludeMethods">*</param>
                    <param name="includeMethods">save</param>
</interceptor-ref>

public class UIActionSaltInterceptor extends MethodFilterInterceptor {

	private static final Logger log = LoggerFactory
			.getLogger(UIActionSaltInterceptor.class);

	private String inputResultName = Action.INPUT;

	/**
	 * Set the <code>inputResultName</code> (result name to be returned when
	 * action fails the salt check). Default to {@link Action#INPUT}
	 * 
	 * struts.xml interceptor parameter:
	 * 
	 * <param name="inputResultName">input</param>
	 * 
	 * @param inputResultName
	 *            what result name to use when there is a salt error.
	 */
	public void setInputResultName(String inputResultName) {
		this.inputResultName = inputResultName;
	}

	/**
	 * Intercept {@link ActionInvocation} and returns a
	 * <code>inputResultName</code> when action fails the salt check.
	 * 
	 * @return String result name
	 */
	@Override
	protected String doIntercept(ActionInvocation invocation) throws Exception {
		Object action = invocation.getAction();

		if (action instanceof UIAction) {

			UIAction theAction = (UIAction) action;

			final ActionContext context = invocation.getInvocationContext();
			HttpServletRequest request = (HttpServletRequest) context
					.get(ServletActionContext.HTTP_REQUEST);

			// Check post
			if (("POST").equals(request.getMethod())) {

				SaltCache saltCache = SaltCache.getInstance();
				if (saltCache.isCacheEnabled()) {

					String salt = (String) request.getParameter("salt");

					if (salt == null || saltCache.get(salt) == null
							|| saltCache.get(salt).equals(false)) {

						if (log.isDebugEnabled())
							log.debug("Failed salt check on action "
									+ theAction
									+ ", returning result name 'input'");

						// Indicate the error to the user
						theAction.addError("error.permissions.deniedSalt");

						return inputResultName;

					}

					// Cleanup
					saltCache.remove(salt);
				}
			}
		}

		return invocation.invoke();
	}
} 

> No salt renewal on POST request
> -------------------------------
>
>                 Key: ROL-2058
>                 URL: https://issues.apache.org/jira/browse/ROL-2058
>             Project: Apache Roller
>          Issue Type: Bug
>          Components: User Interface - General
>    Affects Versions: 5.1.1
>         Environment: WildFly 8.2.0.Final
>            Reporter: Kohei Nozaki
>            Assignee: David Johnson
>             Fix For: 5.1.2
>
>         Attachments: ROL-2058.patch
>
>
> Roller continues using previous salt value which sent from client as POST parameter. this leads fixing of salt value in the form element of html, and brings ServletException("Security Violation") by ValidateSaltFilter at some use cases (e.g. long-term editing over 60 minutes) unexpectedly.
> Seems to that the cause is existence of org.apache.roller.weblogger.ui.struts2.util.UIAction#setSalt(String) method. this overwrites salt with previous value which sent by client as POST parameter. it's unnecessary behavior because new salt value comes through preceding invocation of UIAction#setRequest(Map).
> Original discussion in the mailing list:
> http://markmail.org/search/?q=list%3Aorg.apache.roller.user#query:list%3Aorg.apache.roller.user+page:1+mid:tnqn4qjuwmwun4oh+state:results



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)