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.

Sign up for GitHub

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

Jump to bottom

crypto/x509: ParseCertificate should mention the ownership of the slice passed to it. #59548

Open
mateusz834 opened this issue Apr 11, 2023 · 2 comments
Open

crypto/x509: ParseCertificate should mention the ownership of the slice passed to it. #59548

mateusz834 opened this issue Apr 11, 2023 · 2 comments
Labels
Documentation Issues describing a change to documentation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@mateusz834
Copy link
Member

ParseCertificate and ParseCertificates should mention that the argument (der) is the same as the cert.Raw.

go/src/crypto/x509/parser.go

Lines 800 to 810 in 267b50a

func parseCertificate(der []byte) (*Certificate, error) {
cert := &Certificate{}
input := cryptobyte.String(der)
// we read the SEQUENCE including length and tag bytes so that
// we can populate Certificate.Raw, before unwrapping the
// SEQUENCE so it can be operated on
if !input.ReadASN1Element(&input, cryptobyte_asn1.SEQUENCE) {
return nil, errors.New("x509: malformed certificate")
}
cert.Raw = input

From the docs is doesn't seem to be the case.

go/src/crypto/x509/parser.go

Lines 983 to 985 in 267b50a

// ParseCertificate parses a single certificate from the given ASN.1 DER data.
func ParseCertificate(der []byte) (*Certificate, error) {
cert, err := parseCertificate(der)

All of these fields should mention that they share the memory with the slice passed to ParseCertificate.

go/src/crypto/x509/x509.go

Lines 677 to 684 in 267b50a

type Certificate struct {
Raw []byte // Complete ASN.1 DER content (certificate, signature algorithm and signature).
RawTBSCertificate []byte // Certificate part of raw ASN.1 DER content.
RawSubjectPublicKeyInfo []byte // DER encoded SubjectPublicKeyInfo.
RawSubject []byte // DER encoded Subject
RawIssuer []byte // DER encoded Issuer
Signature []byte

@seankhliao seankhliao added Documentation Issues describing a change to documentation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 11, 2023
@seankhliao
Copy link
Member

cc @golang/security

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/496143 mentions this issue: crypto/x509: mention arg ownership of ParseCertificate(Request)

@seankhliao seankhliao added this to the Unplanned milestone Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Documentation Issues describing a change to documentation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
3 participants
@gopherbot @seankhliao @mateusz834