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

Prevent errors linked to Security Manager when reading config #7846

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import datadog.remoteconfig.ConfigurationPoller;
import datadog.remoteconfig.DefaultConfigurationPoller;
import datadog.trace.api.Config;
import datadog.trace.util.SystemUtils;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
import okhttp3.HttpUrl;
Expand Down Expand Up @@ -98,7 +99,7 @@ public DDAgentFeaturesDiscovery featuresDiscovery(Config config) {
agentUrl,
config.isTraceAgentV05Enabled(),
config.isTracerMetricsEnabled());
if (!"true".equalsIgnoreCase(System.getProperty("dd.test.no.early.discovery"))) {
if (!"true".equalsIgnoreCase(SystemUtils.tryGetProperty("dd.test.no.early.discovery"))) {
featuresDiscovery.discover();
}
}
Expand Down
1 change: 1 addition & 0 deletions internal-api/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ excludedClassesCoverage += [
"datadog.trace.util.PidHelper.Fallback",
"datadog.trace.util.ProcessUtils",
"datadog.trace.util.PropagationUtils",
"datadog.trace.util.SystemUtils",
"datadog.trace.util.UnsafeUtils",
"datadog.trace.api.ConfigCollector",
"datadog.trace.api.Config.HostNameHolder",
Expand Down
36 changes: 25 additions & 11 deletions internal-api/src/main/java/datadog/trace/api/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import datadog.trace.context.TraceScope;
import datadog.trace.util.PidHelper;
import datadog.trace.util.Strings;
import datadog.trace.util.SystemUtils;
import datadog.trace.util.throwable.FatalAgentMisconfigurationError;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.BufferedReader;
Expand Down Expand Up @@ -549,7 +550,7 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
this.instrumenterConfig = instrumenterConfig;
configFileStatus = configProvider.getConfigFileStatus();
runtimeIdEnabled = configProvider.getBoolean(RUNTIME_ID_ENABLED, true);
runtimeVersion = System.getProperty("java.version", "unknown");
runtimeVersion = SystemUtils.getPropertyOrDefault("java.version", "unknown");

// Note: We do not want APiKey to be loaded from property for security reasons
// Note: we do not use defined default here
Expand Down Expand Up @@ -1799,6 +1800,19 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment())
"AppSec SCA is enabled but telemetry is disabled. AppSec SCA will not work.");
}

// check if the security manager is causing us trouble
if (!SystemUtils.canAccessEnvironmentVariables()) {
log.warn(
"The Java Security Manager is preventing the Datadog Tracer from accessing some environment variables. "
+ "Consider granting AllPermission to the dd-java-agent jar.");
}

if (!SystemUtils.canAccessSystemProperties()) {
log.warn(
"The Java Security Manager is preventing the Datadog Tracer from accessing some system properties. "
+ "Consider granting AllPermission to the dd-java-agent jar.");
}

log.debug("New instance: {}", this);
}

Expand Down Expand Up @@ -2396,8 +2410,8 @@ public static boolean isDatadogProfilerSafeInCurrentEnvironment() {
// don't want to put this logic (which will evolve) in the public ProfilingConfig, and can't
// access Platform there
if (!Platform.isJ9() && Platform.isJavaVersion(8)) {
String arch = System.getProperty("os.arch");
if ("aarch64".equalsIgnoreCase(arch) || "arm64".equalsIgnoreCase(arch)) {
String arch = SystemUtils.tryGetProperty("os.arch");
if (arch == null || "aarch64".equalsIgnoreCase(arch) || "arm64".equalsIgnoreCase(arch)) {
return false;
}
}
Expand Down Expand Up @@ -3305,12 +3319,12 @@ public CiVisibilityWellKnownTags getCiVisibilityWellKnownTags() {
getRuntimeId(),
getEnv(),
LANGUAGE_TAG_VALUE,
System.getProperty("java.runtime.name"),
System.getProperty("java.version"),
System.getProperty("java.vendor"),
System.getProperty("os.arch"),
System.getProperty("os.name"),
System.getProperty("os.version"));
SystemUtils.tryGetProperty("java.runtime.name"),
SystemUtils.tryGetProperty("java.version"),
SystemUtils.tryGetProperty("java.vendor"),
SystemUtils.tryGetProperty("os.arch"),
SystemUtils.tryGetProperty("os.name"),
SystemUtils.tryGetProperty("os.version"));
}

public String getPrimaryTag() {
Expand Down Expand Up @@ -3999,7 +4013,7 @@ private static boolean isWindowsOS() {
}

private static String getEnv(String name) {
String value = System.getenv(name);
String value = SystemUtils.tryGetEnv(name);
if (value != null) {
ConfigCollector.get().put(name, value, ConfigOrigin.ENV);
}
Expand All @@ -4022,7 +4036,7 @@ private static String getProp(String name) {
}

private static String getProp(String name, String def) {
String value = System.getProperty(name, def);
String value = SystemUtils.getPropertyOrDefault(name, def);
if (value != null) {
ConfigCollector.get().put(name, value, ConfigOrigin.JVM_PROP);
}
Expand Down
24 changes: 14 additions & 10 deletions internal-api/src/main/java/datadog/trace/api/Platform.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.api;

import datadog.trace.util.SystemUtils;
import java.lang.management.GarbageCollectorMXBean;
import java.lang.management.ManagementFactory;
import java.util.ArrayList;
Expand Down Expand Up @@ -43,7 +44,8 @@ static GC current() {
}
}

private static final Version JAVA_VERSION = parseJavaVersion(System.getProperty("java.version"));
private static final Version JAVA_VERSION =
parseJavaVersion(SystemUtils.getPropertyOrDefault("java.version", "0"));
private static final JvmRuntime RUNTIME = new JvmRuntime();

private static final GC GARBAGE_COLLECTOR = GC.current();
Expand Down Expand Up @@ -201,11 +203,11 @@ static final class JvmRuntime {

public JvmRuntime() {
this(
System.getProperty("java.version"),
System.getProperty("java.runtime.version"),
System.getProperty("java.runtime.name"),
System.getProperty("java.vm.vendor"),
System.getProperty("java.vendor.version"));
SystemUtils.tryGetProperty("java.version"),
SystemUtils.tryGetProperty("java.runtime.version"),
SystemUtils.tryGetProperty("java.runtime.name"),
SystemUtils.tryGetProperty("java.vm.vendor"),
SystemUtils.tryGetProperty("java.vendor.version"));
}

// Only visible for testing
Expand Down Expand Up @@ -293,17 +295,19 @@ public static boolean isJavaVersionBetween(
}

public static boolean isLinux() {
return System.getProperty("os.name").toLowerCase(Locale.ROOT).contains("linux");
return SystemUtils.getPropertyOrDefault("os.name", "")
.toLowerCase(Locale.ROOT)
.contains("linux");
}

public static boolean isWindows() {
// https://mkyong.com/java/how-to-detect-os-in-java-systemgetpropertyosname/
final String os = System.getProperty("os.name").toLowerCase(Locale.ROOT);
final String os = SystemUtils.getPropertyOrDefault("os.name", "").toLowerCase(Locale.ROOT);
return os.contains("win");
}

public static boolean isMac() {
final String os = System.getProperty("os.name").toLowerCase(Locale.ROOT);
final String os = SystemUtils.getPropertyOrDefault("os.name", "").toLowerCase(Locale.ROOT);
return os.contains("mac");
}

Expand All @@ -314,7 +318,7 @@ public static boolean isOracleJDK8() {
}

public static boolean isJ9() {
return System.getProperty("java.vm.name").contains("J9");
return SystemUtils.getPropertyOrDefault("java.vm.name", "").contains("J9");
}

public static boolean isIbm8() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.api.env;

import datadog.trace.api.config.GeneralConfig;
import datadog.trace.util.SystemUtils;
import de.thetaphi.forbiddenapis.SuppressForbidden;
import java.io.File;
import java.util.HashMap;
Expand Down Expand Up @@ -40,8 +41,8 @@ static void useFixedEnv(final Map<String, String> props) {
* autodetection will return either the JAR filename or the java main class.
*/
private String autodetectServiceName() {
String inAas = System.getenv("DD_AZURE_APP_SERVICES");
String siteName = System.getenv("WEBSITE_SITE_NAME");
String inAas = SystemUtils.tryGetEnv("DD_AZURE_APP_SERVICES");
String siteName = SystemUtils.tryGetEnv("WEBSITE_SITE_NAME");

if (("true".equalsIgnoreCase(inAas) || "1".equals(inAas)) && siteName != null) {
return siteName;
Expand All @@ -50,7 +51,7 @@ private String autodetectServiceName() {
// Besides "sun.java.command" property is not an standard, all main JDKs has set this property.
// Tested on:
// - OracleJDK, OpenJDK, AdoptOpenJDK, IBM JDK, Azul Zulu JDK, Amazon Coretto JDK
return extractJarOrClass(System.getProperty("sun.java.command"));
return extractJarOrClass(SystemUtils.tryGetProperty("sun.java.command"));
}

@SuppressForbidden
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.bootstrap.config.provider;

import datadog.trace.util.Strings;
import datadog.trace.util.SystemUtils;
import java.util.Map;

public class AgentArgsInjector {
Expand All @@ -21,14 +22,14 @@ public static void injectAgentArgsConfig(Map<String, String> args) {
}
for (Map.Entry<String, String> e : args.entrySet()) {
String propertyName = e.getKey();
String existingPropertyValue = System.getProperty(propertyName);
String existingPropertyValue = SystemUtils.tryGetProperty(propertyName);
if (existingPropertyValue != null) {
// system properties should have higher priority than agent arguments
continue;
}

String envVarName = Strings.toEnvVar(propertyName);
String envVarValue = System.getenv(envVarName);
String envVarValue = SystemUtils.tryGetEnv(envVarName);
if (envVarValue != null) {
// env variables should have higher priority than agent arguments
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
import static datadog.trace.util.Strings.propertyNameToEnvironmentVariableName;

import datadog.trace.api.ConfigOrigin;
import datadog.trace.util.SystemUtils;

final class EnvironmentConfigSource extends ConfigProvider.Source {

@Override
protected String get(String key) {
return System.getenv(propertyNameToEnvironmentVariableName(key));
return SystemUtils.tryGetEnv(propertyNameToEnvironmentVariableName(key));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import datadog.trace.api.TracePropagationStyle;
import datadog.trace.api.telemetry.OtelEnvMetricCollector;
import datadog.trace.util.Strings;
import datadog.trace.util.SystemUtils;
import java.io.BufferedInputStream;
import java.io.File;
import java.io.FileInputStream;
Expand Down Expand Up @@ -186,9 +187,9 @@ private String getDatadogProperty(String sysProp) {
* <p>Checks system properties and environment variables.
*/
private static String getProperty(String sysProp) {
String value = System.getProperty(sysProp);
String value = SystemUtils.tryGetProperty(sysProp);
if (null == value) {
value = System.getenv(Strings.toEnvVar(sysProp));
value = SystemUtils.tryGetEnv(Strings.toEnvVar(sysProp));
}
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
import static datadog.trace.util.Strings.propertyNameToSystemPropertyName;

import datadog.trace.api.ConfigOrigin;
import datadog.trace.util.SystemUtils;

public final class SystemPropertiesConfigSource extends ConfigProvider.Source {

@Override
protected String get(String key) {
return System.getProperty(propertyNameToSystemPropertyName(key));
return SystemUtils.tryGetProperty(propertyNameToSystemPropertyName(key));
}

@Override
Expand Down
54 changes: 54 additions & 0 deletions internal-api/src/main/java/datadog/trace/util/SystemUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package datadog.trace.util;

public final class SystemUtils {

private SystemUtils() {}

public static String tryGetEnv(String envVar) {
return getEnvOrDefault(envVar, null);
}

public static String getEnvOrDefault(String envVar, String defaultValue) {
try {
return System.getenv(envVar);
} catch (SecurityException e) {
return defaultValue;
}
}

public static String tryGetProperty(String property) {
try {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could just be getPropertyOrDefault(property, null)?

return System.getProperty(property);
} catch (SecurityException e) {
return null;
}
}

public static String getPropertyOrDefault(String property, String defaultValue) {
try {
return System.getProperty(property, defaultValue);
} catch (SecurityException e) {
return defaultValue;
}
}

public static boolean canAccessSystemProperties() {
try {
// try to access a common system property and see what happens
System.getProperty("os.name");
} catch (SecurityException e) {
return false;
}
return true;
}

public static boolean canAccessEnvironmentVariables() {
try {
// try to access a common env var and see what happens
System.getenv("DD_ENV");
} catch (SecurityException e) {
return false;
}
return true;
}
}