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

Expose GeneralDnsNameRef #50

Closed

Conversation

moparisthebest
Copy link

This is a continuation of the conversation started here, basically I'm going to have thousands of certificates, and I need to look up the right one when a new connection comes in (by reading SNI from the connection).

Looping through thousands of certs and calling EndEntityCert.html.verify_is_valid_for_dns_name on each would be way too slow, I need to look them up via a hashmap, but that means I need to know the difference between a FQDN and a wildcard cert.

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #50 (f3390d8) into main (15acd62) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
- Coverage   93.78%   93.77%   -0.01%     
==========================================
  Files          14       13       -1     
  Lines        2573     2572       -1     
==========================================
- Hits         2413     2412       -1     
  Misses        160      160              

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

So wouldn't you also want to expose WildcardDnsNameRef?

I can't think of any reason not to expose this stuff, so I guess I'd be fine with merging this.

@cpu
Copy link
Member

cpu commented May 24, 2023

So wouldn't you also want to expose WildcardDnsNameRef?

@moparisthebest Do you still need this? Can you reply to the above?

@cpu
Copy link
Member

cpu commented May 31, 2023

Closing this one while there are outstanding questions + conflicts. If it's rebased and the requirements clarified we can reopen. Thanks

@cpu cpu closed this May 31, 2023
@hawkw
Copy link
Contributor

hawkw commented Sep 1, 2023

I would also like this type exposed --- currently, the EndEntityCert::dns_names() method being public is not particularly useful for downstream code. EndEntityCert::dns_names() as it returns an iterator of GeneralDnsNameRefs, which user code can't use. So, there's not actually a way for user code to iterate over the DNS names in an end-entity cert.

I'm happy to rebase this branch and open a new PR, if maintainers are willing to merge it.

@cpu
Copy link
Member

cpu commented Sep 1, 2023

I'm happy to rebase this branch and open a new PR, if maintainers are willing to merge it.

👍 Sounds good to me. It would be nice to get that in for 0.102 (#159)

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.

4 participants