diff --git a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/jcr/JCRWebdavServer.java b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/jcr/JCRWebdavServer.java index 461b5818701..6de04427f79 100644 --- a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/jcr/JCRWebdavServer.java +++ b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/server/jcr/JCRWebdavServer.java @@ -227,6 +227,10 @@ private DavSession get(WebdavRequest request) } } + if (session != null) { + session = checkSameUser(session, request); + } + // no cached session present -> create new one. if (session == null) { Session repSession = getRepositorySession(request); @@ -242,7 +246,34 @@ private DavSession get(WebdavRequest request) return session; } - /** + private DavSession checkSameUser(DavSession cached, WebdavRequest request) + throws DavException { + Session repSession = getRepositorySession(request); + String userId = repSession.getUserID(); + String cachedUserId = null; + try { + Session cachedSession = DavSessionImpl.getRepositorySession(cached); + if (cachedSession != null) { + cachedUserId = cachedSession.getUserID(); + } + } catch (DavException e) { + // should not occur, since only DavSessionImpl instances + // are put into the cache. + log.error("Unexpected error: {}", e.getMessage(), e.getCause()); + } + if (cachedUserId != null && cachedUserId.equals(userId)) { + // same user: reuse the cached session and release the + // session that was only needed to authenticate the request. + sessionProvider.releaseSession(repSession); + return cached; + } else { + log.warn("Request of user '{}' presented a token referencing a session of user '{}' - token ignored.", userId, cachedUserId); + DavSession session = new DavSessionImpl(repSession); + sessionMap.put(session, new HashSet<>()); + return session; + } + } + /** * Add a references to the specified DavSession. * * @param session diff --git a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/DefaultItemCollection.java b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/DefaultItemCollection.java index b6e3efff1bb..a762ed6918f 100644 --- a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/DefaultItemCollection.java +++ b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/DefaultItemCollection.java @@ -62,6 +62,7 @@ import org.apache.jackrabbit.webdav.DavResourceLocator; import org.apache.jackrabbit.webdav.DavServletResponse; import org.apache.jackrabbit.webdav.MultiStatusResponse; +import org.apache.jackrabbit.webdav.jcr.lock.LockTokenMapper; import org.apache.jackrabbit.webdav.jcr.property.JcrDavPropertyNameSet; import org.apache.jackrabbit.webdav.jcr.security.JcrUserPrivilegesProperty; import org.apache.jackrabbit.webdav.jcr.security.JcrSupportedPrivilegesProperty; @@ -695,6 +696,10 @@ public ActiveLock lock(LockInfo reqLockInfo) throws DavException { javax.jcr.lock.LockManager lockMgr = getRepositorySession().getWorkspace().getLockManager(); Lock jcrLock = lockMgr.lock((item).getPath(), reqLockInfo.isDeep(), sessionScoped, timeout, reqLockInfo.getOwner()); + if (sessionScoped) { + // discard any mapping of a previous lock on this node + LockTokenMapper.releaseSessionScopedToken(jcrLock.getNode().getIdentifier()); + } ActiveLock lock = new JcrActiveLock(jcrLock); // add reference to DAVSession for this lock getSession().addReference(lock.getToken()); @@ -767,8 +772,12 @@ public void unlock(String lockToken) throws DavException { ActiveLock lock = getWriteLock(); if (lock != null && lockToken.equals(lock.getToken())) { try { - ((Node) item).unlock(); - getSession().removeReference(lock.getToken()); + Node n = (Node) item; + n.unlock(); + getSession().removeReference(lockToken); + // the lock is gone: discard its DAV token secret so it can + // not be replayed against a future lock on this node. + LockTokenMapper.releaseSessionScopedToken(n.getIdentifier()); } catch (RepositoryException e) { throw new JcrDavException(e); } diff --git a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/lock/LockTokenMapper.java b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/lock/LockTokenMapper.java index 2daba0a0828..2aed70264fe 100644 --- a/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/lock/LockTokenMapper.java +++ b/jackrabbit-jcr-server/src/main/java/org/apache/jackrabbit/webdav/jcr/lock/LockTokenMapper.java @@ -19,15 +19,20 @@ import javax.jcr.RepositoryException; import javax.jcr.lock.Lock; +import org.apache.commons.collections4.map.LRUMap; import org.apache.jackrabbit.util.Text; +import java.util.Collections; +import java.util.Map; +import java.util.UUID; + /** * Maps between WebDAV lock tokens and JCR lock tokens. *

* The following notations are used: * *

- * opaquelocktoken:SESSIONSCOPED:NODEIDENTIFIER
+ * urn:uuid:UUID of mapping
  * opaquelocktoken:OPENSCOPED:JCRLOCKTOKEN
  * 
* @@ -44,22 +49,34 @@ public class LockTokenMapper { private static final String OL = "opaquelocktoken:"; - private static final String SESSIONSCOPED = "4403ef44-4124-11e1-b965-00059a3c7a00"; private static final String OPENSCOPED = "dccce564-412e-11e1-b969-00059a3c7a00"; - private static final String SESSPREFIX = OL + SESSIONSCOPED + ":"; + private static final String SESSPREFIX = "urn:uuid:"; private static final String OPENPREFIX = OL + OPENSCOPED + ":"; + // map node identifiers to randomized session locks, 4k mappings should be enough for everyone + private static final Map mappings = Collections.synchronizedMap(new LRUMap<>(4196)); + + private LockTokenMapper() { + /* This utility class should not be instantiated */ + } + + /** + * Generate a WebDAV lock token from a JCR {@link Lock}. + */ public static String getDavLocktoken(Lock lock) throws RepositoryException { String jcrLockToken = lock.getLockToken(); if (jcrLockToken == null) { - return SESSPREFIX + Text.escape(lock.getNode().getIdentifier()); + return SESSPREFIX + Text.escape(getMapping(lock.getNode().getIdentifier())); } else { return OPENPREFIX + Text.escape(jcrLockToken); } } + /** + * Map from a WebDAV lock token to JCR lock token (requires an open scoped lock). + */ public static String getJcrLockToken(String token) throws RepositoryException { if (token.startsWith(OPENPREFIX)) { return Text.unescape(token.substring(OPENPREFIX.length())); @@ -68,7 +85,26 @@ public static String getJcrLockToken(String token) throws RepositoryException { } } + /** + * Discards the DAV lock token mapping recorded for the given node, if + * present. This is called when a lock is removed or newly created, so + * that the token of an earlier lock on the same node can never be + * reused against a later one. + * + * @param nodeIdentifier the identifier of the lock holding node. + */ + public static void releaseSessionScopedToken(String nodeIdentifier) { + mappings.remove(nodeIdentifier); + } + public static boolean isForSessionScopedLock(String token) { return token.startsWith(SESSPREFIX); } + + private static String getMapping(String nodeIdentifier) { + return mappings.computeIfAbsent( + nodeIdentifier, + k -> UUID.randomUUID().toString() + ); + } } diff --git a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/webdav/jcr/LockTokenMappingTest.java b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/webdav/jcr/LockTokenMappingTest.java index 4b40748b7f0..49ab8a6297d 100644 --- a/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/webdav/jcr/LockTokenMappingTest.java +++ b/jackrabbit-jcr-server/src/test/java/org/apache/jackrabbit/webdav/jcr/LockTokenMappingTest.java @@ -16,6 +16,9 @@ */ package org.apache.jackrabbit.webdav.jcr; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import java.net.URI; import java.net.URISyntaxException; import java.util.UUID; @@ -23,7 +26,6 @@ import javax.jcr.Node; import javax.jcr.RepositoryException; import javax.jcr.lock.Lock; -import javax.jcr.lock.LockException; import org.apache.jackrabbit.webdav.jcr.lock.LockTokenMapper; @@ -44,6 +46,42 @@ public void testOpenScopedFancy() throws RepositoryException, URISyntaxException testRoundtrip("\n\u00c4 \u20ac"); } + // the DAV token of a session-scoped lock must not be derivable from + // the (world-readable) node identifier + public void testSessionScopedTokenIsNotDerivedFromNodeId() + throws RepositoryException, URISyntaxException { + String nodeId = UUID.randomUUID().toString(); + Lock l = new TestLock(nodeWithIdentifier(nodeId), true); + String davtoken = LockTokenMapper.getDavLocktoken(l); + + assertNotNull(davtoken); + assertTrue(LockTokenMapper.isForSessionScopedLock(davtoken)); + assertTrue("token must not contain the node identifier", + davtoken.indexOf(nodeId) < 0); + + // valid URI? + URI u = new URI(davtoken); + assertTrue("lock token must be absolute URI", u.isAbsolute()); + assertEquals("lock token URI must be all-ASCII", u.toASCIIString(), u.toString()); + + // stable for the same lock... + assertEquals(davtoken, LockTokenMapper.getDavLocktoken(l)); + + // ...but not reused once the lock is gone + LockTokenMapper.releaseSessionScopedToken(nodeId); + String next = LockTokenMapper.getDavLocktoken(l); + assertFalse("a new lock on the same node must get a fresh token", + davtoken.equals(next)); + } + + // two session-scoped locks on different nodes must get distinct tokens + public void testSessionScopedTokensAreDistinct() throws RepositoryException { + Lock l1 = new TestLock(nodeWithIdentifier(UUID.randomUUID().toString()), true); + Lock l2 = new TestLock(nodeWithIdentifier(UUID.randomUUID().toString()), true); + assertFalse(LockTokenMapper.getDavLocktoken(l1).equals( + LockTokenMapper.getDavLocktoken(l2))); + } + private void testRoundtrip(String token) throws RepositoryException, URISyntaxException { Lock l = new TestLock(token); @@ -58,15 +96,46 @@ private void testRoundtrip(String token) throws RepositoryException, URISyntaxEx assertEquals(jcrtoken, l.getLockToken()); } + /** + * Returns a minimal {@link Node} that only supports + * {@link Node#getIdentifier()}. + */ + private static Node nodeWithIdentifier(final String id) { + return (Node) Proxy.newProxyInstance( + LockTokenMappingTest.class.getClassLoader(), + new Class[] {Node.class}, + new InvocationHandler() { + public Object invoke(Object proxy, Method method, Object[] args) { + if ("getIdentifier".equals(method.getName())) { + return id; + } + throw new UnsupportedOperationException(method.getName()); + } + }); + } + /** * Minimal Lock impl for tests above */ private static class TestLock implements Lock { private final String token; + private final Node node; + private final boolean sessionScoped; + private final boolean lockOwningSession; public TestLock(String token) { this.token = token; + this.node = null; + this.sessionScoped = false; + this.lockOwningSession = false; + } + + public TestLock(Node node, boolean lockOwningSession) { + this.token = null; + this.node = node; + this.sessionScoped = true; + this.lockOwningSession = lockOwningSession; } public String getLockOwner() { @@ -78,30 +147,30 @@ public boolean isDeep() { } public Node getNode() { - return null; + return node; } public String getLockToken() { return token; } - public long getSecondsRemaining() throws RepositoryException { + public long getSecondsRemaining() { return 0; } - public boolean isLive() throws RepositoryException { + public boolean isLive() { return false; } public boolean isSessionScoped() { - return false; + return sessionScoped; } public boolean isLockOwningSession() { - return false; + return lockOwningSession; } - public void refresh() throws LockException, RepositoryException { + public void refresh() { } } -} +} \ No newline at end of file