Skip to content

SNOW-1660409 JDBC Connector - GET file from stage weak file permissions #2066

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

Merged
merged 11 commits into from
Feb 12, 2025
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package net.snowflake.client.jdbc;

import static net.snowflake.client.core.Constants.NO_SPACE_LEFT_ON_DEVICE_ERR;
import static net.snowflake.client.jdbc.SnowflakeUtil.createOwnerOnlyPermissionDir;
import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty;

import com.fasterxml.jackson.core.JsonProcessingException;
Expand Down Expand Up @@ -1565,8 +1566,10 @@ public boolean execute() throws SQLException {
if (commandType == CommandType.DOWNLOAD) {
File dir = new File(localLocation);
if (!dir.exists()) {
boolean created = dir.mkdirs();

boolean created =
SnowflakeUtil.isWindows()
? dir.mkdirs()
: createOwnerOnlyPermissionDir(localLocation);
if (created) {
logger.debug("Directory created: {}", localLocation);
} else {
Expand Down Expand Up @@ -2507,7 +2510,7 @@ private static void pullFileFromRemoteStore(
RemoteStoreFileEncryptionMaterial encMat,
String presignedUrl,
String queryId)
throws SQLException {
throws SQLException, IOException {
remoteLocation remoteLocation = extractLocationAndPath(stage.getLocation());

String stageFilePath = filePath;
Expand Down
54 changes: 54 additions & 0 deletions src/main/java/net/snowflake/client/jdbc/SnowflakeUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,18 @@
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.google.common.base.Strings;
import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.sql.SQLException;
import java.sql.Time;
import java.sql.Types;
Expand All @@ -33,6 +39,7 @@
import java.util.Optional;
import java.util.Properties;
import java.util.Random;
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.Executors;
import java.util.concurrent.ThreadFactory;
Expand Down Expand Up @@ -66,6 +73,9 @@ public class SnowflakeUtil {
private static final SFLogger logger = SFLoggerFactory.getLogger(SnowflakeUtil.class);
private static final ObjectMapper OBJECT_MAPPER = ObjectMapperFactory.getObjectMapper();

public static final Set<PosixFilePermission> ownerOnlyPermission =
PosixFilePermissions.fromString("rw-------");

/** Additional data types not covered by standard JDBC */
public static final int EXTRA_TYPES_TIMESTAMP_LTZ = 50000;

Expand Down Expand Up @@ -902,6 +912,50 @@ public static Map<String, String> createCaseInsensitiveMap(Header[] headers) {
}
}

/** create a directory with Owner only permission (0600) */
@SnowflakeJdbcInternalApi
public static boolean createOwnerOnlyPermissionDir(String location) {
if (isWindows()) {
return false;
}

boolean isDirCreated = true;
Path dir = Paths.get(location);
try {
Files.createDirectory(dir, PosixFilePermissions.asFileAttribute(ownerOnlyPermission));
} catch (IOException e) {
logger.error(
"Failed to set OwnerOnly permission for {}. This may cause the file download to fail ",
location);
isDirCreated = false;
}
return isDirCreated;
}

@SnowflakeJdbcInternalApi
public static void assureOnlyUserAccessibleFilePermissions(File file) throws IOException {
if (isWindows()) {
return;
}
boolean disableUserPermissions =
file.setReadable(false, false)
&& file.setWritable(false, false)
&& file.setExecutable(false, false);
boolean setOwnerPermissionsOnly = file.setReadable(true, true) && file.setWritable(true, true);

if (disableUserPermissions && setOwnerPermissionsOnly) {
logger.info("Successfuly set OwnerOnly permission for {}. ", file.getAbsolutePath());
} else {
file.delete();
logger.error(
"Failed to set OwnerOnly permission for {}. Failed to download", file.getAbsolutePath());
throw new IOException(
String.format(
"Failed to set OwnerOnly permission for %s. Failed to download",
file.getAbsolutePath()));
}
}

/**
* Check whether the OS is Windows
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ public void download(
transferOptions.setConcurrentRequestCount(parallelism);

blob.downloadToFile(localFilePath, null, transferOptions, opContext);
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
stopwatch.stop();
long downloadMillis = stopwatch.elapsedMillis();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ public void download(
outStream.flush();
outStream.close();
bodyStream.close();
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
if (isEncrypting()) {
Map<String, String> userDefinedHeaders =
createCaseInsensitiveMap(response.getAllHeaders());
Expand Down Expand Up @@ -351,6 +352,7 @@ public void download(
logger.debug("Starting download without presigned URL", false);
blob.downloadTo(
localFile.toPath(), Blob.BlobSourceOption.shouldReturnRawInputStream(true));
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
stopwatch.stop();
downloadMillis = stopwatch.elapsedMillis();
logger.debug("Download successful", false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ public ExecutorService newExecutor() {
String iv = metaMap.get(AMZ_IV);

myDownload.waitForCompletion();

SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(localFile);
stopwatch.stop();
long downloadMillis = stopwatch.elapsedMillis();

Expand Down
59 changes: 59 additions & 0 deletions src/test/java/net/snowflake/client/jdbc/SnowflakeUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package net.snowflake.client.jdbc;

import static net.snowflake.client.jdbc.SnowflakeUtil.createCaseInsensitiveMap;
import static net.snowflake.client.jdbc.SnowflakeUtil.createOwnerOnlyPermissionDir;
import static net.snowflake.client.jdbc.SnowflakeUtil.extractColumnMetadata;
import static net.snowflake.client.jdbc.SnowflakeUtil.getSnowflakeType;
import static org.junit.jupiter.api.Assertions.assertEquals;
Expand All @@ -15,18 +16,29 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.sql.Types;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import net.snowflake.client.annotations.DontRunOnWindows;
import net.snowflake.client.category.TestTags;
import net.snowflake.client.core.ObjectMapperFactory;
import org.apache.http.Header;
import org.apache.http.message.BasicHeader;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;

@Tag(TestTags.CORE)
public class SnowflakeUtilTest extends BaseJDBCTest {
Expand Down Expand Up @@ -117,6 +129,53 @@ public void shouldConvertHeadersCreateCaseInsensitiveMap() {
assertEquals("value2", map.get("Key2"));
}

@Test
@DontRunOnWindows
public void testCreateOwnerOnlyPermissionDir(@TempDir Path tempDir)
throws IOException, UnsupportedOperationException {
String folderPath = tempDir.toAbsolutePath() + "\\folder-permission-testing";
boolean isDirCreated = createOwnerOnlyPermissionDir(folderPath);
assertTrue(isDirCreated);

Path tmp = Paths.get(folderPath);
try {
PosixFileAttributes attributes = Files.readAttributes(tmp, PosixFileAttributes.class);
Set<PosixFilePermission> permissions = attributes.permissions();
assertEquals(PosixFilePermissions.toString(permissions), "rw-------");
} finally {
File folder = new File(folderPath);
assertTrue(folder.exists());
assertTrue(folder.isDirectory());
assertTrue(folder.delete());
}
}

@Test
@DontRunOnWindows
public void testValidateFilePermission() throws IOException, UnsupportedOperationException {
String folderPath = "file-permission-testing";
String fileName = "fileTesting.txt";

File folder = new File(folderPath);
File file = new File(folderPath + "/" + fileName);
try {
boolean isFolderCreated = folder.mkdir();
boolean isFileCreated = file.createNewFile();
assertTrue(isFileCreated);
assertTrue(isFolderCreated);
SnowflakeUtil.assureOnlyUserAccessibleFilePermissions(file);

Path tmp = Paths.get(folderPath + "/" + fileName);
PosixFileAttributes attributes = Files.readAttributes(tmp, PosixFileAttributes.class);
Set<PosixFilePermission> permissions = attributes.permissions();
assertEquals(PosixFilePermissions.toString(permissions), "rw-------");

} finally {
assertTrue(file.delete());
assertTrue(folder.delete());
}
}

private static SnowflakeColumnMetadata createExpectedMetadata(
JsonNode rootNode, JsonNode fieldOne, JsonNode fieldTwo) throws SnowflakeSQLLoggedException {
ColumnTypeInfo columnTypeInfo =
Expand Down