Skip to content

security: accept CA keys in either PKCS#1 or PKCS#8 encodings#64943

Draft
knz wants to merge 1 commit intocockroachdb:masterfrom
knz:20210510-pkcs
Draft

security: accept CA keys in either PKCS#1 or PKCS#8 encodings#64943
knz wants to merge 1 commit intocockroachdb:masterfrom
knz:20210510-pkcs

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented May 10, 2021

Fixes #64942.

NB: I am not sure at which location I should implement a unit test for this. MY original goal was to create an end-to-end integration test for cockroach connect join, and this catches it immediately. However, maybe we can make a unit test that's smaller in scope?

Taking suggestions.

@knz knz requested review from a team, aaron-crl and bdarnell May 10, 2021 15:22
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown

@aaron-crl aaron-crl left a comment

Choose a reason for hiding this comment

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

Approach looks good.

I'd be happy to see a light unit test in the security package here for this with a static pair of test certs. E2E would be good too but shouldn't block this.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell)

@bdarnell
Copy link
Copy Markdown
Contributor

It looks like cockroach cert create-ca unconditionally uses PKCS1 (PKCS8 is only an option for client certs) and the new auto-TLS stuff unconditionally uses PKCS8. Why the difference? When would this code ever see a PKCS1 key?

I think we already have a function for this: parsePrivateKey in security/pem.go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cockroach connect join gets confused by CA key file format

4 participants