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 bc130df54a..927dd3f2af 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/Repository.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/Repository.java @@ -351,12 +351,22 @@ public class Repository implements FileSystemListener, RepositoryLogger { validate(); validateAdminPrivilege(currentUser); + Set allUsers = Set.of(mgr.getAllUsers(currentUser)); + Set updatedUsers = new HashSet<>(); + LinkedHashMap newUserMap = new LinkedHashMap<>(); for (User user : users) { String userName = user.getName(); - if (UserManager.ANONYMOUS_USERNAME.equals(userName)) { + if (UserManager.ANONYMOUS_USERNAME.equals(userName) || + !allUsers.contains(userName)) { continue; // ignore } + if (!user.hasWritePermission() && !user.isReadOnly() && !user.isAdmin()) { + throw new IOException("User specified with invalid permission: " + userName); + } + if (!updatedUsers.add(userName)) { + throw new IOException("Duplicate user entry specified: " + userName); + } newUserMap.put(userName, user); } User user = newUserMap.get(currentUser); @@ -508,7 +518,8 @@ public class Repository implements FileSystemListener, RepositoryLogger { * @throws UserAccessException if currentUser does not have admin priviledge * @throws IOException if an IO error occurs */ - private void writeUserList(LinkedHashMap newUserMap, boolean allowAnonymous) + private synchronized void writeUserList(LinkedHashMap newUserMap, + boolean allowAnonymous) throws IOException { File temp = new File(userAccessFile.getParentFile(), "tempAccess.tmp"); diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java index 515434c47a..a64090f1f6 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/UserManager.java @@ -751,10 +751,10 @@ public class UserManager { } /* - * Regex: matches if the entire string is alpha, digit, ".", "-", "_", fwd or back slash. + * Regex: matches if the entire string is alpha, digit, ".", "-", "_". */ private static final Pattern VALID_USERNAME_REGEX = - Pattern.compile("[a-zA-Z0-9][a-zA-Z0-9.\\-_/\\\\]*"); + Pattern.compile("[a-zA-Z0-9][a-zA-Z0-9.\\-_]*"); /** * Ensures a name only contains valid characters. diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/remote/RepositoryHandleImpl.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/remote/RepositoryHandleImpl.java index e0cf51dcb5..38134408c8 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/remote/RepositoryHandleImpl.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/remote/RepositoryHandleImpl.java @@ -718,7 +718,7 @@ public class RepositoryHandleImpl extends UnicastRemoteObject public void terminateCheckout(String parentPath, String itemName, long checkoutId, boolean notify) throws IOException { synchronized (syncObject) { - validate(); // relax read-only restriction + validate(); try { RepositoryFile rf = getFile(parentPath, itemName); if (rf != null) { diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/PKIAuthenticationModule.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/PKIAuthenticationModule.java index 626610b272..c1543a6311 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/PKIAuthenticationModule.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/PKIAuthenticationModule.java @@ -39,10 +39,8 @@ import ghidra.server.remote.RemoteLoggingUtil; * use of a dual-signed token. */ public class PKIAuthenticationModule implements AuthenticationModule { - static final Logger log = LogManager.getLogger(PKIAuthenticationModule.class); - private static final long MAX_TOKEN_TIME = 5 * 60000; // 5-minutes - private static final int TOKEN_SIZE = 64; + static final Logger log = LogManager.getLogger(PKIAuthenticationModule.class); private X500Principal[] authorities; // imposed on client certificate private boolean anonymousAllowed; @@ -70,7 +68,7 @@ public class PKIAuthenticationModule implements AuthenticationModule { public Callback[] getAuthenticationCallbacks() { SignatureCallback sigCb; try { - byte[] token = TokenGenerator.getNewToken(TOKEN_SIZE); + byte[] token = TokenGenerator.getNewToken(); boolean usingSelfSignedCert = DefaultKeyManagerFactory.usingGeneratedSelfSignedCertificate(); SignedToken signedToken = DefaultKeyManagerFactory @@ -88,27 +86,6 @@ public class PKIAuthenticationModule implements AuthenticationModule { return false; } - private void checkTokenIntegrity(byte[] token) throws LoginException { - if (token.length != TOKEN_SIZE) { - throw new FailedLoginException("Invalid Signature callback"); - } - - boolean isZeroToken = true; - for (byte b : token) { - if (b != 0) { - isZeroToken = false; - break; - } - } - if (isZeroToken) { - throw new FailedLoginException("Invalid Signature callback"); - } - - if (!TokenGenerator.isRecentToken(token, MAX_TOKEN_TIME)) { - throw new FailedLoginException("Stale Signature callback"); - } - } - /* * @see ghidra.server.security.AuthenticationModule#authenticate(ghidra.server.UserManager, javax.security.auth.Subject, javax.security.auth.callback.Callback[]) */ @@ -142,7 +119,9 @@ public class PKIAuthenticationModule implements AuthenticationModule { try { byte[] token = sigCb.getToken(); - checkTokenIntegrity(token); + if (!TokenGenerator.isValidToken(token)) { + throw new FailedLoginException("Stale Signature callback"); + } boolean usingSelfSignedCert = DefaultKeyManagerFactory.usingGeneratedSelfSignedCertificate(); diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/SSHAuthenticationModule.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/SSHAuthenticationModule.java index 17eda870b0..80e1c2966f 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/SSHAuthenticationModule.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/security/SSHAuthenticationModule.java @@ -45,8 +45,7 @@ import ghidra.server.UserManager; */ public class SSHAuthenticationModule { - private static final long MAX_TOKEN_TIME = 10000; - private static final int TOKEN_SIZE = 64; + private final boolean nameCallbackAllowed; @@ -72,7 +71,7 @@ public class SSHAuthenticationModule { if (addNameCallback) { list.add(new NameCallback("User ID:")); } - byte[] token = TokenGenerator.getNewToken(TOKEN_SIZE); + byte[] token = TokenGenerator.getNewToken(); try { boolean usingSelfSignedCert = DefaultKeyManagerFactory.usingGeneratedSelfSignedCertificate(); @@ -190,7 +189,7 @@ public class SSHAuthenticationModule { } byte[] token = sshCb.getToken(); - if (!TokenGenerator.isRecentToken(token, MAX_TOKEN_TIME)) { + if (!TokenGenerator.isValidToken(token)) { throw new FailedLoginException("Stale SSH Signature callback"); } 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 991b01c94f..ea2349c397 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 @@ -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. @@ -17,27 +17,52 @@ package ghidra.server.security; import java.security.SecureRandom; import java.util.Date; +import java.util.Map; +import java.util.concurrent.*; import generic.random.SecureRandomFactory; public class TokenGenerator { - static byte[] getNewToken(int size) { + private static final long MAX_TTL_MS = 60_000; // max token time-to-live 60s + + private static final int TOKEN_SIZE = 64; + + private static CachedTokenSet tokenCache = new CachedTokenSet(); + + /** + * {@return a single-use token byte sequence with embedded timestamp} + */ + static byte[] getNewToken() { SecureRandom random = SecureRandomFactory.getSecureRandom(); - byte[] token = new byte[size - 8]; + byte[] token = new byte[TOKEN_SIZE - 8]; random.nextBytes(token); - byte[] stampedToken = new byte[token.length + 8]; + byte[] stampedToken = new byte[TOKEN_SIZE]; System.arraycopy(token, 0, stampedToken, 8, token.length); putLong(stampedToken, 0, (new Date()).getTime()); + tokenCache.add(stampedToken); return stampedToken; } - static boolean isRecentToken(byte[] token, long maxTime) { - if (token.length < 8) { + /** + * Determine if the specified token has not yet been consumed and is still valid. + *

+ * NOTE: This method may only be invoked once per token after which the token will become + * invalid. + * + * @param token token previously issued + * @return true if token is valid and now consumed + */ + static boolean isValidToken(byte[] token) { + if (token.length != TOKEN_SIZE || !tokenCache.consume(token)) { return false; } - long diff = (new Date()).getTime() - getLong(token, 0); - return (diff >= 0 && diff < maxTime); + long issueTime = getLong(token, 0); + if (issueTime <= 0) { + return false; + } + long diff = (new Date()).getTime() - issueTime; + return (diff >= 0 && diff < MAX_TTL_MS); } private static long getLong(byte[] data, int offset) { @@ -59,4 +84,36 @@ public class TokenGenerator { return ++offset; } + /** + * {@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 ScheduledExecutorService scheduler = + Executors.newSingleThreadScheduledExecutor(); + + CachedTokenSet() { + // Perform token cleanup every 5-seconds + scheduler.scheduleAtFixedRate(this::cleanup, 5, 5, TimeUnit.SECONDS); + } + + void add(byte[] token) { + cache.put(token, System.currentTimeMillis()); + } + + boolean consume(byte[] value) { + Long storedAt = cache.remove(value); // remove on retrieval + if (storedAt == null) + return false; + return (System.currentTimeMillis() - storedAt < MAX_TTL_MS); + } + + private void cleanup() { + long now = System.currentTimeMillis(); + cache.entrySet().removeIf(e -> now - e.getValue() >= MAX_TTL_MS); + } + } + } diff --git a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java index 53038ae175..edfa61d294 100644 --- a/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java +++ b/Ghidra/Features/GhidraServer/src/main/java/ghidra/server/store/RepositoryFile.java @@ -353,6 +353,7 @@ public class RepositoryFile { throws IOException { synchronized (fileSystem) { validate(); + repository.validateWritePrivilege(user); // don't allow update if read-only folderItem.updateCheckoutVersion(checkoutId, checkoutVersion, user); } } @@ -367,6 +368,7 @@ public class RepositoryFile { public void terminateCheckout(long checkoutId, String user, boolean notify) throws IOException { synchronized (fileSystem) { validate(); + repository.validateWritePrivilege(user); // don't allow update if read-only ItemCheckoutStatus coStatus = folderItem.getCheckout(checkoutId); if (coStatus != null) { User userObj = repository.getUser(user); diff --git a/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SSHSignatureCallback.java b/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SSHSignatureCallback.java index 2f6e1a6ace..217a3865fc 100644 --- a/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SSHSignatureCallback.java +++ b/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SSHSignatureCallback.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. @@ -33,9 +33,12 @@ import org.bouncycastle.util.Strings; * by the server with a random token which must be signed using the * user's SSH private key. *

- * It is the responsibility of the callback handler to invoke the - * sign method and return this object in response - * to the callback. + * It is the responsibility of the callback handler to invoke the sign method and return this + * object in response to the callback. This callback must be signed and returned to the server + * in a short period of time or the authentication will fail. + *

+ * The supplied token is validated by the server during authentication as one that it had issued + * but is primarily intended as the basis for the client's signature. */ public class SSHSignatureCallback implements Callback, Serializable { @@ -56,17 +59,17 @@ public class SSHSignatureCallback implements Callback, Serializable { } /** - * @return token to be signed using user certificate. + * {@return token to be signed using user certificate} */ public byte[] getToken() { - return (token == null ? null : (byte[]) token.clone()); + return token; } /** - * @return signed token bytes set by callback handler. + * {@return signed token bytes set by callback handler} */ public byte[] getSignature() { - return (signature == null ? null : (byte[]) signature.clone()); + return signature; } /** @@ -78,7 +81,7 @@ public class SSHSignatureCallback implements Callback, Serializable { } /** - * @return true if callback has been signed + * {@return true if callback has been signed} */ public boolean isSigned() { return signature != null; diff --git a/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SignatureCallback.java b/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SignatureCallback.java index 22d947feb4..f236552604 100644 --- a/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SignatureCallback.java +++ b/Ghidra/Framework/FileSystem/src/main/java/ghidra/framework/remote/SignatureCallback.java @@ -32,6 +32,10 @@ import javax.security.auth.x500.X500Principal; * It is the responsibility of the callback handler to invoke the * sign(X509Certificate[], byte[]) and return this object in response * to the callback. + *

+ * The supplied token is validated by the server during authentication as one that it had issued + * but is primarily intended as the basis for the client's signature. This callback must be + * signed and returned to the server in a short period of time or the authentication will fail. */ public class SignatureCallback implements Callback, Serializable { @@ -49,6 +53,7 @@ public class SignatureCallback implements Callback, Serializable { * @param recognizedAuthorities list of CA's from which one must occur * within the certificate chain of the signing certificate. * @param token random bytes to be signed + * @param serverSignature servers signature of token at time of generation */ public SignatureCallback(X500Principal[] recognizedAuthorities, byte[] token, byte[] serverSignature) { @@ -58,35 +63,36 @@ public class SignatureCallback implements Callback, Serializable { } /** - * Returns list of approved certificate authorities. + * {@return list of approved certificate authorities which constrains which user certificate is + * used to authenticate.} */ public Principal[] getRecognizedAuthorities() { - return (recognizedAuthorities == null ? null : (Principal[]) recognizedAuthorities.clone()); + return recognizedAuthorities; } /** - * Returns token to be signed using user certificate. + * {@return token to be signed using user certificate} */ public byte[] getToken() { - return (token == null ? null : (byte[]) token.clone()); + return token; } /** - * Returns signed token bytes set by callback handler. + * {@return signed token bytes set by callback handler} */ public byte[] getSignature() { - return (signature == null ? null : (byte[]) signature.clone()); + return signature; } /** - * Returns the server's signature of the token bytes. + * {@return the server's signature of the token bytes} */ public byte[] getServerSignature() { return serverSignature; } /** - * Returns certificate chain used to sign token. + * {@return certificate chain used to sign token} */ public X509Certificate[] getCertificateChain() { return certChain; @@ -100,12 +106,7 @@ public class SignatureCallback implements Callback, Serializable { */ public void sign(X509Certificate[] sigCertChain, byte[] certSignature) { this.certChain = sigCertChain; - this.signature = (certSignature == null ? null : certSignature.clone()); - } - - public String getSigAlg() { - // TODO Auto-generated method stub - return null; + this.signature = certSignature; } } diff --git a/Ghidra/Framework/Utility/src/main/java/ghidra/util/SystemUtilities.java b/Ghidra/Framework/Utility/src/main/java/ghidra/util/SystemUtilities.java index de27bb7cc2..338c792109 100644 --- a/Ghidra/Framework/Utility/src/main/java/ghidra/util/SystemUtilities.java +++ b/Ghidra/Framework/Utility/src/main/java/ghidra/util/SystemUtilities.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. @@ -101,7 +101,8 @@ public class SystemUtilities { /** * Clean the specified user name to eliminate any spaces or leading domain name - * which may be present (e.g., "MyDomain\John Doe" becomes "JohnDoe"). + * which may be present (e.g., "MyDomain\John Doe" becomes "JohnDoe"). Treat '/' in + * a similar fashion. * @param name user name string to be cleaned-up * @return the clean user name */ @@ -118,11 +119,15 @@ public class SystemUtilities { uname = nameBuf.toString(); } - // Remove leading Domain Name if present + // Remove leading Domain Name if present (treat / and \ in a similar fashion) int slashIndex = uname.lastIndexOf('\\'); if (slashIndex >= 0) { uname = uname.substring(slashIndex + 1); } + slashIndex = uname.lastIndexOf('/'); + if (slashIndex >= 0) { + uname = uname.substring(slashIndex + 1); + } return uname; }