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

deps: add new dependency to gopenpgp #103720

Closed
wants to merge 1 commit into from

Conversation

andyyang890
Copy link
Collaborator

@andyyang890 andyyang890 commented May 22, 2023

This patch adds a new dependency to github.com/ProtonMail/gopenpgp,
which will be used to help implement some of the remaining
unimplemented pgcrypto functions.

Informs #21001

Release note: None

@andyyang890 andyyang890 requested review from knz, rafiss and a team May 22, 2023 15:31
@andyyang890 andyyang890 requested review from a team as code owners May 22, 2023 15:31
@blathers-crl
Copy link

blathers-crl bot commented May 22, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

This patch adds a new dependency to github.com/ProtonMail/gopenpgp,
which will be used to help implement some of the remaining
unimplemented pgcrypto functions.

Release note: None
Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following dependencies are problematic:

There are two major problems with these new dependencies:

  • they implement their own cryptography, which is generally a no-no.

    Reference: see all the content available elsewhere already -- https://duckduckgo.com/?q=do+not+implement+your+own+cryptography

    It seems unreasonable to import so much untrusted code doing who-knows-what with cryptography just to implement some esoteric pgcrypto functions. There's a major imbalance in the cost/benefit trade-off.

  • they use native Go code, which is not going to be routed to a FIPS-enabled implementation during FIPS builds. This means that all the functionality we'd build on top would not enable FIPS compliance for customers. This is a major deviation from our "Customer Trust" and "Enterprise Readiness" goals for v23.1 and beyond.

@knz
Copy link
Contributor

knz commented May 23, 2023

Internal discussion - https://cockroachlabs.slack.com/archives/CGA9F858R/p1684855555474749
Note that Abhinav is on PTO this week.

@rafiss rafiss added the X-noremind Bots won't notify about PRs with X-noremind label Jun 1, 2023
@rafiss
Copy link
Collaborator

rafiss commented Jun 1, 2023

We've decided to deprioritize work on PGP related functions for now.

@rafiss rafiss closed this Jun 24, 2023
@andyyang890 andyyang890 deleted the dep_gopenpgp branch June 24, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants