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

Add dangerous server cert verifier which uses pinned DNS name for cert validation #312

Closed
wants to merge 1 commit into from

Conversation

gorup
Copy link
Contributor

@gorup gorup commented Oct 8, 2019

Hello again!

As commit message states, this allows users to have a client config which will trust the server based on if its certificates are valid for the pinned DNS name.

This is useful when you want to use the SNI to indicate to the target to the server, but the server does not have a certificate valid for the target. Specifically useful for TLS proxies. Let me know if tests would be needed seeing as this is a dangerous-only provided struct.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 96.253% when pulling b42a77d on gorup:danger into 32cade4 on ctz:master.

@ctz
Copy link
Member

ctz commented Oct 9, 2019

Since this can be done entirely on top of the existing API I'm minded not to accept this. It doesn't seem generally useful, and is a quite application-specific behaviour?

@ctz
Copy link
Member

ctz commented Oct 9, 2019

If you do move this to a higher-level crate, note that WebPKIVerifier is public so you can call its verify_server_cert behaviour in your own verify_server_cert to get the behaviour you have here.

@ctz ctz closed this Oct 12, 2019
@gorup
Copy link
Contributor Author

gorup commented Oct 15, 2019

unless I'm mistaken, WebPKIVerifier is not public? Its not re-exported when Dangerous is on, and not present otherwise.

Should I put up a pull-request w/ just adding WebPKIVerifier to be exported when Dangerous is on?

@ctz
Copy link
Member

ctz commented Oct 15, 2019

Yes please do! That's an oversight.

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.

None yet

3 participants