Start working on sensitive commands [WIP]

Test doesn't compile, needs feedback.
#1703 related
This commit is contained in:
Gabriele C 2018-12-07 12:24:57 +01:00
parent b90f5fb85b
commit 93d16d273f
9 changed files with 102 additions and 65 deletions

View File

@ -211,7 +211,8 @@ public class AuthMe extends JavaPlugin {
// Get settings and set up logger
settings = injector.getSingleton(Settings.class);
ConsoleLogger.setLoggingOptions(settings);
OnStartupTasks.setupConsoleFilter(settings, getLogger());
OnStartupTasks onStartupTasks = injector.newInstance(OnStartupTasks.class);
onStartupTasks.setupConsoleFilter(settings, getLogger());
// Set all service fields on the AuthMe class
instantiateServices(injector);
@ -229,7 +230,6 @@ public class AuthMe extends JavaPlugin {
registerEventListeners(injector);
// Start Email recall task if needed
OnStartupTasks onStartupTasks = injector.newInstance(OnStartupTasks.class);
onStartupTasks.scheduleRecallEmailTask();
}

View File

@ -55,6 +55,10 @@ public class CommandDescription {
* Permission node required to execute this command.
*/
private PermissionNode permission;
/**
* Defines if the command contains sensitive data
*/
private boolean sensitive;
/**
* Private constructor.
@ -72,7 +76,8 @@ public class CommandDescription {
*/
private CommandDescription(List<String> labels, String description, String detailedDescription,
Class<? extends ExecutableCommand> executableCommand, CommandDescription parent,
List<CommandArgumentDescription> arguments, PermissionNode permission) {
List<CommandArgumentDescription> arguments, PermissionNode permission,
boolean sensitive) {
this.labels = labels;
this.description = description;
this.detailedDescription = detailedDescription;
@ -80,6 +85,7 @@ public class CommandDescription {
this.parent = parent;
this.arguments = arguments;
this.permission = permission;
this.sensitive = sensitive;
}
/**
@ -184,6 +190,10 @@ public class CommandDescription {
return permission;
}
public boolean isSensitive() {
return sensitive;
}
/**
* Return a builder instance to create a new command description.
*
@ -204,6 +214,7 @@ public class CommandDescription {
private CommandDescription parent;
private List<CommandArgumentDescription> arguments = new ArrayList<>();
private PermissionNode permission;
private Boolean sensitive;
/**
* Build a CommandDescription and register it onto the parent if available.
@ -232,7 +243,7 @@ public class CommandDescription {
// parents and permissions may be null; arguments may be empty
return new CommandDescription(labels, description, detailedDescription, executableCommand,
parent, arguments, permission);
parent, arguments, permission, sensitive == null ? false : sensitive);
}
public CommandBuilder labels(List<String> labels) {
@ -289,6 +300,17 @@ public class CommandDescription {
this.permission = permission;
return this;
}
/**
* Defines if the command contains sensitive data
*
* @param sensitive The sensitive data flag
* @return The builder
*/
public CommandBuilder sensitive(boolean sensitive) {
this.sensitive = sensitive;
return this;
}
}
}

View File

@ -94,6 +94,7 @@ public class CommandInitializer {
.detailedDescription("Command to log in using AuthMeReloaded.")
.withArgument("password", "Login password", MANDATORY)
.permission(PlayerPermission.LOGIN)
.sensitive(true)
.executableCommand(LoginCommand.class)
.register();
@ -116,6 +117,7 @@ public class CommandInitializer {
.withArgument("password", "Password", OPTIONAL)
.withArgument("verifyPassword", "Verify password", OPTIONAL)
.permission(PlayerPermission.REGISTER)
.sensitive(true)
.executableCommand(RegisterCommand.class)
.register();
@ -139,6 +141,7 @@ public class CommandInitializer {
.withArgument("oldPassword", "Old password", MANDATORY)
.withArgument("newPassword", "New password", MANDATORY)
.permission(PlayerPermission.CHANGE_PASSWORD)
.sensitive(true)
.executableCommand(ChangePasswordCommand.class)
.register();
@ -197,6 +200,7 @@ public class CommandInitializer {
.withArgument("player", "Player name", MANDATORY)
.withArgument("password", "Password", MANDATORY)
.permission(AdminPermission.REGISTER)
.sensitive(true)
.executableCommand(RegisterAdminCommand.class)
.register();
@ -231,6 +235,7 @@ public class CommandInitializer {
.withArgument("player", "Player name", MANDATORY)
.withArgument("pwd", "New password", MANDATORY)
.permission(AdminPermission.CHANGE_PASSWORD)
.sensitive(true)
.executableCommand(ChangePasswordAdminCommand.class)
.register();
@ -540,6 +545,7 @@ public class CommandInitializer {
.detailedDescription("Set a new password after successfully recovering your account.")
.withArgument("password", "New password", MANDATORY)
.permission(PlayerPermission.RECOVER_EMAIL)
.sensitive(true)
.executableCommand(EmailSetPasswordCommand.class)
.register();
@ -568,6 +574,7 @@ public class CommandInitializer {
.description("Command for logging in")
.detailedDescription("Processes the two-factor authentication code during login.")
.withArgument("code", "The TOTP code to use to log in", MANDATORY)
.sensitive(true)
.executableCommand(TotpCodeCommand.class)
.register();
@ -589,6 +596,7 @@ public class CommandInitializer {
.detailedDescription("Saves the generated TOTP secret after confirmation.")
.withArgument("code", "Code from the given secret from /totp add", MANDATORY)
.permission(PlayerPermission.ENABLE_TWO_FACTOR_AUTH)
.sensitive(true)
.executableCommand(ConfirmTotpCommand.class)
.register();
@ -600,6 +608,7 @@ public class CommandInitializer {
.detailedDescription("Disables two-factor authentication for your account.")
.withArgument("code", "Current 2FA code", MANDATORY)
.permission(PlayerPermission.DISABLE_TWO_FACTOR_AUTH)
.sensitive(true)
.executableCommand(RemoveTotpCommand.class)
.register();

View File

@ -7,6 +7,7 @@ import fr.xephi.authme.message.MessageKey;
import fr.xephi.authme.message.Messages;
import fr.xephi.authme.output.ConsoleFilter;
import fr.xephi.authme.output.Log4JFilter;
import fr.xephi.authme.output.LogFilterService;
import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.DatabaseSettings;
@ -36,6 +37,8 @@ public class OnStartupTasks {
private BukkitService bukkitService;
@Inject
private Messages messages;
@Inject
private LogFilterService logFilterService;
OnStartupTasks() {
}
@ -61,7 +64,7 @@ public class OnStartupTasks {
* @param settings the settings
* @param logger the plugin logger
*/
public static void setupConsoleFilter(Settings settings, Logger logger) {
public void setupConsoleFilter(Settings settings, Logger logger) {
// Try to set the log4j filter
try {
Class.forName("org.apache.logging.log4j.core.filter.AbstractFilter");
@ -69,7 +72,7 @@ public class OnStartupTasks {
} catch (ClassNotFoundException | NoClassDefFoundError e) {
// log4j is not available
ConsoleLogger.info("You're using Minecraft 1.6.x or older, Log4J support will be disabled");
ConsoleFilter filter = new ConsoleFilter();
ConsoleFilter filter = new ConsoleFilter(logFilterService);
logger.setFilter(filter);
Bukkit.getLogger().setFilter(filter);
Logger.getLogger("Minecraft").setFilter(filter);
@ -77,10 +80,10 @@ public class OnStartupTasks {
}
// Set the console filter to remove the passwords
private static void setLog4JFilter() {
private void setLog4JFilter() {
org.apache.logging.log4j.core.Logger logger;
logger = (org.apache.logging.log4j.core.Logger) LogManager.getRootLogger();
logger.addFilter(new Log4JFilter());
logger.addFilter(new Log4JFilter(logFilterService));
}
/**

View File

@ -10,13 +10,19 @@ import java.util.logging.LogRecord;
*/
public class ConsoleFilter implements Filter {
private LogFilterService filterService;
public ConsoleFilter(LogFilterService filterService) {
this.filterService = filterService;
}
@Override
public boolean isLoggable(LogRecord record) {
if (record == null || record.getMessage() == null) {
return true;
}
if (LogFilterHelper.isSensitiveAuthMeCommand(record.getMessage())) {
if (filterService.isSensitiveAuthMeCommand(record.getMessage())) {
String playerName = record.getMessage().split(" ")[0];
record.setMessage(playerName + " issued an AuthMe command");
}

View File

@ -16,6 +16,12 @@ public class Log4JFilter extends AbstractFilter {
private static final long serialVersionUID = -5594073755007974254L;
private LogFilterService filterService;
public Log4JFilter(LogFilterService filterService) {
this.filterService = filterService;
}
/**
* Validates a Message instance and returns the {@link Result} value
* depending on whether the message contains sensitive AuthMe data.
@ -24,7 +30,7 @@ public class Log4JFilter extends AbstractFilter {
*
* @return The Result value
*/
private static Result validateMessage(Message message) {
private Result validateMessage(Message message) {
if (message == null) {
return Result.NEUTRAL;
}
@ -39,8 +45,8 @@ public class Log4JFilter extends AbstractFilter {
*
* @return The Result value
*/
private static Result validateMessage(String message) {
return LogFilterHelper.isSensitiveAuthMeCommand(message)
private Result validateMessage(String message) {
return filterService.isSensitiveAuthMeCommand(message)
? Result.DENY
: Result.NEUTRAL;
}

View File

@ -1,50 +0,0 @@
package fr.xephi.authme.output;
import com.google.common.annotations.VisibleForTesting;
import fr.xephi.authme.util.StringUtils;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
/**
* Service class for the log filters.
*/
final class LogFilterHelper {
@VisibleForTesting
static final List<String> COMMANDS_TO_SKIP = withAndWithoutAuthMePrefix(
"/login ", "/l ", "/log ", "/register ", "/reg ", "/unregister ", "/unreg ",
"/changepassword ", "/cp ", "/changepass ", "/authme register ", "/authme reg ", "/authme r ",
"/authme changepassword ", "/authme password ", "/authme changepass ", "/authme cp ", "/email setpassword ");
private static final String ISSUED_COMMAND_TEXT = "issued server command:";
private LogFilterHelper() {
// Util class
}
/**
* Validate a message and return whether the message contains a sensitive AuthMe command.
*
* @param message The message to verify
*
* @return True if it is a sensitive AuthMe command, false otherwise
*/
static boolean isSensitiveAuthMeCommand(String message) {
if (message == null) {
return false;
}
String lowerMessage = message.toLowerCase();
return lowerMessage.contains(ISSUED_COMMAND_TEXT) && StringUtils.containsAny(lowerMessage, COMMANDS_TO_SKIP);
}
private static List<String> withAndWithoutAuthMePrefix(String... commands) {
List<String> commandList = new ArrayList<>(commands.length * 2);
for (String command : commands) {
commandList.add(command);
commandList.add(command.substring(0, 1) + "authme:" + command.substring(1));
}
return Collections.unmodifiableList(commandList);
}
}

View File

@ -0,0 +1,41 @@
package fr.xephi.authme.output;
import fr.xephi.authme.command.CommandMapper;
import fr.xephi.authme.command.FoundCommandResult;
import javax.inject.Inject;
import java.util.Arrays;
import java.util.List;
/**
* Service class for the log filters.
*/
public class LogFilterService {
@Inject
private CommandMapper commandMapper;
private static final String ISSUED_COMMAND_PREFIX_TEXT = "issued server command: /";
/**
* Validate a message and return whether the message contains a sensitive AuthMe command.
*
* @param message The message to verify
*
* @return True if it is a sensitive AuthMe command, false otherwise
*/
public boolean isSensitiveAuthMeCommand(String message) {
if (message == null || !message.contains(ISSUED_COMMAND_PREFIX_TEXT)) {
return false;
}
String rawCommand = message.substring(message.indexOf("/"));
List<String> components = Arrays.asList(rawCommand.split(" "));
FoundCommandResult command = commandMapper.mapPartsToCommand(null, components);
switch (command.getResultStatus()) {
case UNKNOWN_LABEL:
case MISSING_BASE_COMMAND:
return false;
}
return command.getCommandDescription().isSensitive();
}
}

View File

@ -13,14 +13,14 @@ import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertThat;
/**
* Test for {@link LogFilterHelper}.
* Test for {@link LogFilterService}.
*/
public class LogFilterHelperTest {
private static final List<CommandDescription> ALL_COMMANDS = new CommandInitializer().getCommands();
/**
* Checks that {@link LogFilterHelper#COMMANDS_TO_SKIP} contains the entries we expect
* Checks that {@link LogFilterService#COMMANDS_TO_SKIP} contains the entries we expect
* (commands with password argument).
*/
@Test
@ -39,7 +39,7 @@ public class LogFilterHelperTest {
.toArray(String[]::new);
// when / then
assertThat(LogFilterHelper.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries));
assertThat(LogFilterService.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries));
}