tls: Account for a change in BoringSSL verify depth semantics (#31287)
Prior to OpenSSL 1.1.0, the certificate depth limit in OpenSSL omitted the leaf but included the trust anchor. That is, if your chain was Leaf, Intermediate, Root, any depth limit of 2 or more allowed the certificate. OpenSSL 1.1.0 included d9b8b89bec4480de3a10bdaf9425db371c19145b, which was described as a cleanup change to X509_verify_cert. However, this change the semantics of the depth limit to omit *both* the leaf and trust anchor. So the example above was accepted also at depth limit 1. This is also why common.proto had a comment about different semantics between the libraries. BoringSSL originally forked a little before 1.0.2, so it had the older OpenSSL behavior. Now that the new behavior has been in OpenSSL upstream for a while, BoringSSL plans to match the new behavior in https://boringssl-review.googlesource.com/c/boringssl/+/64707/ This change makes Envoy compatible with BoringSSLs before and after that change. When BORINGSSL_API_VERSION is new enough, we adjust the value before passing it in, to preserve the original semantics. I'm assuming here that Envoy would prefer to maintain its existing semantics, rather than change the test expectation. I've also removed the comment about backend-specific behavior difference. Supposing Envoy prefers to maintain existing semantics, any OpenSSL port of Envoy should similarly adjust the value on OpenSSL 1.1.0 and up. Along the way, fix an overflow. maxVerifyDepth is a uint32_t, but the OpenSSL API takes an int. When we exceed INT_MAX, saturate the cast. Signed-off-by: David Benjamin <davidben@google.com> Mirrored from https://github.com/envoyproxy/envoy @ f7ef1eeca94f714f0d48af3dd8a43757dc63d770main
parent
674de4c962
commit
dd03497769
1 changed files with 4 additions and 6 deletions
Loading…
Reference in new issue