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-1825607 Initial OCSP deprecation plan steps #973

Merged
merged 18 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
10 changes: 8 additions & 2 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ declare module 'snowflake-sdk' {

// 403001
ERR_GLOBAL_CONFIGURE_INVALID_LOG_LEVEL = 403001,
ERR_GLOBAL_CONFIGURE_INVALID_INSECURE_CONNECT = 403002,
ERR_GLOBAL_CONFIGURE_INVALID_DISABLE_OCSP_CHECKS = 403002,
ERR_GLOBAL_CONFIGURE_INVALID_OCSP_MODE = 403003,
ERR_GLOBAL_CONFIGURE_INVALID_JSON_PARSER = 403004,
ERR_GLOBAL_CONFIGURE_INVALID_XML_PARSER = 403005,
Expand Down Expand Up @@ -219,10 +219,16 @@ declare module 'snowflake-sdk' {
additionalLogToConsole?: boolean | null;

/**
* Check the ocsp checking is off.
* @deprecated
* This option will be deprecated. Use disableOCSPChecks
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
*/
insecureConnect?: boolean;

/**
* The option to turn off the OCSP check.
*/
disableOCSPChecks?: boolean;

/**
* The default value is true.
* Detailed information: https://docs.snowflake.com/en/user-guide/ocsp.
Expand Down
2 changes: 1 addition & 1 deletion lib/agent/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ function getResponse(uri, req, cb) {

module.exports = function check(options, cb, mock) {
let sync = true;
const maxNumRetries = GlobalConfig.getOcspMode() === GlobalConfig.ocspModes.FAIL_CLOSED ? 5 : 1;
const maxNumRetries = GlobalConfig.getOcspMode() === GlobalConfig.ocspModes.FAIL_CLOSED ? 2 : 1;

function done(err, data) {
if (sync) {
Expand Down
9 changes: 2 additions & 7 deletions lib/agent/socket_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ const ErrorCodes = Errors.codes;

const REGEX_SNOWFLAKE_ENDPOINT = /.snowflakecomputing./;

const ocspFailOpenWarning =
'WARNING!!! using fail-open to connect. Driver is connecting to an HTTPS endpoint ' +
'without OCSP based Certificated Revocation checking as it could not obtain a valid OCSP Response to use from ' +
'the CA OCSP responder. Details: ';

const socketSecuredEvent = 'secureConnect';

const rawOcspFlag =
Expand Down Expand Up @@ -120,7 +115,7 @@ exports.secureSocket = function (socket, host, agent, mock) {
function isOcspValidationDisabled(host) {
// ocsp is disabled if insecure-connect is enabled, or if we've disabled ocsp
// for non-snowflake endpoints and the host is a non-snowflake endpoint
return GlobalConfig.isInsecureConnect() ||
return GlobalConfig.isOCSPChecksDisabled() ||
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
(Parameters.getValue(Parameters.names.JS_DRIVER_DISABLE_OCSP_FOR_NON_SF_ENDPOINTS) &&
!REGEX_SNOWFLAKE_ENDPOINT.test(host));
}
Expand Down Expand Up @@ -158,7 +153,7 @@ function canEarlyExitForOCSP(errors) {
const err = errors[errorIndex];
if (err && !isValidOCSPError(err)) {
// any of the errors is NOT good/revoked/unknown
Logger.getInstance().warn(ocspFailOpenWarning + err);
Logger.getInstance().debug('OCSP responder didn\'t respond correctly. Assuming certificate is not revoked');
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
return null;
} else if (err && err.code === ErrorCodes.ERR_OCSP_REVOKED) {
anyRevoked = err;
Expand Down
2 changes: 1 addition & 1 deletion lib/constants/error_messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ exports[402002] = 'Request to S3/Blob failed.';

// 403001
exports[403001] = 'Invalid logLevel. The specified value must be one of these five levels: error, warn, debug, info and trace.';
exports[403002] = 'Invalid insecureConnect option. The specified value must be a boolean.';
exports[403002] = 'Invalid disableOCSPChecks option. The specified value must be a boolean.';
exports[403003] = 'Invalid OCSP mode. The specified value must be FAIL_CLOSED, FAIL_OPEN, or INSECURE_MODE.';
exports[403004] = 'Invalid custom JSON parser. The specified value must be a function.';
exports[403005] = 'Invalid custom XML parser. The specified value must be a function.';
Expand Down
16 changes: 10 additions & 6 deletions lib/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,18 @@
Logger.getInstance().info('Configuring logger with level: %s, filePath: %s, additionalLogToConsole: %s', logLevel, logFilePath, additionalLogToConsole);
}

const insecureConnect = options.insecureConnect;
if (Util.exists(insecureConnect)) {
if (Util.exists(options.insecureConnect)) {
Logger.getInstance().warn('Warning! The option insecureConnect will be deprecated. Please use the disableOCSPChecks.');

Check warning on line 200 in lib/core.js

View check run for this annotation

Codecov / codecov/patch

lib/core.js#L200

Added line #L200 was not covered by tests
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
}

const disableOCSPChecks = options.insecureConnect || options.disableOCSPChecks;
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
if (Util.exists(disableOCSPChecks)) {
// check that the specified value is a boolean
Errors.checkArgumentValid(Util.isBoolean(insecureConnect),
ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_INSECURE_CONNECT);
Errors.checkArgumentValid(Util.isBoolean(disableOCSPChecks),
ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_DISABLE_OCSP_CHECKS);

GlobalConfig.setInsecureConnect(insecureConnect);
Logger.getInstance().debug('Setting insecureConnect to value from core options: %s', insecureConnect);
GlobalConfig.setDisableOCSPChecks(disableOCSPChecks);
Logger.getInstance().debug('Setting disableOCSPChecks to value from core options: %s', disableOCSPChecks);
}

const ocspFailOpen = options.ocspFailOpen;
Expand Down
2 changes: 1 addition & 1 deletion lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ codes.ERR_LARGE_RESULT_SET_RESPONSE_FAILURE = 402002;

// 403001
codes.ERR_GLOBAL_CONFIGURE_INVALID_LOG_LEVEL = 403001;
codes.ERR_GLOBAL_CONFIGURE_INVALID_INSECURE_CONNECT = 403002;
codes.ERR_GLOBAL_CONFIGURE_INVALID_DISABLE_OCSP_CHECKS = 403002;
codes.ERR_GLOBAL_CONFIGURE_INVALID_OCSP_MODE = 403003;
codes.ERR_GLOBAL_CONFIGURE_INVALID_JSON_PARSER = 403004;
codes.ERR_GLOBAL_CONFIGURE_INVALID_XML_PARSER = 403005;
Expand Down
16 changes: 8 additions & 8 deletions lib/global_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,27 @@ const Util = require('./util');
const Logger = require('./logger');
const { XMLParser, XMLValidator } = require('fast-xml-parser');

let insecureConnect = false;
let disableOCSPChecks = false;

/**
* Updates the value of the 'insecureConnect' parameter.
* Updates the value of the 'disableOCSPChecks' parameter.
*
* @param {boolean} value
*/
exports.setInsecureConnect = function (value) {
exports.setDisableOCSPChecks = function (value) {
// validate input
Errors.assertInternal(Util.isBoolean(value));

insecureConnect = value;
disableOCSPChecks = value;
};

/**
* Returns the value of the 'insecureConnect' parameter.
* Returns the value of the 'disableOCSPChecks' parameter.
*
* @returns {boolean}
*/
exports.isInsecureConnect = function () {
return insecureConnect;
exports.isOCSPChecksDisabled = function () {
return disableOCSPChecks;
};

let ocspFailOpen = true;
Expand Down Expand Up @@ -71,7 +71,7 @@ exports.ocspModes = ocspModes;
* @returns {string}
*/
exports.getOcspMode = function () {
if (insecureConnect) {
if (disableOCSPChecks) {
return ocspModes.INSECURE;
} else if (!ocspFailOpen) {
return ocspModes.FAIL_CLOSED;
Expand Down
2 changes: 1 addition & 1 deletion test/integration/testStructuredType.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('Test Structured types', function () {
connection = testUtil.createConnection();
async.series([
function (callback) {
// snowflake.configure({ 'insecureConnect': true });
// snowflake.configure({ 'disableOCSPChecks': true });
// GlobalConfig.setInsecureConnect(true);
testUtil.connect(connection, callback);
},
Expand Down
4 changes: 2 additions & 2 deletions test/unit/ocsp/test_unit_ocsp_mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ const assert = require('assert');
describe('OCSP mode', function () {
it('getOcspMode', function (done) {
// insecure mode
GlobalConfig.setInsecureConnect(true);
GlobalConfig.setDisableOCSPChecks(true);
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
assert.equal(GlobalConfig.getOcspMode(), GlobalConfig.ocspModes.INSECURE);

// insecure mode + Fail open
GlobalConfig.setOcspFailOpen(true);
assert.equal(GlobalConfig.getOcspMode(), GlobalConfig.ocspModes.INSECURE);
GlobalConfig.setInsecureConnect(false);
GlobalConfig.setDisableOCSPChecks(false);
assert.equal(GlobalConfig.getOcspMode(), GlobalConfig.ocspModes.FAIL_OPEN);

GlobalConfig.setOcspFailOpen(false);
Expand Down
20 changes: 10 additions & 10 deletions test/unit/snowflake_config_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('Snowflake Configure Tests', function () {
before(function () {
originalConfig = {
logLevel: Logger.getInstance().getLevelTag(),
insecureConnect: GlobalConfig.isInsecureConnect(),
disableOCSPChecks: GlobalConfig.isOCSPChecksDisabled(),
sfc-gh-pfus marked this conversation as resolved.
Show resolved Hide resolved
ocspFailOpen: GlobalConfig.getOcspFailOpen(),
keepAlive: GlobalConfig.getKeepAlive(),
jsonColumnVariantParser: GlobalConfig.jsonColumnVariantParser,
Expand All @@ -36,9 +36,9 @@ describe('Snowflake Configure Tests', function () {
errorCode: ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_LOG_LEVEL
},
{
name: 'invalid insecureConnect',
options: { insecureConnect: 'unsupported' },
errorCode: ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_INSECURE_CONNECT
name: 'invalid disableOCSPChecks',
options: { disableOCSPChecks: 'unsupported' },
errorCode: ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_DISABLE_OCSP_CHECKS
},
{
name: 'invalid ocspMode',
Expand Down Expand Up @@ -134,17 +134,17 @@ describe('Snowflake Configure Tests', function () {
}
},
{
name: 'insecureConnect false',
name: 'disableOCSPChecks false',
options:
{
insecureConnect: false
disableOCSPChecks: false
}
},
{
name: 'insecureConnect true',
name: 'disableOCSPChecks true',
options:
{
insecureConnect: true
disableOCSPChecks: true
}
},
{
Expand Down Expand Up @@ -213,8 +213,8 @@ describe('Snowflake Configure Tests', function () {
let val;
if (key === 'logLevel') {
val = Logger.getInstance().getLevelTag();
} else if (key === 'insecureConnect') {
val = GlobalConfig.isInsecureConnect();
} else if (key === 'disableOCSPChecks') {
val = GlobalConfig.isOCSPChecksDisabled();
} else if (key === 'ocspFailOpen') {
val = GlobalConfig.getOcspFailOpen();
} else if (key === 'keepAlive') {
Expand Down
Loading