Skip to content

fix: Remove duplicate authority key identifier extension #766

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

Merged
merged 1 commit into from
Apr 16, 2024

Conversation

Techassi
Copy link
Member

@Techassi Techassi commented Apr 16, 2024

Without removing this duplicate extension, the Go X.509 parsing code would error out stating that the "certificate contains duplicate extensions", which was indeed correct.

We accidentally included the authority key identifier twice, once in the leaf profile and once by manually adding the extension after building the cert.

We now removed the manually added extension. This resolved the Go error and the HTTP client was able to establish a TLS-secured connection to the dummy webhook.

See merge: https://go-review.googlesource.com/c/go/+/383215
See code: https://github.com/golang/go/blob/315b6ae682a2a4e7718924a45b8b311a0fe10043/src/crypto/x509/parser.go#L965-L968


To be able to access the OID of the duplicate extension, it was required to change Go's standard library. I will provide an upstream patch to improve the error message.

Also see the follow-up issue #694 to add tests / assertions around correct certificate generation.

Without removing this duplicate extension, the Go
X.509 parsing code would error out stating that the
"certificate contains duplicate extensions", which
was indeed correct.

We accidentially included the authority key identifier
twice, once in the leaf profile and once by manually
adding the extention after building the cert.

We now removed the manually added extension. This
resolved the Go error and the HTTP client was able
to establish a TLS-secured connection to the dummy
webhook.

See merge: https://go-review.googlesource.com/c/go/+/383215
See code: https://github.com/golang/go/blob/315b6ae682a2a4e7718924a45b8b311a0fe10043/src/crypto/x509/parser.go#L965-L968
@Techassi Techassi self-assigned this Apr 16, 2024
@NickLarsenNZ NickLarsenNZ self-requested a review April 16, 2024 11:44
Copy link
Member

@NickLarsenNZ NickLarsenNZ left a comment

Choose a reason for hiding this comment

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

LGTM

@Techassi Techassi added this pull request to the merge queue Apr 16, 2024
Merged via the queue into main with commit deecaa9 Apr 16, 2024
17 checks passed
@Techassi Techassi deleted the fix/tls-duplicate-extension branch April 16, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants