diff --git a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java index ca3c41838..b247af0c5 100644 --- a/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java +++ b/src/main/java/com/google/cloud/logging/logback/LoggingAppender.java @@ -23,6 +23,7 @@ import ch.qos.logback.core.UnsynchronizedAppenderBase; import ch.qos.logback.core.util.Loader; import com.google.api.core.InternalApi; +import com.google.api.core.ObsoleteApi; import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.MonitoredResource; import com.google.cloud.logging.Instrumentation; @@ -35,10 +36,12 @@ import com.google.cloud.logging.Payload; import com.google.cloud.logging.Severity; import com.google.cloud.logging.Synchronicity; +import com.google.common.base.Preconditions; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; -import java.io.FileInputStream; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; @@ -138,6 +141,7 @@ public class LoggingAppender extends UnsynchronizedAppenderBase { private String log; private String resourceType; private String credentialsFile; + private GoogleCredentials credentials; private String logDestinationProjectId; private boolean autoPopulateMetadata = true; private boolean redirectToStdout = false; @@ -185,17 +189,46 @@ public void setResourceType(String resourceType) { } /** - * Sets the path to the If you know that you will be loading credential configurations of a specific type, it is + * recommended to use a credential-type-specific `fromStream()` method. This will ensure that an + * unexpected credential type with potential for malicious intent is not loaded unintentionally. + * You might still have to do validation for certain credential types. Please follow the + * recommendation for that method. + * + *

If you are loading your credential configuration from an untrusted source and have not + * mitigated the risks (e.g. by validating the configuration yourself), make these changes as soon + * as possible to prevent security risks to your environment. + * + *

Regardless of the method used, it is always your responsibility to validate configurations + * received from external sources. + * + *

Sets the path to the credential * file. If not set the appender will use {@link GoogleCredentials#getApplicationDefault()} to * authenticate. * * @param credentialsFile the path to the credentials file. */ + @ObsoleteApi( + "This method is obsolete because of a potential security risk. Use the setCredentials() method instead") public void setCredentialsFile(String credentialsFile) { this.credentialsFile = credentialsFile; } + /** + * Sets the credential to use. If not set the appender will use {@link + * GoogleCredentials#getApplicationDefault()} to authenticate. + * + * @param credentials the GoogleCredentials to set + */ + public void setCredentials(GoogleCredentials credentials) { + Preconditions.checkNotNull(credentials, "Credentials cannot be null"); + this.credentials = credentials; + } + /** * Sets project ID to be used to customize log destination name for written log entries. * @@ -445,10 +478,12 @@ protected LoggingOptions getLoggingOptions() { if (loggingOptions == null) { LoggingOptions.Builder builder = LoggingOptions.newBuilder(); builder.setProjectId(logDestinationProjectId); - if (!Strings.isNullOrEmpty(credentialsFile)) { + if (credentials != null) { + builder.setCredentials(credentials); + } else if (!Strings.isNullOrEmpty(credentialsFile)) { try { builder.setCredentials( - GoogleCredentials.fromStream(new FileInputStream(credentialsFile))); + GoogleCredentials.fromStream(Files.newInputStream(Paths.get(credentialsFile)))); } catch (IOException e) { throw new RuntimeException( String.format( diff --git a/src/main/resources/META-INF/native-image/com.google.cloud/google-cloud-logging-logback/reflect-config.json b/src/main/resources/META-INF/native-image/com.google.cloud/google-cloud-logging-logback/reflect-config.json index 68b566d6e..9d249db03 100644 --- a/src/main/resources/META-INF/native-image/com.google.cloud/google-cloud-logging-logback/reflect-config.json +++ b/src/main/resources/META-INF/native-image/com.google.cloud/google-cloud-logging-logback/reflect-config.json @@ -35,6 +35,7 @@ {"name":"","parameterTypes":[] }, {"name":"setAutoPopulateMetadata","parameterTypes":["boolean"] }, {"name":"setCredentialsFile","parameterTypes":["java.lang.String"] }, + {"name":"setCredentials","parameterTypes":["com.google.auth.oauth2.GoogleCredentials"] }, {"name":"setFlushLevel","parameterTypes":["ch.qos.logback.classic.Level"] }, {"name":"setLog","parameterTypes":["java.lang.String"] }, {"name":"setLogDestinationProjectId","parameterTypes":["java.lang.String"] }, diff --git a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java index 9632cf330..027230a83 100644 --- a/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java +++ b/src/test/java/com/google/cloud/logging/logback/LoggingAppenderTest.java @@ -23,11 +23,13 @@ import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.filter.ThresholdFilter; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.classic.spi.LoggingEvent; +import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.MonitoredResource; import com.google.cloud.Timestamp; import com.google.cloud.logging.Instrumentation; @@ -60,7 +62,7 @@ public class LoggingAppenderTest { private static final String PROJECT_ID = "test-project"; private static final String CRED_FILE_PROJECT_ID = "project-12345"; - private static final String OVERRIDED_PROJECT_ID = "some-project-id"; + private static final String OVERRIDDEN_PROJECT_ID = "some-project-id"; private static final String DUMMY_CRED_FILE_PATH = "src/test/java/com/google/cloud/logging/logback/dummy-credentials.json"; private static final Payload.JsonPayload JSON_PAYLOAD = @@ -289,6 +291,22 @@ public void testMdcValuesAreConvertedToLabels() { assertThat(capturedArgument.getValue().iterator().next()).isEqualTo(INFO_ENTRY); } + @Test + public void testCreateLoggingOptionsWithValidCredentials() { + LoggingAppender appender = new LoggingAppender(); + appender.setCredentials(GoogleCredentials.newBuilder().build()); + // ServiceOptions requires a projectId to be set. Normally this is determined by the + // GoogleCredentials (Credential set above is a dummy value with no ProjectId). + appender.setLogDestinationProjectId(PROJECT_ID); + appender.getLoggingOptions(); + } + + @Test + public void testCreateLoggingOptionsWithNullCredentials() { + LoggingAppender appender = new LoggingAppender(); + assertThrows(NullPointerException.class, () -> appender.setCredentials(null)); + } + @Test(expected = RuntimeException.class) public void testCreateLoggingOptionsWithInvalidCredentials() { final String nonExistentFile = "/path/to/non/existent/file"; @@ -310,8 +328,8 @@ public void testCreateLoggingOptionsWithDestination() { // Try to build LoggingOptions with file based credentials. LoggingAppender appender = new LoggingAppender(); appender.setCredentialsFile(DUMMY_CRED_FILE_PATH); - appender.setLogDestinationProjectId(OVERRIDED_PROJECT_ID); - assertThat(appender.getLoggingOptions().getProjectId()).isEqualTo(OVERRIDED_PROJECT_ID); + appender.setLogDestinationProjectId(OVERRIDDEN_PROJECT_ID); + assertThat(appender.getLoggingOptions().getProjectId()).isEqualTo(OVERRIDDEN_PROJECT_ID); } private LoggingEvent createLoggingEvent(Level level, long timestamp) {