SNOW-1825607 Initial OCSP deprecation plan steps

[sfc-gh-ext-simba-jy]

Description

Please explain the changes you made here.

• FAIL_OPEN log changed (message + log level)?
• FAIL_OPEN passing request along if responder URL is missing?
• FAIL_CLOSED - no NPE if responder URL is missing? Please check the message below about this.
  Updated: the driver will log the debug message and return callback with null if the OCSP status is FAIL_OPEN. If the status is FAIL_CLOSED, the driver will log the error message and return callback with the error.
• Retries and timeouts confirmed (both to responder and cache server)?
• disableOCSPChecks introduced?
  • Deprecated insecureMode?
  • disableOCSPChecks has precedence?

Checklist

• Format code according to the existing code style (run npm run lint:check -- CHANGED_FILES and fix problems in changed code)
• Create tests which fail without the change (if possible)
• Make all tests (unit and integration) pass (npm run test:unit and npm run test:integration)
• Extend the README / documentation and ensure is properly displayed (if necessary)
• Provide JIRA issue id (if possible) or GitHub issue id in commit message

[sfc-gh-ext-simba-jy]

It seems like the node.js driver does not have the use case that the responder URL does not exist according to the code:

snowflake-connector-nodejs/lib/agent/check.js

Line 176 in bada542

let parsedUrl = require('url').parse(process.env.SF_OCSP_RESPONSE_CACHE_SERVER_URL);

. As we have the default OCSP_RESPONSE_CACHE_SERVER_URL and there will be no error for the responder URL, this will not throw the error like JDBC.

[codecov]

Codecov Report

Attention: Patch coverage is 81.81818% with 4 lines in your changes missing coverage. Please review.

Project coverage is 88.94%. Comparing base (b74508b) to head (a3d8d8e).

Files with missing lines	Patch %	Lines
lib/agent/check.js	42.85%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #973      +/-   ##
==========================================
- Coverage   88.96%   88.94%   -0.03%     
==========================================
  Files          70       70              
  Lines        6825     6830       +5     
==========================================
+ Hits         6072     6075       +3     
- Misses        753      755       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sfc-gh-pfus]

It seems like the node.js driver does not have the use case that the responder URL does not exist according to the code:

snowflake-connector-nodejs/lib/agent/check.js

Line 176 in bada542

let parsedUrl = require('url').parse(process.env.SF_OCSP_RESPONSE_CACHE_SERVER_URL);

. As we have the default OCSP_RESPONSE_CACHE_SERVER_URL and there will be no error for the responder URL, this will not throw the error like JDBC.

What you mentioned is not a responder URL - it is a cache server URL. Responder URL is provided in each certificate.

[sfc-gh-dszmolka]

It seems like the node.js driver does not have the use case that the responder URL does not exist according to the code:

snowflake-connector-nodejs/lib/agent/check.js

Line 176 in bada542

let parsedUrl = require('url').parse(process.env.SF_OCSP_RESPONSE_CACHE_SERVER_URL);

. As we have the default OCSP_RESPONSE_CACHE_SERVER_URL and there will be no error for the responder URL, this will not throw the error like JDBC.

OCSP Responder url != OCSP Cache URL. The (default) OCSP Cache URL is always the same, by default, ocsp.snowflakecomputing.com (gracefully ignoring privatelink situations here)

However OCSP Responder URL is not from Snowflake. It is always hardcoded into the particular certificate, which is under verification. See this random cert as an example: https://crt.sh/?sha256=9c3f2fd11c57d7c649ad5a0932c0f0d29756f6a0a1c74c43e1e89a62d64cd320

            Authority Information Access: 
                CA Issuers - URI:http://i.pki.goog/r4.crt

It does not have OCSP endpoint. The driver today will complain if it encounters such certs which doesn't have the OCSP endpoint, because the driver expects the cert to have it. See this issue what exactly happens: #932

As a comparison, here's a cert which does have OCSP endpoint: https://crt.sh/?id=12092745633
Observe:

            Authority Information Access: 
>>>>            OCSP - URI:http://ocsp.r2m03.amazontrust.com
                CA Issuers - URI:http://crt.r2m03.amazontrust.com/r2m03.cer

(this section is missing from the first cert)

The goal here, is for the driver if encountering a cert without the said OCSP endpoint, then it should just log the debug-level message and gracefully continue as if the verification was successful.
Implementing this change will also fix 932.

edit: I see Piotr also pointed the same out while I was typing this message :)

[sfc-gh-pmotacki]

Hi @sfc-gh-ext-simba-jy. Because we are planning to release the BCR in the next version, we have an option to remove the old parameter entirely, not only to set it deprecated. Could you make changes to the PR?

[sfc-gh-ext-simba-jy]

@sfc-gh-pmotacki, I removed the old option (insecureConnect). Please have a look.

[sfc-gh-pmotacki]

LGTM