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 a method to collect the DNS names from a certificate #91

Closed
wants to merge 1 commit into from

Conversation

seanmonstar
Copy link

This continues on the work from #65, and adds what seems to have been missing per the review:

  • Added a test that a certificate that contains wildcards in its Subject Alternative Name list has the wildcard skipped when returning the list.
  • I also changed the behavior for the test invalid_subject_alternative_names. Instead of returning a BadDER error because one name wasn't a valid DNSName, those names are just skipped instead.

Closes #64

@seanmonstar
Copy link
Author

r? @briansmith

@seanmonstar
Copy link
Author

Looks like CI failures are spurious apt-get errors?

@Geal
Copy link

Geal commented Mar 22, 2019

Why skip wildcard names? This is the kind of information I'd like to gather from a certificate. Or is there another way to get it?

@briansmith
Copy link
Owner

Why skip wildcard names? This is the kind of information I'd like to gather from a certificate. Or is there another way to get it?

I support adding support for wildcards as long as non-wildcard and wildcard names are explicitly distinguished in the type system, e.g. adding a WildcardDnsName type and then having this kind of functionality return an enum GeneralDnsName { DnsName(DnsName), WildCard(WildCardDnsName) }.

Base automatically changed from master to main January 14, 2021 01:31
@briansmith
Copy link
Owner

Closing this in favor of PR #103. PLMK if this was the wrong thing to do.

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.

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.

Add method to get all the names from a certificate
3 participants