add -mutual-tls-san option#874
Conversation
|
not sure if there is any interest in merging this, but, I needed this so I added it. |
Codecov Report
@@ Coverage Diff @@
## master #874 +/- ##
==========================================
- Coverage 56.55% 56.42% -0.13%
==========================================
Files 80 80
Lines 7113 7131 +18
==========================================
+ Hits 4023 4024 +1
- Misses 2634 2651 +17
Partials 456 456
Continue to review full report at Codecov.
|
|
@adamjacobmuller will try to go over the PR in the next day or two. |
|
Thanks @kisom! |
| } | ||
| server.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| if r != nil && r.TLS != nil && len(r.TLS.PeerCertificates) > 0 { | ||
| for _, name := range r.TLS.PeerCertificates[0].DNSNames { |
There was a problem hiding this comment.
The SAN list includes more than DNS names; if you look at the Certificate structure, it has
// Subject Alternate Name values
DNSNames []string
EmailAddresses []string
IPAddresses []net.IP
URIs []*url.URL
so this should probably also check more than the DNS names.
There was a problem hiding this comment.
Hi Kisom,
I'd considered this, but, should it?
I wasn't too sure what the use-case would be for validating based on those other attributes, but, validating based on the DNSNames is ultimately arbitrary.
If I updated to validate against all of them, do you consider this merge ready/worthy?
-Adam
There was a problem hiding this comment.
@adamjacobmuller Other than that, it looks fine.
similar to -mutual-tls-cn option, but allows the regular expression to match against any SAN on the supplied certificate
adamjacobmuller
force-pushed
the
master
branch
from
a35e01c to
4384373
Compare
3 participants
similar to -mutual-tls-cn option, but allows the regular expression to
match against any SAN on the supplied certificate