-
Notifications
You must be signed in to change notification settings - Fork 165
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
Support for IPv4 and IPv6 #122
base: main
Are you sure you want to change the base?
Support for IPv4 and IPv6 #122
Conversation
…rds in SAN and CN.
CommonName is deprecated and should (must) no longer be used for validation: |
@Darkspirit Thanks for pointing this out. This poses the following questions:
For the time being, I change my code to ingore CN by default (as you suggested), but still allow API users to opt-in to legacy behavior in case they need to process (very) old certificates. |
…d for API clients who need to process old certificates. - Added support for more ASN/DER string types, although they are rarely seen out in the wild, this cannot hurt. - Cleaned up IP address parsing - Added tests for IPv6
@briansmith any chance to proceed with this PR? |
/// - https://bugs.chromium.org/p/chromium/issues/detail?id=308330 | ||
/// - https://bugzilla.mozilla.org/show_bug.cgi?id=1245280 | ||
pub(crate) fn iterate_names( | ||
subject: Option<untrusted::Input>, subject_alt_name: Option<untrusted::Input>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be misunderstanding how webpki, before this PR, is using subject
. webpki uses subject
ONLY to verify that the subject conforms to any directoryName constraints that may be present in the chain. It doesn't do any DNS name or IP address matching of the common name. And, as was mentioned in the initial review of this PR, we don't want to do any such processing of the common name. Instead we only want to look at iPAddress subjectAltNames.
Thus, unless I'm massively understanding what you're trying to accomplish in this PR, everything related to the subject and especially the subject's common name should be removed.
Note: I renamed the "master" branch to "main". Sorry for the inconvenience. This PR has had its base branch updated to "main" but you'll need to deal with the change in your local repo yourself. |
@@ -0,0 +1,10 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please include the script for generating the *.der files from the *.json files?
This PR adds support for IPv4 and IPv6 addresses in Subject Alternative Names (SAN) and CommonName (CN).
This PR is related to #120 and addresses #54 as well as many other issues from downstream projects which are waiting for webpki (respectively rustls) to add support for IP addresses. Based upon the machinery implemented by this PR we could also easily add support for Email address in a later stage.