Draft: Add SSLVHostSNIPolicy directive #561
Conversation
notroj
changed the title
|
We could actually eliminate all the runtime complexity by computing and storing a hash for each |
notroj
force-pushed
the
ssl-vhost-sni-policy
branch
from
e0b5a2f to
a504301
Compare
modules/ssl/ssl_engine_kernel.c
Outdated
| hash_ssl_params(a1, dig1); | ||
| hash_ssl_params(a2, dig2); | ||
|
|
||
| return strcmp((char *)dig1, (char *)dig2) == 0; |
There was a problem hiding this comment.
Are dig1 and dig2 null terminated strings that do not contain any nulls inside or do we need to do a memcmp(dig1, dig2, APR_MD5_DIGESTSIZE) instead of the strcmp?
modules/ssl/ssl_engine_kernel.c
Outdated
|
|
||
| apr_md5_update(&ctx, p->name, strlen(p->name)); | ||
| apr_md5_update(&ctx, "##", 2); | ||
| apr_md5_update(&ctx, p->value, strlen(p->value)); |
There was a problem hiding this comment.
Does a different ordering of the same key value pairs in the array mean a different configuration?
Won't the hashes be different if the arrays are the same but have a different ordering of the key value pairs?
Hence wouldn't we need to sort the arrays first prior to hashing? e.g.
- Transform the original array into an array that contains
name + '##' + valueas strings - Sort the new array
- Hash the new array as above
This sounds like a very good idea, but see my comments on the updated code. |
|
I'll try rewriting it to use hashes. For would have different semantics to the inverse order. |
If order matters than all should be good with the current approach. What about the |
|
Definitely will switch to using memcmp to compare the digests |
policy for SSL configuration compatibility between VirtualHosts.
SSLVHostSNIPolicy
strict => fail for any vhost mismatch
authonly => fail only for client verification/auth differences
secure => fail as authonly plus ciphersuite, protocol, keypair diff
insecure => allow everything
"secure" is the default.
PR: 69743
notroj
force-pushed
the
ssl-vhost-sni-policy
branch
from
a504301 to
15234fa
Compare
|
Switched to converting the digest to ASCII hex and using strcmp because it was easier for debugging for now. Still PoC but it passes my tests and the runtime code is a lot simpler now. 😉 |
|
Looks good |
3 participants
Draft: Add SSLVHostSNIPolicy directive, which sets a global policy for SSL configuration compatibility between VirtualHosts.
Usage:
SSLVHostSNIPolicy <policy>wherepolicymust be one of:strict=> fail for any vhost mismatchauthonly=> fail only for client verification/auth differencessecure=> fail as authonly plus ciphersuite, protocol, keypair differencesinsecure=> allow everything"secure" is the default.
PR: 69743