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

SNOW-1653464: Creating .cache directory and cache files permissions #2052

Merged
merged 1 commit into from
Jan 29, 2025
Merged
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package net.snowflake.client.config;

import static net.snowflake.client.jdbc.SnowflakeUtil.convertSystemGetEnvToBooleanValue;
import static net.snowflake.client.jdbc.SnowflakeUtil.isWindows;
import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetEnv;

import com.fasterxml.jackson.dataformat.toml.TomlMapper;
Expand All @@ -18,7 +19,6 @@
import java.util.Map;
import java.util.Optional;
import java.util.Properties;
import net.snowflake.client.core.Constants;
import net.snowflake.client.core.SnowflakeJdbcInternalApi;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.log.SFLogger;
Expand Down Expand Up @@ -117,7 +117,7 @@ private static Map<String, Map> readParametersMap(Path configFilePath)

private static void verifyFilePermissionSecure(Path configFilePath)
throws IOException, SnowflakeSQLException {
if (Constants.getOS() != Constants.OS.WINDOWS) {
if (!isWindows()) {
PosixFileAttributeView posixFileAttributeView =
Files.getFileAttributeView(configFilePath, PosixFileAttributeView.class);
if (!posixFileAttributeView.readAttributes().permissions().stream()
Expand Down
61 changes: 50 additions & 11 deletions src/main/java/net/snowflake/client/core/FileCacheManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package net.snowflake.client.core;

import static net.snowflake.client.jdbc.SnowflakeUtil.isWindows;
import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetEnv;
import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty;

Expand All @@ -23,7 +24,11 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.attribute.BasicFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.Date;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import net.snowflake.client.log.SFLogger;
import net.snowflake.client.log.SFLoggerFactory;

Expand All @@ -46,6 +51,8 @@ class FileCacheManager {

private File cacheDir;

private boolean onlyOwnerPermissions = true;

private FileCacheManager() {}

static FileCacheManager builder() {
Expand Down Expand Up @@ -78,16 +85,29 @@ FileCacheManager setCacheFileLockExpirationInSeconds(long cacheFileLockExpiratio
return this;
}

FileCacheManager setOnlyOwnerPermissions(boolean onlyOwnerPermissions) {
this.onlyOwnerPermissions = onlyOwnerPermissions;
return this;
}

/**
* Override the cache file.
*
* @param newCacheFile a file object to override the default one.
*/
void overrideCacheFile(File newCacheFile) {
if (!newCacheFile.exists()) {
logger.debug("Cache file doesn't exists. File: {}", newCacheFile);
}
if (onlyOwnerPermissions) {
FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(
newCacheFile, "Override cache file");
} else {
FileUtil.logFileUsage(cacheFile, "Override cache file", false);
}
this.cacheFile = newCacheFile;
this.cacheDir = newCacheFile.getParentFile();
this.baseCacheFileName = newCacheFile.getName();
FileUtil.logFileUsage(cacheFile, "Override cache file", true);
}

FileCacheManager build() {
Expand Down Expand Up @@ -116,15 +136,12 @@ FileCacheManager build() {
} else {
// use user home directory to store the cache file
String homeDir = systemGetProperty("user.home");
if (homeDir == null) {
// use tmp dir if not exists.
homeDir = systemGetProperty("java.io.tmpdir");
} else {
if (homeDir != null) {
// Checking if home directory is writable.
File homeFile = new File(homeDir);
if (!homeFile.canWrite()) {
logger.debug("Home directory not writeable, using tmpdir", false);
homeDir = systemGetProperty("java.io.tmpdir");
logger.debug("Home directory not writeable, skip using cache", false);
homeDir = null;
}
}
if (homeDir == null) {
Expand Down Expand Up @@ -155,7 +172,16 @@ FileCacheManager build() {
// If exists. the method returns false.
// In this particular case, it doesn't matter as long as the file is
// writable.
if (cacheFileTmp.createNewFile()) {
if (!cacheFileTmp.exists()) {
if (!isWindows() && onlyOwnerPermissions) {
Files.createFile(
cacheFileTmp.toPath(),
PosixFilePermissions.asFileAttribute(
Stream.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE)
.collect(Collectors.toSet())));
} else {
Files.createFile(cacheFileTmp.toPath());
}
logger.debug("Successfully created a cache file {}", cacheFileTmp);
} else {
logger.debug("Cache file already exists {}", cacheFileTmp);
Expand All @@ -165,7 +191,10 @@ FileCacheManager build() {
this.cacheLockFile =
new File(this.cacheFile.getParentFile(), this.baseCacheFileName + ".lck");
} catch (IOException | SecurityException ex) {
logger.debug("Failed to touch the cache file. Ignored. {}", cacheFileTmp.getAbsoluteFile());
logger.info(
"Failed to touch the cache file: {}. Ignored. {}",
ex.getMessage(),
cacheFileTmp.getAbsoluteFile());
}
return this;
}
Expand All @@ -184,7 +213,13 @@ JsonNode readCacheFile() {

try (Reader reader =
new InputStreamReader(new FileInputStream(cacheFile), DEFAULT_FILE_ENCODING)) {
FileUtil.logFileUsage(cacheFile, "Read cache", false);

if (onlyOwnerPermissions) {
FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(cacheFile, "Read cache");
FileUtil.throwWhenOwnerDifferentThanCurrentUser(cacheFile, "Read cache");
} else {
FileUtil.logFileUsage(cacheFile, "Read cache", false);
}
return OBJECT_MAPPER.readTree(reader);
}
} catch (IOException ex) {
Expand All @@ -208,7 +243,11 @@ void writeCacheFile(JsonNode input) {
}
try (Writer writer =
new OutputStreamWriter(new FileOutputStream(cacheFile), DEFAULT_FILE_ENCODING)) {
FileUtil.logFileUsage(cacheFile, "Write to cache", false);
if (onlyOwnerPermissions) {
FileUtil.throwWhenPermiossionDifferentThanReadWriteForOwner(cacheFile, "Write to cache");
} else {
FileUtil.logFileUsage(cacheFile, "Write to cache", false);
}
writer.write(input.toString());
}
} catch (IOException ex) {
Expand Down
78 changes: 74 additions & 4 deletions src/main/java/net/snowflake/client/core/FileUtil.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package net.snowflake.client.core;

import static net.snowflake.client.jdbc.SnowflakeUtil.isWindows;

import com.google.common.base.Strings;
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.FileOwnerAttributeView;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Arrays;
import java.util.Collection;
Expand All @@ -19,9 +22,13 @@ public class FileUtil {
Arrays.asList(PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_WRITE);
private static final Collection<PosixFilePermission> READ_BY_OTHERS =
Arrays.asList(PosixFilePermission.GROUP_READ, PosixFilePermission.OTHERS_READ);
private static final Collection<PosixFilePermission> EXECUTABLE =
Arrays.asList(
PosixFilePermission.OWNER_EXECUTE,
PosixFilePermission.GROUP_EXECUTE,
PosixFilePermission.OTHERS_EXECUTE);

public static void logFileUsage(Path filePath, String context, boolean logReadAccess) {
logger.info("{}Accessing file: {}", getContextStr(context), filePath);
logWarnWhenAccessibleByOthers(filePath, context, logReadAccess);
}

Expand All @@ -34,10 +41,40 @@ public static void logFileUsage(String stringPath, String context, boolean logRe
logFileUsage(path, context, logReadAccess);
}

public static void throwWhenPermiossionDifferentThanReadWriteForOwner(File file, String context) {
throwWhenPermiossionDifferentThanReadWriteForOwner(file.toPath(), context);
}

public static void throwWhenPermiossionDifferentThanReadWriteForOwner(
Path filePath, String context) {
// we do not check the permissions for Windows
if (isWindows()) {
return;
}

try {
Collection<PosixFilePermission> filePermissions = Files.getPosixFilePermissions(filePath);
boolean isWritableByOthers = isPermPresent(filePermissions, WRITE_BY_OTHERS);
boolean isReadableByOthers = isPermPresent(filePermissions, READ_BY_OTHERS);
boolean isExecutable = isPermPresent(filePermissions, EXECUTABLE);

if (isWritableByOthers || isReadableByOthers || isExecutable) {
logger.debug(
"{}File {} access rights: {}", getContextStr(context), filePath, filePermissions);
throw new SecurityException(
String.format("Access to file %s is wider than allowed only to the owner", filePath));
}
} catch (IOException e) {
throw new SecurityException(
String.format(
"%s Unable to access the file to check the permissions. Error: %s", filePath, e));
}
}

private static void logWarnWhenAccessibleByOthers(
Path filePath, String context, boolean logReadAccess) {
// we do not check the permissions for Windows
if (Constants.getOS() == Constants.OS.WINDOWS) {
if (isWindows()) {
return;
}

Expand All @@ -48,14 +85,16 @@ private static void logWarnWhenAccessibleByOthers(

boolean isWritableByOthers = isPermPresent(filePermissions, WRITE_BY_OTHERS);
boolean isReadableByOthers = isPermPresent(filePermissions, READ_BY_OTHERS);
boolean isExecutable = isPermPresent(filePermissions, EXECUTABLE);

if (isWritableByOthers || (isReadableByOthers && logReadAccess)) {
if (isWritableByOthers || (isReadableByOthers || isExecutable)) {
logger.warn(
"{}File {} is accessible by others to:{}{}",
getContextStr(context),
filePath,
isReadableByOthers && logReadAccess ? " read" : "",
isWritableByOthers ? " write" : "");
isWritableByOthers ? " write" : "",
isExecutable ? " executable" : "");
}
} catch (IOException e) {
logger.warn(
Expand All @@ -66,12 +105,43 @@ private static void logWarnWhenAccessibleByOthers(
}
}

public static void throwWhenOwnerDifferentThanCurrentUser(File file, String context) {
// we do not check the permissions for Windows
if (isWindows()) {
return;
}

Path filePath = Paths.get(file.getPath());

try {
String fileOwnerName = getFileOwnerName(filePath);
String currentUser = System.getProperty("user.name");
if (!currentUser.equalsIgnoreCase(fileOwnerName)) {
logger.debug(
"The file owner: {} is different than current user: {}", fileOwnerName, currentUser);
throw new SecurityException("The file owner is different than current user");
}
} catch (IOException e) {
logger.warn(
"{}Unable to access the file to check the owner: {}. Error: {}",
getContextStr(context),
filePath,
e);
}
}

private static boolean isPermPresent(
Collection<PosixFilePermission> filePerms, Collection<PosixFilePermission> permsToCheck)
throws IOException {
return filePerms.stream().anyMatch(permsToCheck::contains);
}

static String getFileOwnerName(Path filePath) throws IOException {
FileOwnerAttributeView ownerAttributeView =
Files.getFileAttributeView(filePath, FileOwnerAttributeView.class);
return ownerAttributeView.getOwner().getName();
}

private static String getContextStr(String context) {
return Strings.isNullOrEmpty(context) ? "" : context + ": ";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ public class SFTrustManager extends X509ExtendedTrustManager {
.setBaseCacheFileName(CACHE_FILE_NAME)
.setCacheExpirationInSeconds(CACHE_EXPIRATION_IN_SECONDS)
.setCacheFileLockExpirationInSeconds(CACHE_FILE_LOCK_EXPIRATION_IN_SECONDS)
.setOnlyOwnerPermissions(false)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static net.snowflake.client.core.SessionUtil.CLIENT_SFSQL;
import static net.snowflake.client.core.SessionUtil.JVM_PARAMS_TO_PARAMS;
import static net.snowflake.client.jdbc.SnowflakeUtil.isWindows;
import static net.snowflake.client.jdbc.SnowflakeUtil.systemGetProperty;

import java.io.IOException;
Expand Down Expand Up @@ -278,7 +279,7 @@ private void createLogFolder(Path path) throws SnowflakeSQLLoggedException {
}

private void checkLogFolderPermissions(Path path) throws SnowflakeSQLLoggedException {
if (Constants.getOS() != Constants.OS.WINDOWS) {
if (!isWindows()) {
try {
Set<PosixFilePermission> folderPermissions = Files.getPosixFilePermissions(path);
if (folderPermissions.contains(PosixFilePermission.GROUP_WRITE)
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/net/snowflake/client/jdbc/SnowflakeUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -901,4 +901,14 @@ public static Map<String, String> createCaseInsensitiveMap(Header[] headers) {
return new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
}
}

/**
* Check whether the OS is Windows
*
* @return boolean
*/
@SnowflakeJdbcInternalApi
public static boolean isWindows() {
return Constants.getOS() == Constants.OS.WINDOWS;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static net.snowflake.client.config.SFConnectionConfigParser.SKIP_TOKEN_FILE_PERMISSIONS_VERIFICATION;
import static net.snowflake.client.config.SFConnectionConfigParser.SNOWFLAKE_DEFAULT_CONNECTION_NAME_KEY;
import static net.snowflake.client.config.SFConnectionConfigParser.SNOWFLAKE_HOME_KEY;
import static net.snowflake.client.jdbc.SnowflakeUtil.isWindows;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand All @@ -25,7 +26,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import net.snowflake.client.core.Constants;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.jdbc.SnowflakeUtil;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -237,7 +237,7 @@ private void prepareConnectionConfigurationTomlFile(

private Path createFilePathWithPermission(Path path, boolean onlyUserPermission)
throws IOException {
if (Constants.getOS() != Constants.OS.WINDOWS) {
if (!isWindows()) {
FileAttribute<Set<PosixFilePermission>> fileAttribute =
onlyUserPermission
? PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rw-------"))
Expand Down
Loading
Loading