grpc
security: Stabilize AdvancedTlsX509TrustManager.
This PR is a part of 'Stabilize Advanced TLS' effort. Clean up, improve javadoc, de-experimentalize of AdvancedTlsX509TrustManager, add a unit test (e2e already exists).
API review meeting notes:
CERTIFICATE_ONLY_VERIFICATION and INSECURELY_SKIP_ALL_VERIFICATION
Should document that these are dangerous to us, unless also specifying your own cert validation. Yes, one has INSECURELY in its name, but there should be some javadoc.
INSECURELY_SKIP_ALL_VERIFICATION
Mention that any loaded trust certs will be ignored. Yes, that's what it says, but just "those other methods that you use all the time stop doing anything" is helpful to point out.
- Add docs to say that after build, no trust certs are loaded. You need to call one of the update/usesystem methods.
- Builder needs more docs. For example, to explain that setSslSocketAndEnginePeerVerifier is called in addition to verifying certs as specified by Verification.
- Should make clear that it is normal for nothing in the builder needs to be called? That should be the common case. Maybe just improve the AdvancedTlsX509TrustManager javadoc with a code snippet?
The minimum refresh period of 1 minute is enforced.
"enforced" could mean several different things, including "causes an error." Probably want to tweak that to be more clear.
API review meeting notes:
CERTIFICATE_ONLY_VERIFICATION and INSECURELY_SKIP_ALL_VERIFICATION
Should document that these are dangerous to us, unless also specifying your own cert validation. Yes, one has INSECURELY in its name, but there should be some javadoc.
INSECURELY_SKIP_ALL_VERIFICATION
Mention that any loaded trust certs will be ignored. Yes, that's what it says, but just "those other methods that you use all the time stop doing anything" is helpful to point out.
- Add docs to say that after build, no trust certs are loaded. You need to call one of the update/usesystem methods.
- Builder needs more docs. For example, to explain that setSslSocketAndEnginePeerVerifier is called in addition to verifying certs as specified by Verification.
- Should make clear that it is normal for nothing in the builder needs to be called? That should be the common case. Maybe just improve the AdvancedTlsX509TrustManager javadoc with a code snippet?
The minimum refresh period of 1 minute is enforced.
"enforced" could mean several different things, including "causes an error." Probably want to tweak that to be more clear.
Done - I also reworded few comments before 'bumping up' to javadoc level. PTAL