diff --git a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp index a41515edfbb..58d6c029bc1 100644 --- a/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp +++ b/src/hotspot/share/jfr/dcmd/jfrDcmds.cpp @@ -486,8 +486,9 @@ void JfrConfigureFlightRecorderDCmd::print_help(outputStream* out, bool startup) out->print_cr(" The option redact-argument is best-effort and applies only to"); out->print_cr(" command-line arguments in the jdk.JVMInformation event and to"); out->print_cr(" the java.command system property in the jdk.InitialSystemProperty"); - out->print_cr(" event. Other events, such as jdk.ProcessStart (child processes),"); - out->print_cr(" are not redacted."); + out->print_cr(" event, and to matching command-line argument text in the values"); + out->print_cr(" of jdk.InitialEnvironmentVariable events. Other events, such as"); + out->print_cr(" jdk.ProcessStart (child processes), are not redacted."); out->print_cr(""); out->print_cr(" If the redact-argument option is not specified, the following"); out->print_cr(" filters are used by default:"); diff --git a/src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp b/src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp index 331c28cffa2..652320c9904 100644 --- a/src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp +++ b/src/hotspot/share/jfr/periodic/jfrRedactedEvents.cpp @@ -29,6 +29,7 @@ #include "logging/logMessage.hpp" #include "runtime/arguments.hpp" #include "runtime/flags/jvmFlag.hpp" +#include "runtime/javaThread.hpp" #include "runtime/os.hpp" #include "runtime/vm_version.hpp" #include "services/diagnosticArgument.hpp" @@ -46,17 +47,20 @@ using StringFlag = JfrRedactedEvents::StringFlag; using StringKeyValueArray = GrowableArray*; static const char REDACTED[] = "[REDACTED]"; +static const char REDACTED_MARKER = (char)0xFF; static const char DELIMITER[] = " "; +static const char REDACT_ARGUMENT[] = "redact-argument"; static const char REDACT_ARGUMENT_EQUAL[] = "redact-argument="; static const size_t REDACTED_LENGTH = sizeof(REDACTED) -1; static const size_t DELIMITER_LENGTH = sizeof(DELIMITER) -1; -static const size_t REDACT_ARGUMENT_EQUAL_LENGTH = sizeof(REDACT_ARGUMENT_EQUAL) -1; +static const size_t REDACT_ARGUMENT_LENGTH = sizeof(REDACT_ARGUMENT) -1; String* JfrRedactedEvents::_redacted_java_command_line = nullptr; String* JfrRedactedEvents::_redacted_jvm_command_line = nullptr; String* JfrRedactedEvents::_redacted_flags_command_line = nullptr; String* JfrRedactedEvents::_redacted_flight_recorder_options = nullptr; +String* JfrRedactedEvents::_redacted_flight_recorder_options_with_marker = nullptr; StringKeyValueArray JfrRedactedEvents::_initial_environment_variables = nullptr; StringKeyValueArray JfrRedactedEvents::_initial_system_properties = nullptr; @@ -71,6 +75,10 @@ bool JfrRedactedEvents::_initialized = false; bool JfrRedactedEvents::set_argument_filter(const char* filters) { assert (_argument_filters == nullptr, "invariant"); assert (filters != nullptr, "invariant"); + if (strcmp(filters, "*") != 0 && strcmp(filters, "none") != 0) { + _redacted_arguments = new StringArray(); + _redacted_arguments->add(filters); + } _argument_filters = new StringArray(); return append_filters(_argument_filters, true, filters); } @@ -139,9 +147,8 @@ bool JfrRedactedEvents::append_filters(StringArray* target, bool argument, const } if (filters[0] == '\0') { LogMessage(jfr, redact) msg; - msg.warning("Default redaction filters are replaced. Specify:"); - msg.warning("-XX:FlightRecorderOptions:%s=none to disable filters without a warning.", option_name); - return true; + msg.error("Specify -XX:FlightRecorderOptions:%s=none to disable filters completely.", option_name); + return false; } if (strcmp(filters, "none") == 0) { return true; @@ -187,6 +194,67 @@ char* JfrRedactedEvents::new_redacted_text() { return result; } +void JfrRedactedEvents::redact(String* scratch_string, const char* target, const String* redaction) { + if (strchr(redaction->text(), REDACTED_MARKER)) { + return; + } + const char* position = target; + while (true) { + const char* sensitive = strstr(position, redaction->text()); + if (sensitive == nullptr) { + return; + } + size_t index = (size_t)(sensitive - target); + for (size_t i = 0; i < redaction->length(); i++) { + scratch_string->set(index + i, REDACTED_MARKER); + } + position = sensitive + 1; + } +} + +String* JfrRedactedEvents::redact_environment_variable_value(const char* value) { + if (strchr(value, REDACTED_MARKER)) { + return new String(REDACTED); + } + bool changed = false; + String* input = new String(value); + if (_redacted_flight_recorder_options_with_marker != nullptr) { + size_t length = strlen(FlightRecorderOptions); + while (const char* start = strstr(input->text(), FlightRecorderOptions)) { + changed = true; + const char* end = start + length; + stringStream s; + s.write(input->text(), start - input->text()); + s.write(_redacted_flight_recorder_options_with_marker->text(), _redacted_flight_recorder_options_with_marker->length()); + s.write(end, strlen(end)); + String* result = new String(s.base()); + delete input; + input = result; + } + } + String* scratch_string = new String(input->text()); + for (int i = 0; i < _redacted_arguments->length(); i++) { + redact(scratch_string, input->text(), _redacted_arguments->at(i)); + } + stringStream result; + bool inside_redaction = false; + for (size_t i = 0; i < scratch_string->length(); i++) { + if (scratch_string->at(i) == REDACTED_MARKER) { + changed = true; + if (!inside_redaction) { + result.print(REDACTED); + } + inside_redaction = true; + } else { + result.put(scratch_string->at(i)); + inside_redaction = false; + } + } + delete scratch_string; + delete input; + return changed ? new String(result.base()) : nullptr; +} + bool JfrRedactedEvents::emit_initial_environment_variables(bool log) { if (_initial_environment_variables == nullptr) { ensure_initialized(); @@ -207,6 +275,16 @@ bool JfrRedactedEvents::emit_initial_environment_variables(bool log) { if (log) { log_debug(jfr, redact)("Redacted initial environment variable named '%s'", key->text()); } + } else { + String* redacted_value = redact_environment_variable_value(value); + if (redacted_value != nullptr) { + if (log) { + log_debug(jfr, redact)("Redacted argument in initial environment variable value named '%s'", key->text()); + } + _initial_environment_variables->append(new StringKeyValue(key, redacted_value->text())); + delete redacted_value; + continue; + } } _initial_environment_variables->append(new StringKeyValue(key, value)); } @@ -262,6 +340,9 @@ bool JfrRedactedEvents::match_flag(const char* flag_name, const char* arg) { if (flag_name == nullptr || arg == nullptr) { return false; } + if (strncmp(arg, "-XX:", 4) == 0) { + arg += 4; + } while (*flag_name) { if (*arg != *flag_name) { return false; @@ -346,6 +427,34 @@ void JfrRedactedEvents::emit_jvm_information(bool log) { } } +// Method assumes that FlightRecorderOptions has been successfully parsed during startup +String* JfrRedactedEvents::redact_flight_recorder_options(const char* option, bool marker) { + JavaThread* THREAD = JavaThread::current(); + const size_t length = strlen(option); + DCmdArgIter iterator(option, length, ','); + while (iterator.next(THREAD)) { + if (strncmp(iterator.key_addr(), REDACT_ARGUMENT, REDACT_ARGUMENT_LENGTH) == 0) { + const char* start = iterator.value_addr(); + const char* end = start + iterator.value_length(); + stringStream result; + result.write(option, start - option); + if (marker) { + result.put(REDACTED_MARKER); + } else { + result.write(REDACTED, REDACTED_LENGTH); + } + result.write(end, option + length - end); + return new String(result.base()); + } + } + if (HAS_PENDING_EXCEPTION) { + DEBUG_ONLY(ShouldNotReachHere();) + CLEAR_PENDING_EXCEPTION; + return new String(REDACTED); + } + return nullptr; +} + void JfrRedactedEvents::ensure_initialized() { if (_initialized) { return; @@ -359,31 +468,12 @@ void JfrRedactedEvents::ensure_initialized() { add_default_filters(_argument_filters, true); } if (FlightRecorderOptions != nullptr) { - if (strstr(FlightRecorderOptions, REDACT_ARGUMENT_EQUAL) != nullptr) { - DCmdIter iterator(FlightRecorderOptions, ','); - stringStream result; - size_t pos = 0; - while(iterator.has_next()) { - CmdLine line = iterator.next(); - const char* start = line.cmd_addr(); - if (strncmp(start, REDACT_ARGUMENT_EQUAL, REDACT_ARGUMENT_EQUAL_LENGTH) == 0) { - result.print(REDACT_ARGUMENT_EQUAL); - result.print(REDACTED); - // Preserve ',' if there are more tokens - pos = iterator.has_next() ? iterator.cursor() - 1 : iterator.cursor(); - } - while (pos < iterator.cursor()) { - result.write(FlightRecorderOptions + pos, 1); - pos++; - } - } - _redacted_flight_recorder_options = new String(result.base()); - } else { - _redacted_flight_recorder_options = new String(FlightRecorderOptions); - } + _redacted_flight_recorder_options = redact_flight_recorder_options(FlightRecorderOptions, false); + _redacted_flight_recorder_options_with_marker = redact_flight_recorder_options(FlightRecorderOptions, true); + } + if (_redacted_arguments == nullptr) { + _redacted_arguments = new StringArray(); } - - _redacted_arguments = new StringArray(); StringArray* java_args = make_java_args_array(); _redacted_java_command_line = redact_command_line(java_args); @@ -396,7 +486,6 @@ void JfrRedactedEvents::ensure_initialized() { StringArray* flags_args = make_jvm_args_array(Arguments::jvm_flags_array(), Arguments::num_jvm_flags()); _redacted_flags_command_line = redact_command_line(flags_args); delete flags_args; - _initialized = true; } @@ -422,8 +511,8 @@ String* JfrRedactedEvents::redact_command_line(StringArray* arguments) { for (int j = arg_index; j < next_index; j++) { result->add(REDACTED); const char* arg = arguments->at(j)->text(); - if (arg != nullptr && strncmp(arg, "-XX:", 4) == 0) { - _redacted_arguments->add(arg + 4); + if (arg != nullptr) { + _redacted_arguments->add(arg); } } arg_index = next_index; @@ -504,15 +593,23 @@ StringArray* JfrRedactedEvents::make_jvm_args_array(char** jvm_args_array, int a return nullptr; } StringArray* result = new StringArray(array_length); - for(int i = 0; i < array_length; i++) { + for (int i = 0; i < array_length; i++) { char* argument = jvm_args_array[i]; - if (_redacted_flight_recorder_options != nullptr && - strncmp(argument, "-XX:FlightRecorderOptions", 25) == 0) { - const char* text = _redacted_flight_recorder_options->text(); - size_t length = _redacted_flight_recorder_options->length(); - // Length must be at least 26 or the JVM will not start. - result->add(new String(argument, 26, text, length)); - continue; + if (strncmp(argument, "-XX:FlightRecorderOptions", 25) == 0) { + size_t length = strlen(argument); + if (length > 25 && + _redacted_flight_recorder_options != nullptr && + strcmp(argument + 26, FlightRecorderOptions) == 0) { + const char* text = _redacted_flight_recorder_options->text(); + // Length must be at least 26 or the JVM will not start. + result->add(new String(argument, 26, text, _redacted_flight_recorder_options->length())); + continue; + } + if (strstr(argument, REDACT_ARGUMENT_EQUAL) != nullptr) { + _redacted_arguments->add(argument); + result->add("-XX:FlightRecorderOptions:[REDACTED]"); + continue; + } } if (strncmp(argument, "-D", 2) == 0) { const char* key_start = argument + 2; @@ -523,6 +620,7 @@ StringArray* JfrRedactedEvents::make_jvm_args_array(char** jvm_args_array, int a bool redact = match_key(_key_filters, key_tmp->text()); delete key_tmp; if (redact) { + _redacted_arguments->add(argument); size_t unsensitive_length = (size_t)(eq - argument) + 1; result->add(new String(argument, unsensitive_length, REDACTED, REDACTED_LENGTH)); continue; diff --git a/src/hotspot/share/jfr/periodic/jfrRedactedEvents.hpp b/src/hotspot/share/jfr/periodic/jfrRedactedEvents.hpp index dc972190b6c..c3d6fd32cbc 100644 --- a/src/hotspot/share/jfr/periodic/jfrRedactedEvents.hpp +++ b/src/hotspot/share/jfr/periodic/jfrRedactedEvents.hpp @@ -195,6 +195,7 @@ class JfrRedactedEvents: public AllStatic { static String* _redacted_jvm_command_line; static String* _redacted_flags_command_line; static String* _redacted_flight_recorder_options; + static String* _redacted_flight_recorder_options_with_marker; static GrowableArray* _initial_system_properties; static GrowableArray* _initial_environment_variables; static GrowableArray* _string_flags; @@ -217,7 +218,10 @@ class JfrRedactedEvents: public AllStatic { static int match_arguments(StringArray* filter_array, StringArray* arguments, int arg_index); static bool match_key(StringArray* array, const char* text); static bool read_file(StringArray* target, const char* filename); + static void redact(String* scratch_string, const char* target, const String* redaction); + static String* redact_flight_recorder_options(const char* option, bool marker); static String* redact_command_line(StringArray* arguments); + static String* redact_environment_variable_value(const char* value); static StringArray* split(const char* text, char separator); }; diff --git a/src/java.base/share/man/java.md b/src/java.base/share/man/java.md index 89166ae39e1..a0717055864 100644 --- a/src/java.base/share/man/java.md +++ b/src/java.base/share/man/java.md @@ -1215,9 +1215,11 @@ These `java` options control the runtime behavior of the Java HotSpot VM. be replaced with `[REDACTED]`. The option `redact-argument` is best-effort and applies only to command-line arguments in the `jdk.JVMInformation` event and to the `java.command` system property in the - `jdk.InitialSystemProperty` event. Other events, such as `jdk.ProcessStart` - (child processes), are not redacted. Use `-XX:FlightRecorderOptions:help` - to see the default filters used by the `redact-argument` option. + `jdk.InitialSystemProperty` event, and to matching command-line argument + text in the values of `jdk.InitialEnvironmentVariable` events. Other + events, such as `jdk.ProcessStart` (child processes), are not redacted. + Use `-XX:FlightRecorderOptions:help` to see the default filters used by + the `redact-argument` option. `redact-key=`key-filter : Replace the value of environment variables and system properties diff --git a/test/jdk/jdk/jfr/startupargs/TestRedact.java b/test/jdk/jdk/jfr/startupargs/TestRedact.java index 2d96408a3f5..f3f3a9e5fa6 100644 --- a/test/jdk/jdk/jfr/startupargs/TestRedact.java +++ b/test/jdk/jdk/jfr/startupargs/TestRedact.java @@ -42,6 +42,7 @@ import jdk.jfr.consumer.EventStream; import jdk.jfr.consumer.RecordingFile; import jdk.test.lib.Asserts; import jdk.test.lib.jfr.CommonHelper; +import jdk.test.lib.Platform; import jdk.test.lib.process.OutputAnalyzer; import jdk.test.lib.process.ProcessTools; @@ -169,6 +170,7 @@ public class TestRedact { testRedactKey(); testRedactArgument(); testRedactMultiple(); + testOptionVariable(); testWildcards(); testDefaults(); testRedactFile(); @@ -315,15 +317,6 @@ public class TestRedact { private static void testEmpty() throws Exception { var environment = Map.of("API_TOKEN", "Zebra1"); var properties = Map.of("API_KEY", "Zebra2"); - Execution e1 = run(environment, properties, - "-XX:FlightRecorderOptions:redact-key=,redact-argument=", "Zebra3" - ); - e1.output().shouldContain("Default redaction filters are replaced."); - e1.output().shouldContain("redact-key=none to disable filters without a warning"); - e1.output().shouldContain("redact-argument=none to disable filters without a warning"); - e1.assertUnredacted("Zebra1"); - e1.assertUnredacted("Zebra2"); - e1.assertUnredacted("Zebra3"); Execution e2 = run(environment, properties, "-XX:FlightRecorderOptions:redact-argument=none,redact-key=none", "Zebra3" @@ -377,6 +370,12 @@ public class TestRedact { e.assertRedactedArgument("N4711"); e.assertRedactedArgument("Smith:abc123"); e.assertUnredacted("Banana"); + + String option = Platform.isWindows() ? + "-XX:FlightRecorderOptions:redact-argument='Foo,bar'" : + "-XX:FlightRecorderOptions:redact-argument=\"Foo,bar\""; + Execution e2 = run(option,"Foo,bar"); + e2.assertRedactedArgument("Foo,bar"); } private static void testRedactMultiple() throws Exception { @@ -390,6 +389,69 @@ public class TestRedact { e.assertRedactedArgument("Quz"); } + private static void testOptionVariable() throws Exception { + // Simulate shell expansion with the three options: + // SYSTEM_PROPS, JVM_OPTIONS and PROGRAM_OPTIONS + String systemProperty = "-Dsecret=apple"; + String jvmOption = "-XX:FlightRecorderOptions:stackdepth=32,redact-argument=+Aracuan"; + String programOption = "Aracuan"; + Execution e1 = run( + Map.of("SYSTEM_PROPS", systemProperty, + "JVM_OPTIONS", jvmOption, + "PROGRAM_OPTIONS", programOption), + Map.of("secret","apple"), + List.of(systemProperty, jvmOption), + programOption + ); + e1.assertRedactedKey("SYSTEM_PROPS"); + String redactedJVMOption = e1.environment.get("JVM_OPTIONS"); + if (!redactedJVMOption.equals("-XX:FlightRecorderOptions:stackdepth=32,redact-argument=[REDACTED]")) { + throw new Exception("Expected partial redaction for environment variable with -XX:FlightRecorderOptions:redact-argument="); + } + e1.assertRedactedKey("PROGRAM_OPTIONS"); + e1.assertRedactedKey("secret"); + e1.assertRedactedArgument("Aracuan"); + + Execution e2 = run( + Map.of("PROGRAM_OPTIONS", "BLUE RED GREEN GREDELINE"), + Map.of(), + "-XX:FlightRecorderOptions:redact-argument=+*red*", + "BLUE", "RED", "GREEN", "GREDELINE" + ); + String programOptions = e2.environment().get("PROGRAM_OPTIONS"); + if (!programOptions.equals("BLUE [REDACTED] GREEN [REDACTED]")) { + e2.print(); + throw new Exception("Missing redaction inside option variable"); + } + + Execution e3 = run( + Map.of("PROGRAM_OPTIONS", "ZEBRA FISH ZEBRACCOON FISH RACCOON"), + Map.of(), + "-XX:FlightRecorderOptions:redact-argument=+zebra;raccoon", + "ZEBRA", "FISH", "ZEBRACCOON", "FISH", "RACCOON" + ); + programOptions = e3.environment().get("PROGRAM_OPTIONS"); + if (!programOptions.equals("[REDACTED] FISH [REDACTED] FISH [REDACTED]")) { + e3.print(); + throw new Exception("Incorrect redaction when option arguments overlap"); + } + + String option1 = "-XX:FlightRecorderOptions:redact-argument=Zebra,gibberish=,,,"; + String option2 = "-XX:FlightRecorderOptions:redact-argument=Tiger"; + Execution e4 = run( + Map.of("MY_JVM_OPTIONS", option1 + " " + option2), + Map.of(), + List.of(option1, option2), + "TIGER" + ); + e4.assertRedactedArgument("TIGER"); + String redacted = e4.environment().get("MY_JVM_OPTIONS"); + if (!redacted.equals("[REDACTED] -XX:FlightRecorderOptions:redact-argument=[REDACTED]")) { + e4.print(); + throw new Exception("Incorrect redaction with multiple options in environment variables"); + } + } + private static void testRedactKey() throws Exception { Execution e = run( Map.of("cart", "wheel", "banana", "split", "rose", "bud"), @@ -407,14 +469,20 @@ public class TestRedact { return run(Map.of(), Map.of(), options, args); } - private static Execution run(Map environment, Map properties, String options, String... args) throws Exception { + private static Execution run(Map environment, Map properties, String option, String... args) throws Exception { + return run(environment, properties, List.of(option), args); + } + + private static Execution run(Map environment, Map properties, List options, String... args) throws Exception { List arguments = new ArrayList<>(); Path file = Path.of("file.jfr"); for (var entry : properties.entrySet()) { arguments.add("-D" + entry.getKey() + "=" + entry.getValue()); } arguments.add("-XX:StartFlightRecording:filename=" + file.toAbsolutePath().toString()); - arguments.add(options); + for (String option : options) { + arguments.add(option); + } arguments.add("jdk.jfr.startupargs.Application"); arguments.addAll(Arrays.asList(args));