Fixes #148 #149
Fixes #148 #149
Conversation
| @@ -105,11 +105,19 @@ def validate_host(cert, name): | |||
| ext = cert.get_extension(i) | |||
| if ext.get_short_name() == b'subjectAltName': | |||
| s = str(ext) | |||
There was a problem hiding this comment.
Please refactor this code: extract logic inside this if branch into its own method is_san_matching(san: str, host_name: str): bool.
Also add unit tests for that method for following conditions:
- exact SAN match
- wildcard SAN match
- SAN mismatch
There was a problem hiding this comment.
I added a number of tests in tls_san_test.py, I hope thats correct this way?
src/pytds/tls.py
Outdated
| # SANs are usually have form like: DNS:hostname | ||
| if dnsentry.startswith('DNS:') and s[4:] == s_name: | ||
| return True | ||
| if dnsentry.startswith('DNS:*.'): # support for wildcards, but only at the first position |
There was a problem hiding this comment.
It would be better to extract SAN host name suffix before previous condition, and then check host name for exact match and for wildcard match. Instead of checking for DNS: prefix twice
There was a problem hiding this comment.
I did that refactoring, hope it's fine now
Codecov Report
@@ Coverage Diff @@
## master #149 +/- ##
==========================================
+ Coverage 91.73% 91.75% +0.02%
==========================================
Files 27 28 +1
Lines 7743 7763 +20
==========================================
+ Hits 7103 7123 +20
Misses 640 640
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Thank you! Merged |
|
Thank you! |
|
Sure, I have one more PR pending, I will make a release after it is merged too: #151 |
Fixes for #148