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

Cert verification for non-CA certs #2761

Merged
merged 3 commits into from
May 25, 2017
Merged

Conversation

@vishalnayak vishalnayak added this to the 0.7.3 milestone May 24, 2017
@jefferai
Copy link
Member

Thinking back on this I'm not super convinced this wasn't on purpose. The idea being if you are putting in a specific cert then you're really tying to a key more than a lifecycle. I agree that it's unexpected though so probably we should change this.

// We are not looking for trusted chains here since this is a
// non-CA cert. But validating the connection state detects
// expired certificates.
_, err := validateConnState(roots, connState)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can just move this check that is happening after this block above the block and avoid the code duplication.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@vishalnayak vishalnayak merged commit 6a73552 into master May 25, 2017
@vishalnayak vishalnayak deleted the cert-validation-non-ca branch May 25, 2017 14:49
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.

3 participants