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 1567099 cache permissions #998

Merged
merged 3 commits 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
2 changes: 1 addition & 1 deletion ci/container/test_authentication.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,4 @@ export SNOWFLAKE_AUTH_TEST_PRIVATE_KEY_PATH=./.github/workflows/rsa_keys/rsa_key
export SNOWFLAKE_AUTH_TEST_ENCRYPTED_PRIVATE_KEY_PATH=./.github/workflows/rsa_keys/rsa_encrypted_key.p8
export SNOWFLAKE_AUTH_TEST_INVALID_PRIVATE_KEY_PATH=./.github/workflows/rsa_keys/rsa_key_invalid.p8

npm run test:authentication
npm run test:authentication
2 changes: 1 addition & 1 deletion ci/test_authentication.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@ docker run \
-v $WORKSPACE:/mnt/workspace \
--rm \
nexus.int.snowflakecomputing.com:8086/docker/snowdrivers-test-external-browser:3 \
"/mnt/host/ci/container/test_authentication.sh"
"/mnt/host/ci/container/test_authentication.sh"
25 changes: 4 additions & 21 deletions lib/authentication/secure_storage/json_credential_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,9 @@ const Logger = require('../../logger');
const fs = require('node:fs/promises');
const os = require('os');
const Util = require('../../util');
const { validateOnlyUserReadWritePermissionAndOwner } = require('../../file_util');

function JsonCredentialManager(credentialCacheDir) {

async function validatePermission(filePath) {
try {
await fs.access(filePath, fs.constants.F_OK);
} catch (err) {
return;
}

const mode = (await fs.stat(filePath)).mode;
const permission = mode & 0o600;

//This should be 600 permission, which means the file permission has not been changed by others.
if (permission.toString(8) === '600') {
Logger.getInstance().debug('Validated that the user has read and write permission');
} else {
throw new Error('You do not have read permission or the file has been changed on the user side. Please remove the token file and re run the driver.');
}
}

this.getTokenDir = async function () {
let tokenDir = credentialCacheDir;
Expand All @@ -38,11 +21,11 @@ function JsonCredentialManager(credentialCacheDir) {

if (!Util.exists(tokenDir)) {
throw new Error(`Temporary credential cache directory is invalid, and the driver is unable to use the default location(home).
Please assign the environment variable value SF_TEMPORARY_CREDENTIAL_CACHE_DIR to enable the default credential manager.`);
Please set 'credentialCacheDir' connection configuration option to enable the default credential manager.`);
}

const tokenCacheFile = path.join(tokenDir, 'temporary_credential.json');
await validatePermission(tokenCacheFile);
await validateOnlyUserReadWritePermissionAndOwner(tokenCacheFile);
return tokenCacheFile;
};

Expand All @@ -51,7 +34,7 @@ function JsonCredentialManager(credentialCacheDir) {
const cred = await fs.readFile(await this.getTokenDir(), 'utf8');
return JSON.parse(cred);
} catch (err) {
Logger.getInstance().warn('Failed to read token data from the file. Please check the permission or the file format of the token.');
Logger.getInstance().warn('Failed to read token data from the file. Err: %s', err.message);
return null;
}
};
Expand Down
3 changes: 2 additions & 1 deletion lib/configuration/client_configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@
const os = require('os');
const path = require('path');
const fs = require('fs');
const { isString, exists, isFileNotWritableByGroupOrOthers, getDriverDirectory } = require('../util');
const { isString, exists, getDriverDirectory } = require('../util');
const Logger = require('../logger');
const { isFileNotWritableByGroupOrOthers } = require('../file_util');
const clientConfigFileName = 'sf_client_config.json';

const Levels = Object.freeze({
Expand Down
6 changes: 3 additions & 3 deletions lib/configuration/connection_configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
const toml = require('toml');
const os = require('os');
const fs = require('fs');
const { validateOnlyUserReadWritePermission, generateChecksum } = require('../file_transfer_agent/file_util');
const { validateOnlyUserReadWritePermissionAndOwner, generateChecksum } = require('../file_util');
const path = require('path');
const Logger = require('../logger');
const AuthenticationTypes = require('../authentication/authentication_types');
Expand All @@ -29,7 +29,7 @@
const tokenFilePath = fixedConfiguration.token_file_path ? fixedConfiguration.token_file_path : '/snowflake/session/token';
const resolvedPath = fs.realpathSync(tokenFilePath);
Logger.getInstance().trace('Token file path is : %s', tokenFilePath);
validateOnlyUserReadWritePermission(resolvedPath);
validateOnlyUserReadWritePermissionAndOwner(resolvedPath);

Check warning on line 32 in lib/configuration/connection_configuration.js

View check run for this annotation

Codecov / codecov/patch

lib/configuration/connection_configuration.js#L32

Added line #L32 was not covered by tests
fixedConfiguration.token = fs.readFileSync(resolvedPath, 'utf-8').trim();
if (!fixedConfiguration.token) {
Logger.getInstance().error('The token does not exist or has empty value.');
Expand All @@ -47,7 +47,7 @@
const resolvedPath = fs.realpathSync(filePath);
Logger.getInstance().trace('Connection configuration file found under the path %s. Validating file access.', resolvedPath);

validateOnlyUserReadWritePermission(resolvedPath);
validateOnlyUserReadWritePermissionAndOwner(resolvedPath);
const str = fs.readFileSync(resolvedPath, { encoding: 'utf8' });
const configurationChecksum = generateChecksum(str);
Logger.getInstance().info('Connection configuration file is read from path: %s. Checksum: %s', resolvedPath, configurationChecksum);
Expand Down
8 changes: 4 additions & 4 deletions lib/file_transfer_agent/azure_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
*/

const EncryptionMetadata = require('./encrypt_util').EncryptionMetadata;
const FileHeader = require('./file_util').FileHeader;
const FileHeader = require('../file_util').FileHeader;
const expandTilde = require('expand-tilde');
const resultStatus = require('./file_util').resultStatus;
const resultStatus = require('../file_util').resultStatus;
const ProxyUtil = require('../proxy_util');
const { isBypassProxy } = require('../http/node');
const Logger = require('../logger');
Expand Down Expand Up @@ -50,7 +50,7 @@ function AzureUtil(connectionConfig, azure, filestream) {
if (proxy && !isBypassProxy(proxy, connectionString)) {
Logger.getInstance().debug(`The destination host is: ${ProxyUtil.getHostFromURL(connectionString)} and the proxy host is: ${proxy.host}`);
Logger.getInstance().trace(`Initializing the proxy information for the Azure Client: ${ProxyUtil.describeProxy(proxy)}`);

proxy = ProxyUtil.getAzureProxy(proxy);
}
ProxyUtil.hideEnvironmentProxy();
Expand Down Expand Up @@ -217,7 +217,7 @@ function AzureUtil(connectionConfig, azure, filestream) {
blobContentEncoding: 'UTF-8',
blobContentType: 'application/octet-stream'
}
});
});
} catch (err) {
if (err['statusCode'] === 403 && detectAzureTokenExpireError(err)) {
meta['lastError'] = err;
Expand Down
4 changes: 2 additions & 2 deletions lib/file_transfer_agent/file_transfer_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ const SnowflakeRemoteStorageUtil = require('./remote_storage_util').RemoteStorag
const LocalUtil = require('./local_util').LocalUtil;
const SnowflakeFileEncryptionMaterial = require('./remote_storage_util').SnowflakeFileEncryptionMaterial;
const SnowflakeS3Util = require('./s3_util');
const { FileUtil, getMatchingFilePaths } = require('./file_util');
const resultStatus = require('./file_util').resultStatus;
const { FileUtil, getMatchingFilePaths } = require('../file_util');
const resultStatus = require('../file_util').resultStatus;

const SnowflakeFileUtil = new FileUtil();
const SnowflakeLocalUtil = new LocalUtil();
Expand Down
15 changes: 7 additions & 8 deletions lib/file_transfer_agent/gcs_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@
*/

const EncryptionMetadata = require('./encrypt_util').EncryptionMetadata;
const FileHeader = require('./file_util').FileHeader;
const FileHeader = require('../file_util').FileHeader;
const getProxyAgent = require('../http/node').getProxyAgent;
const ProxyUtil = require('../proxy_util');
const Util = require('../util');
const { shouldPerformGCPBucket, lstrip } = require('../util');


const GCS_METADATA_PREFIX = 'x-goog-meta-';
const SFC_DIGEST = 'sfc-digest';
const MATDESC_KEY = 'matdesc';
Expand All @@ -22,7 +21,8 @@ const GCS_FILE_HEADER_CONTENT_LENGTH = 'gcs-file-header-content-length';
const GCS_FILE_HEADER_ENCRYPTION_METADATA = 'gcs-file-header-encryption-metadata';

const HTTP_HEADER_CONTENT_ENCODING = 'Content-Encoding';
const resultStatus = require('./file_util').resultStatus;
const resultStatus = require('../file_util').resultStatus;

const { Storage } = require('@google-cloud/storage');

const EXPIRED_TOKEN = 'ExpiredToken';
Expand Down Expand Up @@ -54,7 +54,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
let axios = httpClient;
const fs = typeof fileStream !== 'undefined' ? fileStream : require('fs');
let isProxyEnabled = false;

/**
* Retrieve the GCS token from the stage info metadata.
*
Expand All @@ -73,7 +73,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
} else if (isRegionalUrlEnabled) {
endPoint = `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`;
}

let client;
if (gcsToken) {
const interceptors = [];
Expand All @@ -84,7 +84,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
return requestConfig;
}
});

const storage = Util.exists(endPoint) ? new Storage({ interceptors_: interceptors, apiEndpoint: endPoint }) : new Storage({ interceptors_: interceptors });
client = { gcsToken: gcsToken, gcsClient: storage };
} else {
Expand Down Expand Up @@ -162,7 +162,6 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
.file(gcsLocation.path + filename)
.getMetadata();


digest = metadata[0].metadata[SFC_DIGEST];
contentLength = metadata[0].size;
encryptionDataProp = metadata[0].metadata[ENCRYPTIONDATAPROP];
Expand Down Expand Up @@ -297,7 +296,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
try {
if (shouldPerformGCPBucket(accessToken) && !isProxyEnabled) {
const gcsLocation = this.extractBucketNameAndPath(meta['stageInfo']['location']);

await meta['client'].gcsClient
.bucket(gcsLocation.bucketName)
.file(gcsLocation.path + meta['dstFileName'])
Expand Down
2 changes: 1 addition & 1 deletion lib/file_transfer_agent/local_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
const fs = require('fs');
const path = require('path');
const expandTilde = require('expand-tilde');
const resultStatus = require('./file_util').resultStatus;
const resultStatus = require('../file_util').resultStatus;

/**
* Creates a local utility object.
Expand Down
2 changes: 1 addition & 1 deletion lib/file_transfer_agent/remote_storage_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const SnowflakeAzureUtil = require('./azure_util');
const SnowflakeGCSUtil = require('./gcs_util');

const SnowflakeEncryptionUtil = new (require('./encrypt_util').EncryptUtil)();
const resultStatus = require('./file_util').resultStatus;
const resultStatus = require('../file_util').resultStatus;

const DEFAULT_CONCURRENCY = 1;
const DEFAULT_MAX_RETRY = 5;
Expand Down
4 changes: 2 additions & 2 deletions lib/file_transfer_agent/s3_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

const { NodeHttpHandler } = require('@smithy/node-http-handler');
const EncryptionMetadata = require('./encrypt_util').EncryptionMetadata;
const FileHeader = require('./file_util').FileHeader;
const FileHeader = require('../file_util').FileHeader;
const expandTilde = require('expand-tilde');
const getProxyAgent = require('../http/node').getProxyAgent;
const ProxyUtil = require('../proxy_util');
Expand All @@ -21,7 +21,7 @@ const SNOWFLAKE_S3_DESTINATION = 's3.amazonaws.com';
const ERRORNO_WSAECONNABORTED = 10053; // network connection was aborted
const DATA_SIZE_THRESHOLD = 67108864; // magic number, given from error message.

const resultStatus = require('./file_util').resultStatus;
const resultStatus = require('../file_util').resultStatus;

const HTTP_HEADER_VALUE_OCTET_STREAM = 'application/octet-stream';

Expand Down
104 changes: 84 additions & 20 deletions lib/file_transfer_agent/file_util.js → lib/file_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
const zlib = require('zlib');
const os = require('os');
const glob = require('glob');
const Logger = require('../logger');
const Logger = require('./logger');

const resultStatus = {
ERROR: 'ERROR',
Expand Down Expand Up @@ -137,37 +137,101 @@
}
exports.FileUtil = FileUtil;

function getMatchingFilePaths(dir, fileName) {
exports.getMatchingFilePaths = function (dir, fileName) {
const pathWithWildcard = path.join(dir, fileName);
const pathWithWildcardDependsOnPlatform = os.platform() === 'win32'
? pathWithWildcard.replace(/\\/g, '/')
: pathWithWildcard;
return glob.sync(pathWithWildcardDependsOnPlatform);
}
};


function validateOnlyUserReadWritePermission(filePath) {
/**
* Checks if the provided file or directory is writable only by the user and os tha file owner is the same as os user. FsPromises can be provided.
* @param filePath
* @param expectedMode
* @param fsPromises
* @returns {Promise<boolean>} resolves always to true for Windows
*/
exports.validateOnlyUserReadWritePermissionAndOwner = async function (filePath, fsPromises) {
const fsp = fsPromises ? fsPromises : require('fs/promises');
if (os.platform() === 'win32') {
return;
}
fs.accessSync(filePath, fs.constants.F_OK);
const mode = (fs.statSync(filePath)).mode;
const permission = (mode & 0o00777 | 0o600);
//This should be 600 permission, which means the file permission has not been changed by others.
if (permission === 0o600) {
Logger.getInstance().debug(`Validated that the user has only read and write permission for file: ${filePath}, Permission: ${permission}`);
} else {
throw new Error(`File permissions different than read/write for user. File: ${filePath}`);
try {
const stats = await fsp.stat(filePath);
const mode = stats.mode;
const permission = mode & 0o777;

//This should be 600 permission, which means the file permission has not been changed by others.
const octalPermissions = permission.toString(8);
if (octalPermissions === '600') {
Logger.getInstance().debug(`Validated that the user has only read and write permission for file: ${filePath}, Permission: ${permission}`);
} else {
throw new Error(`Invalid file permissions (${octalPermissions} for file ${filePath}). Make sure you have read and write permissions and other users do not have access to it. Please remove the file and re-run the driver.`);
}

const userInfo = os.userInfo();
if (stats.uid === userInfo.uid) {
Logger.getInstance().debug('Validated file owner');
} else {
throw new Error(`Invalid file owner for file ${filePath}). Make sure the system user are the owner of the file otherwise please remove the file and re-run the driver.`);
}
} catch (err) {
//When file doesn't exist - return
if (err.code === 'ENOENT') {
return;
} else {
throw err;
}
}
}
};

function generateChecksum(str, algorithm, encoding) {
/**
* Checks if the provided file or directory permissions are correct.
* @param filePath
* @param expectedMode
* @param fsPromises
* @returns {Promise<boolean>} resolves always to true for Windows
*/
exports.isFileModeCorrect = async function (filePath, expectedMode, fsPromises) {
if (os.platform() === 'win32') {
return true;

Check warning on line 199 in lib/file_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_util.js#L199

Added line #L199 was not covered by tests
}
return await fsPromises.stat(filePath).then((stats) => {
// we have to limit the number of LSB bits to 9 with the mask, as the stats.mode starts with the file type,
// e.g. the directory with permissions 755 will have stats.mask of 40755.
const mask = (1 << 9) - 1;
return (stats.mode & mask) === expectedMode;
});
};

/**
* Checks if the provided file or directory is writable only by the user.
* @param configFilePath
* @param fsPromises
* @returns {Promise<boolean>} resolves always to true for Windows
*/
exports.isFileNotWritableByGroupOrOthers = async function (configFilePath, fsPromises) {
if (os.platform() === 'win32') {
return true;

Check warning on line 217 in lib/file_util.js

View check run for this annotation

Codecov / codecov/patch

lib/file_util.js#L217

Added line #L217 was not covered by tests
}
const stats = await fsPromises.stat(configFilePath);
return (stats.mode & (1 << 4)) === 0 && (stats.mode & (1 << 1)) === 0;
};

/**
* Generate checksum for given text. The algorithm and encoding can be provided.
* @param text
* @param algorithm
* @param encoding
* @returns {Promise<String>} resolves always to true for Windows
*/
exports.generateChecksum = function (text, algorithm, encoding) {
return crypto
.createHash(algorithm || 'sha256')
.update(str, 'utf8')
.update(text, 'utf8')
.digest(encoding || 'hex')
.substring(0, 32);
}
.substring(0, 32);
};

exports.getMatchingFilePaths = getMatchingFilePaths;
exports.validateOnlyUserReadWritePermission = validateOnlyUserReadWritePermission;
exports.generateChecksum = generateChecksum;
3 changes: 2 additions & 1 deletion lib/logger/easy_logging_starter.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const fs = require('fs');
const { logTagToLevel } = require('./core');
const { ConfigurationUtil, Levels } = require('../configuration/client_configuration');
const Logger = require('../logger');
const { isFileModeCorrect, exists } = require('../util');
const { isFileModeCorrect } = require('../file_util');
const { exists } = require('../util');
const clientConfiguration = new ConfigurationUtil();
const getClientConfig = clientConfiguration.getClientConfig;

Expand Down
Loading
Loading