Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sensitive commands [WIP] #1705

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/fr/xephi/authme/AuthMe.java
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,8 @@ private void initialize() {
settings = injector.getSingleton(Settings.class);
ConsoleLoggerFactory.reloadSettings(settings);
logger = ConsoleLoggerFactory.get(AuthMe.class);
OnStartupTasks.setupConsoleFilter(getLogger());
OnStartupTasks onStartupTasks = injector.newInstance(OnStartupTasks.class);
onStartupTasks.setupConsoleFilter(getLogger());

// Set all service fields on the AuthMe class
instantiateServices(injector);
Expand All @@ -236,7 +237,6 @@ private void initialize() {
registerEventListeners(injector);

// Start Email recall task if needed
OnStartupTasks onStartupTasks = injector.newInstance(OnStartupTasks.class);
onStartupTasks.scheduleRecallEmailTask();
}

Expand Down
25 changes: 23 additions & 2 deletions src/main/java/fr/xephi/authme/command/CommandDescription.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -72,14 +76,16 @@ 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;
this.executableCommand = executableCommand;
this.parent = parent;
this.arguments = arguments;
this.permission = permission;
this.sensitive = sensitive;
}

/**
Expand Down Expand Up @@ -184,6 +190,10 @@ public PermissionNode getPermission() {
return permission;
}

public boolean isSensitive() {
return sensitive;
}

/**
* Return a builder instance to create a new command description.
*
Expand All @@ -204,6 +214,7 @@ public static final class CommandBuilder {
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.
Expand Down Expand Up @@ -232,7 +243,7 @@ public CommandDescription build() {
// 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) {
Expand Down Expand Up @@ -289,6 +300,16 @@ public CommandBuilder permission(PermissionNode permission) {
this.permission = permission;
return this;
}

/**
* Defines that the command contains sensitive data
*
* @return The builder
*/
public CommandBuilder sensitive() {
this.sensitive = true;
return this;
}
}

}
9 changes: 9 additions & 0 deletions src/main/java/fr/xephi/authme/command/CommandInitializer.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ private void buildCommands() {
.detailedDescription("Command to log in using AuthMeReloaded.")
.withArgument("password", "Login password", MANDATORY)
.permission(PlayerPermission.LOGIN)
.sensitive()
.executableCommand(LoginCommand.class)
.register();

Expand All @@ -118,6 +119,7 @@ private void buildCommands() {
.withArgument("password", "Password", OPTIONAL)
.withArgument("verifyPassword", "Verify password", OPTIONAL)
.permission(PlayerPermission.REGISTER)
.sensitive()
.executableCommand(RegisterCommand.class)
.register();

Expand All @@ -141,6 +143,7 @@ private void buildCommands() {
.withArgument("oldPassword", "Old password", MANDATORY)
.withArgument("newPassword", "New password", MANDATORY)
.permission(PlayerPermission.CHANGE_PASSWORD)
.sensitive()
.executableCommand(ChangePasswordCommand.class)
.register();

Expand Down Expand Up @@ -199,6 +202,7 @@ private CommandDescription buildAuthMeBaseCommand() {
.withArgument("player", "Player name", MANDATORY)
.withArgument("password", "Password", MANDATORY)
.permission(AdminPermission.REGISTER)
.sensitive()
.executableCommand(RegisterAdminCommand.class)
.register();

Expand Down Expand Up @@ -233,6 +237,7 @@ private CommandDescription buildAuthMeBaseCommand() {
.withArgument("player", "Player name", MANDATORY)
.withArgument("pwd", "New password", MANDATORY)
.permission(AdminPermission.CHANGE_PASSWORD)
.sensitive()
.executableCommand(ChangePasswordAdminCommand.class)
.register();

Expand Down Expand Up @@ -564,6 +569,7 @@ private CommandDescription buildEmailBaseCommand() {
.detailedDescription("Set a new password after successfully recovering your account.")
.withArgument("password", "New password", MANDATORY)
.permission(PlayerPermission.RECOVER_EMAIL)
.sensitive()
.executableCommand(EmailSetPasswordCommand.class)
.register();

Expand Down Expand Up @@ -592,6 +598,7 @@ private CommandDescription buildTotpBaseCommand() {
.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()
.executableCommand(TotpCodeCommand.class)
.register();

Expand All @@ -613,6 +620,7 @@ private CommandDescription buildTotpBaseCommand() {
.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()
.executableCommand(ConfirmTotpCommand.class)
.register();

Expand All @@ -624,6 +632,7 @@ private CommandDescription buildTotpBaseCommand() {
.detailedDescription("Disables two-factor authentication for your account.")
.withArgument("code", "Current 2FA code", MANDATORY)
.permission(PlayerPermission.DISABLE_TWO_FACTOR_AUTH)
.sensitive()
.executableCommand(RemoveTotpCommand.class)
.register();

Expand Down
11 changes: 7 additions & 4 deletions src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import fr.xephi.authme.message.Messages;
import fr.xephi.authme.output.ConsoleFilter;
import fr.xephi.authme.output.Log4JFilter;
import fr.xephi.authme.service.LogFilterService;
import fr.xephi.authme.service.BukkitService;
import fr.xephi.authme.settings.Settings;
import fr.xephi.authme.settings.properties.DatabaseSettings;
Expand Down Expand Up @@ -39,6 +40,8 @@ public class OnStartupTasks {
private BukkitService bukkitService;
@Inject
private Messages messages;
@Inject
private LogFilterService logFilterService;

OnStartupTasks() {
}
Expand All @@ -63,26 +66,26 @@ public static void sendMetrics(AuthMe plugin, Settings settings) {
*
* @param logger the plugin logger
*/
public static void setupConsoleFilter(Logger logger) {
public void setupConsoleFilter(Logger logger) {
// Try to set the log4j filter
try {
Class.forName("org.apache.logging.log4j.core.filter.AbstractFilter");
setLog4JFilter();
} 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);
}
}

// 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));
}

/**
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/fr/xephi/authme/output/ConsoleFilter.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package fr.xephi.authme.output;

import fr.xephi.authme.service.LogFilterService;

import java.util.logging.Filter;
import java.util.logging.LogRecord;

Expand All @@ -10,13 +12,19 @@
*/
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");
}
Expand Down
13 changes: 10 additions & 3 deletions src/main/java/fr/xephi/authme/output/Log4JFilter.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package fr.xephi.authme.output;

import fr.xephi.authme.service.LogFilterService;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Marker;
import org.apache.logging.log4j.core.LogEvent;
Expand All @@ -16,6 +17,12 @@ public class Log4JFilter extends AbstractFilter {

private static final long serialVersionUID = -5594073755007974254L;

private LogFilterService filterService;

public Log4JFilter(LogFilterService filterService) {
this.filterService = filterService;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that using the ConsoleFilter(LogFilterService) constructor is a bid dirty, any suggestion?

I don't really have a better suggestion. In theory we could put @Inject on the constructor and have a Log4JFilter instance injected into OnstartupTasks, but given the nature of how it is set up (in OnStartupTasks we have compatibility checks in case a certain log class doesn't exist) I wouldn't recommend doing that. I think your current solution works out nicely :)

}

/**
* Validates a Message instance and returns the {@link Result} value
* depending on whether the message contains sensitive AuthMe data.
Expand All @@ -24,7 +31,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;
}
Expand All @@ -39,8 +46,8 @@ private static Result validateMessage(Message message) {
*
* @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;
}
Expand Down
50 changes: 0 additions & 50 deletions src/main/java/fr/xephi/authme/output/LogFilterHelper.java

This file was deleted.

42 changes: 42 additions & 0 deletions src/main/java/fr/xephi/authme/service/LogFilterService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package fr.xephi.authme.service;

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 {

private static final String ISSUED_COMMAND_PREFIX_TEXT = "issued server command: /";

@Inject
private CommandMapper commandMapper;

/**
* 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;
default:
return command.getCommandDescription().isSensitive();
}
}
}
Loading