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

crypto/x509: UnknownAuthorityError should expose private Certificate #13519

Closed
rolandshoemaker opened this issue Dec 7, 2015 · 5 comments
Closed
Assignees
Milestone

Comments

@rolandshoemaker
Copy link
Member

@rolandshoemaker rolandshoemaker commented Dec 7, 2015

Like CertificateInvalidError and HostnameError, it would be useful to expose the Certificate in UnknownAuthorityError as this provides important information for TLS/X509 testing tools.

This would also present a slightly more consistent API for the various X509 validation errors.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Dec 7, 2015

The information seems to be there:

// UnknownAuthorityError results when the certificate issuer is unknown                                                                                                                        
type UnknownAuthorityError struct {
        cert *Certificate
        // hintErr contains an error that may be helpful in determining why an                                                                                                                 
        // authority wasn't found.                                                                                                                                                             
        hintErr error
        // hintCert contains a possible authority certificate that was rejected                                                                                                                
        // because of the error in hintErr.                                                                                                                                                    
        hintCert *Certificate
}

So maybe it just needs an accessor method?

/cc @agl for opinions

@bradfitz bradfitz added this to the Go1.7 milestone Dec 7, 2015
@rolandshoemaker
Copy link
Member Author

@rolandshoemaker rolandshoemaker commented Dec 7, 2015

An accessor method would work, I was originally thinking of renaming cert->Certificate following the example of the other error types in the package but I didn't delve too deep into figuring out how much of a pain that change would be.

Either way I'm happy to write up a patch for this and sit on it until the freeze is over.

@agl agl self-assigned this Jan 31, 2016
@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@quentinmit quentinmit added the NeedsFix label Oct 5, 2016
@rsc
Copy link
Contributor

@rsc rsc commented Oct 25, 2016

Ping @agl: Does it seem OK to add:

func (e *UnknownAuthorityError) Certificate() *Certificate { return e.cert } 

?

We can write the CL. Thanks.

@rsc
Copy link
Contributor

@rsc rsc commented Nov 3, 2016

Assuming yes given CertificateInvalidError and HostnameError. Too bad those spell Cert/Certificate differently. I'll go with Cert to match CertificateInvalidError.

@gopherbot
Copy link

@gopherbot gopherbot commented Nov 3, 2016

CL https://golang.org/cl/32644 mentions this issue.

@gopherbot gopherbot closed this in b891357 Nov 3, 2016
@golang golang locked and limited conversation to collaborators Nov 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.