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

Support for IPv4 and IPv6 #122

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alex-berger
Copy link

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.

@Darkspirit
Copy link

CommonName is deprecated and should (must) no longer be used for validation:
https://groups.google.com/a/chromium.org/d/msg/security-dev/IGT2fLJrAeo/csf_1Rh1AwAJ
https://bugs.chromium.org/p/chromium/issues/detail?id=308330
https://bugzilla.mozilla.org/show_bug.cgi?id=1245280
FiloSottile/mkcert#205 (comment)

@alex-berger
Copy link
Author

alex-berger commented Mar 12, 2020

@Darkspirit Thanks for pointing this out. This poses the following questions:

  • Should we ignore the common name (CN) completely?
  • Or should we only ignore it for certificates issued after a certain date?
  • Does this only apply to the CN or also to SAN of type DirectoryName?

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
@alex-berger alex-berger changed the title [WIP] - Support for IPv4 and IPv6 Support for IPv4 and IPv6 Mar 13, 2020
@alex-berger
Copy link
Author

@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>,
Copy link
Owner

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.

@briansmith
Copy link
Owner

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 @@
{
Copy link
Owner

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?

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