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

Adding SNI to peer certificate verification #43

Merged
merged 11 commits into from
Aug 22, 2024

Conversation

WendelHime
Copy link
Contributor

@WendelHime WendelHime commented Aug 22, 2024

On this pull request I'm adding a VerifyHost attribute that shall be used when verifying peer certificates from requests that use the SNI config. Some unit tests were added, some updated with the new parameter and some deprecated libraries from cache.go have been replaced.

@coveralls
Copy link

coveralls commented Aug 22, 2024

Coverage Status

coverage: 86.156% (+1.4%) from 84.776%
when pulling 00ea97e on fix/verify-sni-certificate
into 6976132 on main.

@WendelHime WendelHime marked this pull request as ready for review August 22, 2024 14:36
@WendelHime WendelHime requested a review from hwh33 August 22, 2024 15:29
@WendelHime
Copy link
Contributor Author

Coveralls is failing but tests are passing

Copy link
Contributor

@hwh33 hwh33 left a comment

Choose a reason for hiding this comment

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

Looks great! Nice work

@hwh33
Copy link
Contributor

hwh33 commented Aug 22, 2024

Not sure why coveralls is returning 405, but let's just ignore that for now. Merge away! Could you open a PR to update this in Flashlight? You can just merge it straight away.

@WendelHime
Copy link
Contributor Author

Looks like they're down: https://status.coveralls.io/

@WendelHime WendelHime merged commit 6e97652 into main Aug 22, 2024
0 of 2 checks passed
@WendelHime WendelHime deleted the fix/verify-sni-certificate branch August 22, 2024 22:06
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