diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/Repository.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/Repository.java index 927dd3f2af..95b0602692 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/Repository.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/Repository.java @@ -357,10 +357,12 @@ public class Repository implements FileSystemListener, RepositoryLogger { LinkedHashMap newUserMap = new LinkedHashMap<>(); for (User user : users) { String userName = user.getName(); - if (UserManager.ANONYMOUS_USERNAME.equals(userName) || - !allUsers.contains(userName)) { + if (UserManager.ANONYMOUS_USERNAME.equals(userName)) { continue; // ignore } + if (!allUsers.contains(userName)) { + throw new IOException("Unknown user specified: " + userName); + } if (!user.hasWritePermission() && !user.isReadOnly() && !user.isAdmin()) { throw new IOException("User specified with invalid permission: " + userName); } diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/TokenGenerator.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/TokenGenerator.java index ea2349c397..8744532f88 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/TokenGenerator.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/TokenGenerator.java @@ -16,8 +16,7 @@ package ghidra.server.security; import java.security.SecureRandom; -import java.util.Date; -import java.util.Map; +import java.util.*; import java.util.concurrent.*; import generic.random.SecureRandomFactory; @@ -84,13 +83,45 @@ public class TokenGenerator { return ++offset; } + /** + * {@link Token} provides a byte array token wrapper to facilitate value-based + * hashcode and equality when used as a map key. + */ + private static class Token { + private byte[] token; + + Token(byte[] token) { + this.token = token; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + Arrays.hashCode(token); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + Token other = (Token) obj; + return Arrays.equals(token, other.token); + } + } + /** * {@link CachedTokenSet} tracks timed token issuance and insures that they remain * valid for one-time consumption within limited life-span. */ private static class CachedTokenSet { - private final Map cache = new ConcurrentHashMap<>(); + private final Map cache = new ConcurrentHashMap<>(); private final ScheduledExecutorService scheduler = Executors.newSingleThreadScheduledExecutor(); @@ -100,11 +131,11 @@ public class TokenGenerator { } void add(byte[] token) { - cache.put(token, System.currentTimeMillis()); + cache.put(new Token(token), System.currentTimeMillis()); } - boolean consume(byte[] value) { - Long storedAt = cache.remove(value); // remove on retrieval + boolean consume(byte[] token) { + Long storedAt = cache.remove(new Token(token)); // remove on retrieval if (storedAt == null) return false; return (System.currentTimeMillis() - storedAt < MAX_TTL_MS); @@ -115,5 +146,4 @@ public class TokenGenerator { cache.entrySet().removeIf(e -> now - e.getValue() >= MAX_TTL_MS); } } - } diff --git a/Ghidra/Test/IntegrationTest/src/test.slow/java/ghidra/server/RepositoryTest.java b/Ghidra/Test/IntegrationTest/src/test.slow/java/ghidra/server/RepositoryTest.java index aeadceb3f3..4ada4a187a 100644 --- a/Ghidra/Test/IntegrationTest/src/test.slow/java/ghidra/server/RepositoryTest.java +++ b/Ghidra/Test/IntegrationTest/src/test.slow/java/ghidra/server/RepositoryTest.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -18,6 +18,7 @@ package ghidra.server; import static org.junit.Assert.*; import java.io.File; +import java.io.IOException; import org.junit.*; @@ -50,7 +51,13 @@ public class RepositoryTest extends AbstractGhidraHeadedIntegrationTest { serverRoot.mkdir(); mgr = new RepositoryManager(serverRoot, false, 0, false); - mgr.getUserManager().addUser(userName); + UserManager userManager = mgr.getUserManager(); + userManager.addUser(userName); + + userManager.addUser("user-a"); + userManager.addUser("user-b"); + userManager.addUser("user-c"); + userManager.addUser("user-d"); repository = mgr.createRepository(userName, REPOSITORY_NAME); } @@ -94,17 +101,10 @@ public class RepositoryTest extends AbstractGhidraHeadedIntegrationTest { Assert.fail("Should not have been able to change current user's access!"); } catch (UserAccessException e) { + // expected } - users[3] = new User("user-x", User.ADMIN); - try { - repository.setUserList(userName, users, false); - Assert.fail("Should not have been able to set the user list!"); - } - catch (UserAccessException e) { - } - - users[4] = new User(userName, User.ADMIN); + users[4] = new User(userName, User.ADMIN); // restore current users Admin access repository.setUserList(userName, users, false); User[] reportedUsers = repository.getUserList(userName); @@ -113,6 +113,15 @@ public class RepositoryTest extends AbstractGhidraHeadedIntegrationTest { assertEquals(users[i].getName(), reportedUsers[i].getName()); assertEquals(users[i].getPermissionType(), reportedUsers[i].getPermissionType()); } + + users[3] = new User("user-x", User.ADMIN); + try { + repository.setUserList(userName, users, false); + Assert.fail("Should not have been able to specify unknown user"); + } + catch (IOException e) { + // expected + } } @Test