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

error message may not right #119

Closed
JeyJeyGao opened this issue Feb 9, 2023 · 1 comment · Fixed by #120
Closed

error message may not right #119

JeyJeyGao opened this issue Feb 9, 2023 · 1 comment · Fixed by #120
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@JeyJeyGao
Copy link
Contributor

JeyJeyGao commented Feb 9, 2023

In line 67 of the code, if i == 0 which is the leaf cert, the error message still says that it is the intermediate certificate, so the error message may not right. It may need to return the CommonName of the cert to help use identify the invalid cert.

for i, cert := range certChain {
if signingTime != nil && (signingTime.Before(cert.NotBefore) || signingTime.After(cert.NotAfter)) {
return fmt.Errorf("certificate with subject %q was not valid at signing time of %s", cert.Subject, signingTime.UTC())
}
if i == len(certChain)-1 {
if !isSelfSigned(cert) {
return errors.New("certificate chain must end with a root certificate (root certificates are self-signed)")
}
} else {
// This is to avoid extra/redundant multiple root cert at the end of certificate-chain
if isSelfSigned(cert) {
return errors.New("certificate chain must not contain self-signed intermediate certificates")
} else if nextCert := certChain[i+1]; !isIssuedBy(cert, nextCert) {
return fmt.Errorf("certificate with subject %q is not issued by %q", cert.Subject, nextCert.Subject)
}
}
if i == 0 {
if err := validateLeafCertificate(cert, expectedLeafEku); err != nil {
return err
}
} else {

@JeyJeyGao JeyJeyGao added the enhancement New feature or request label Feb 9, 2023
@patrickzheng200 patrickzheng200 self-assigned this Feb 9, 2023
@patrickzheng200 patrickzheng200 added this to the RC-2 milestone Feb 9, 2023
@patrickzheng200
Copy link

Nice catch. And instead of the CommonName, we could show the Subject of the cert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants