Return-Path: 
 <commits-return-15011-apmail-jackrabbit-commits-archive=jackrabbit.apache.org@jackrabbit.apache.org>
X-Original-To: apmail-jackrabbit-commits-archive@www.apache.org
Delivered-To: apmail-jackrabbit-commits-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 6204810676
	for <apmail-jackrabbit-commits-archive@www.apache.org>;
 Tue, 27 May 2014 14:53:07 +0000 (UTC)
Received: (qmail 95969 invoked by uid 500); 27 May 2014 14:53:07 -0000
Delivered-To: apmail-jackrabbit-commits-archive@jackrabbit.apache.org
Received: (qmail 95918 invoked by uid 500); 27 May 2014 14:53:07 -0000
Mailing-List: contact commits-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@jackrabbit.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@jackrabbit.apache.org>
List-Post: <mailto:commits@jackrabbit.apache.org>
List-Id: <commits.jackrabbit.apache.org>
Reply-To: dev@jackrabbit.apache.org
Delivered-To: mailing list commits@jackrabbit.apache.org
Received: (qmail 95911 invoked by uid 99); 27 May 2014 14:53:07 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 27 May 2014 14:53:07 +0000
X-ASF-Spam-Status: No, hits=-2000.0 required=5.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4)
    by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 27 May 2014 14:53:07 +0000
Received: from eris.apache.org (localhost [127.0.0.1])
	by eris.apache.org (Postfix) with ESMTP id A9C502388860;
	Tue, 27 May 2014 14:52:42 +0000 (UTC)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1597799 - in
 /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security:
 authorization/GlobPattern.java user/PasswordUtility.java
 user/XPathQueryEvaluator.java
Date: Tue, 27 May 2014 14:52:42 -0000
To: commits@jackrabbit.apache.org
From: angela@apache.org
X-Mailer: svnmailer-1.0.9
Message-Id: <20140527145242.A9C502388860@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

Author: angela
Date: Tue May 27 14:52:42 2014
New Revision: 1597799

URL: http://svn.apache.org/r1597799
Log:
JCR-3782 : Backport OAK-1612, OAK-1615, OAK-1616

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/GlobPattern.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/PasswordUtility.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/GlobPattern.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/GlobPattern.java?rev=1597799&r1=1597798&r2=1597799&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/GlobPattern.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/GlobPattern.java Tue May 27 14:52:42 2014
@@ -16,13 +16,13 @@
  */
 package org.apache.jackrabbit.core.security.authorization;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.apache.jackrabbit.util.Text;
-
 import javax.jcr.Item;
 import javax.jcr.RepositoryException;
 
+import org.apache.jackrabbit.util.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * <code>GlobPattern</code> defines a simplistic pattern matching. It consists
  * of a mandatory (leading) path and an optional "glob" that may contain one or
@@ -64,7 +64,7 @@ public final class GlobPattern {
     private static Logger log = LoggerFactory.getLogger(GlobPattern.class);
 
     private static final char WILDCARD_CHAR = '*';
-    private static final String WILDCARD_STRING = "*";
+    private static final int  MAX_WILDCARD = 20;
 
     private final String nodePath;
     private final String restriction;
@@ -220,7 +220,7 @@ public final class GlobPattern {
             }
             char[] tm = (toMatch.endsWith("/")) ? toMatch.substring(0, toMatch.length()-1).toCharArray() : toMatch.toCharArray();
             // shortcut didn't reveal mismatch -> need to process the internal match method.
-            return matches(patternChars, 0, tm, 0);
+            return matches(patternChars, 0, tm, 0, MAX_WILDCARD);
         }
 
         /**
@@ -232,8 +232,11 @@ public final class GlobPattern {
          * @return <code>true</code> if matches, <code>false</code> otherwise
          */
         private boolean matches(char[] pattern, int pOff,
-                                char[] s, int sOff) {
+                                char[] s, int sOff, int cnt) {
 
+            if (cnt <= 0) {
+                throw new IllegalArgumentException("Illegal glob pattern " + GlobPattern.this);
+            }
             /*
             NOTE: code has been copied (and slightly modified) from
             ChildrenCollectorFilter#internalMatches.
@@ -263,8 +266,9 @@ public final class GlobPattern {
                         return true;
                     }
 
+                    cnt--;
                     while (true) {
-                        if (matches(pattern, pOff, s, sOff)) {
+                        if (matches(pattern, pOff, s, sOff, cnt)) {
                             return true;
                         }
                         if (sOff >= sLength) {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/PasswordUtility.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/PasswordUtility.java?rev=1597799&r1=1597798&r2=1597799&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/PasswordUtility.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/PasswordUtility.java Tue May 27 14:52:42 2014
@@ -17,15 +17,15 @@
 
 package org.apache.jackrabbit.core.security.user;
 
-import org.apache.jackrabbit.util.Text;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import java.io.UnsupportedEncodingException;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.SecureRandom;
 
+import org.apache.jackrabbit.util.Text;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * Utility to generate and compare password hashes.
  */
@@ -123,7 +123,7 @@ public class PasswordUtility {
                 }
 
                 String hash = generateHash(password, algorithm, salt, iterations);
-                return hashedPassword.equals(hash);
+                return compareSecure(hashedPassword, hash);
             } // hashedPassword is plaintext -> return false
         } catch (NoSuchAlgorithmException e) {
             log.warn(e.getMessage());
@@ -236,4 +236,33 @@ public class PasswordUtility {
         // no extra iterations
         return NO_ITERATIONS;
     }
+
+    /**
+     * Compare two strings. The comparison is constant time: it will always loop
+     * over all characters and doesn't use conditional operations in the loop to
+     * make sure an attacker can not use a timing attack.
+     *
+     * @param a
+     * @param b
+     * @return true if both parameters contain the same data.
+     */
+    private static boolean compareSecure(String a, String b) {
+        if ((a == null) || (b == null)) {
+            return (a == null) && (b == null);
+        }
+        int len = a.length();
+        if (len != b.length()) {
+            return false;
+        }
+        if (len == 0) {
+            return true;
+        }
+        // don't use conditional operations inside the loop
+        int bits = 0;
+        for (int i = 0; i < len; i++) {
+            // this will never reset any bits
+            bits |= a.charAt(i) ^ b.charAt(i);
+        }
+        return bits == 0;
+    }
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java?rev=1597799&r1=1597798&r2=1597799&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/user/XPathQueryEvaluator.java Tue May 27 14:52:42 2014
@@ -17,6 +17,14 @@
 
 package org.apache.jackrabbit.core.security.user;
 
+import java.util.Iterator;
+import javax.jcr.Node;
+import javax.jcr.PropertyType;
+import javax.jcr.RepositoryException;
+import javax.jcr.Value;
+import javax.jcr.query.Query;
+import javax.jcr.query.QueryManager;
+
 import org.apache.jackrabbit.api.security.user.Authorizable;
 import org.apache.jackrabbit.api.security.user.Group;
 import org.apache.jackrabbit.api.security.user.QueryBuilder.Direction;
@@ -34,14 +42,6 @@ import org.apache.jackrabbit.util.Text;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.jcr.Node;
-import javax.jcr.PropertyType;
-import javax.jcr.RepositoryException;
-import javax.jcr.Value;
-import javax.jcr.query.Query;
-import javax.jcr.query.QueryManager;
-import java.util.Iterator;
-
 /**
  * This evaluator for {@link org.apache.jackrabbit.api.security.user.Query}s use XPath
  * and some minimal client side filtering.
@@ -132,9 +132,9 @@ public class XPathQueryEvaluator impleme
 
         xPath.append('(')
                 .append("jcr:like(@")
-                .append(repPrincipal)
+                .append(escapeForQuery(repPrincipal))
                 .append(",'")
-                .append(condition.getPattern())
+                .append(escapeForQuery(condition.getPattern()))
                 .append("')")
                 .append(" or ")
                 .append("jcr:like(fn:name(),'")
@@ -146,15 +146,15 @@ public class XPathQueryEvaluator impleme
     public void visit(XPathQueryBuilder.PropertyCondition condition) throws RepositoryException {
         RelationOp relOp = condition.getOp();
         if (relOp == RelationOp.EX) {
-            xPath.append(condition.getRelPath());
+            xPath.append(escapeForQuery(condition.getRelPath()));
         } else if (relOp == RelationOp.LIKE) {
             xPath.append("jcr:like(")
-                    .append(condition.getRelPath())
+                    .append(escapeForQuery(condition.getRelPath()))
                     .append(",'")
-                    .append(condition.getPattern())
+                    .append(escapeForQuery(condition.getPattern()))
                     .append("')");
         } else {
-            xPath.append(condition.getRelPath())
+            xPath.append(escapeForQuery(condition.getRelPath()))
                     .append(condition.getOp().getOp())
                     .append(format(condition.getValue()));
         }
@@ -162,15 +162,15 @@ public class XPathQueryEvaluator impleme
 
     public void visit(XPathQueryBuilder.ContainsCondition condition) {
         xPath.append("jcr:contains(")
-                .append(condition.getRelPath())
+                .append(escapeForQuery(condition.getRelPath()))
                 .append(",'")
-                .append(condition.getSearchExpr())
+                .append(escapeForQuery(condition.getSearchExpr()))
                 .append("')");
     }
 
     public void visit(XPathQueryBuilder.ImpersonationCondition condition) {
         xPath.append("@rep:impersonators='")
-                .append(condition.getName())
+                .append(escapeForQuery(condition.getName()))
                 .append('\'');
     }
 
@@ -236,6 +236,21 @@ public class XPathQueryEvaluator impleme
         return result.toString();
     }
 
+    public static String escapeForQuery(String value) {
+        StringBuilder ret = new StringBuilder();
+        for (int i = 0; i < value.length(); i++) {
+            char c = value.charAt(i);
+            if (c == '\\') {
+                ret.append("\\\\");
+            } else if (c == '\'') {
+                ret.append("''");
+            } else {
+                ret.append(c);
+            }
+        }
+        return ret.toString();
+    }
+
     private String getNtName(Class<? extends Authorizable> selector) throws RepositoryException {
         if (User.class.isAssignableFrom(selector)) {
             return session.getJCRName(UserConstants.NT_REP_USER);