Ct 2106 update cryptography dependency

[Surbias]

resolves #465

Description

This PR updates cryptography dependency to be <=39.0.1.

This is needed as there are some high-security vulnerabilities flagged, namely:

Upgrade [email protected] to [email protected] to fix
  ✗ Denial of Service (DoS) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3172287] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Expected Behavior Violation (new) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3314966] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Use After Free (new) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3315324] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Timing Attack (new) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3315331] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Denial of Service (DoS) (new) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3315452] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Denial of Service (DoS) (new) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3315972] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Denial of Service (DoS) (new) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3315975] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Denial of Service (DoS) (new) [Medium Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3316038] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Access of Resource Using Incompatible Type ('Type Confusion') (new) [High Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3315328] in [email protected]
    introduced by [email protected] and 5 other path(s)
  ✗ Denial of Service (DoS) (new) [High Severity][https://security.snyk.io/vuln/SNYK-PYTHON-CRYPTOGRAPHY-3316211] in [email protected]
    introduced by [email protected] and 5 other path(s)

Checklist

• I have read the contributing guide and understand what's expected of me
• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
• I have run changie new to create a changelog entry

[dbeatty10]

Thanks for opening this PR @Surbias (and the last time this happened)!

This got me thinking -- what if we just remove the upper bound altogether?

Background context

At a high-level, we've been discussing revising our approach to pinning dependencies.

In the case of cryptography specifically, we've just been bumping upper bounds recently (even though I didn't see any explicit reasons to cap it):

• Bump upper bound on cryptography to <39.0.0 #360
• Bump cryptography restraint upper bound to <37.0.0 #171

I don't think we need to resolve the general discussion in order to take action on this specific case.

Our three options

So we have three main options as it relates to cryptography:

1. Have an inclusive upper bound that aligns with the most recently released version (<=39.0.1)
2. Maintain an exclusive upper bound that aligns with presumed next major version (<40.0.0)
3. Remove the upper bound altogether

My opinion

Personally, I'd be supportive of us just removing the upper bound here.

Rationale

If cryptography becomes incompatible in the future, we can discover it during nightly CI (or via a bug report, etc); we can add an upper bound at that point.

Alternatively, we can use <40.0.0 as an exclusive upper bound, but we know it's just a matter of time before we'll just bump it again.

I'd really prefer not to use <=39.0.1 as an exclusive upper bound because that would be the shortest amount of time until we're just bumping this up again.

I know there are ways we can do these types of bumps automatically, but the caps don't appear to be doing anything for us right now.

[Surbias]

I am inclined to agree with you @dbeatty10 as indeed the caps do not seem to be providing any benefit here.

I am happy to remove the cap on this PR if that's alright?

[dbeatty10]

@Surbias

I am inclined to agree with you @dbeatty10 as indeed the caps do not seem to be providing any benefit here.

I am happy to remove the cap on this PR if that's alright?

We're still debating our approach to upper bounds, and we're not yet ready to just remove it entirely for cryptography.

In the meantime, if you update the upper bound to <40.0.0, I think we could get this reviewed, merged, and released.

[Surbias]

@dbeatty10 sounds good to me! 😄

I have now updated the PR to update cryptography dependency to have version upper bound <40.0.0.

[fadi-circleci]

@dbeatty10 <40.0.0 should be good for a bit! Nice meeting you today.

[dbeatty10]

@dbeatty10 <40.0.0 should be good for a bit! Nice meeting you today.

Yes, it was great meeting you yesterday @fadi-circleci ! Thanks for weighing in on this.

[fadi-circleci]

@dbeatty10 <40.0.0 should be good for a bit! Nice meeting you today.

Yes, it was great meeting you yesterday @fadi-circleci ! Thanks for weighing in on this.

synk scans were also blocking us! Came over to submit a PR :)

[mikealfare]

Thanks for the PR @Surbias!

[lukehsiao]

We're seeing these vulnerabilities as well. When do you think the next dbt-snowflake release will be that incorporates this change?