envoy-mobile icon indicating copy to clipboard operation
envoy-mobile copied to clipboard

cert validation: use Android cert validation APIs

Open danzh2010 opened this issue 2 years ago • 14 comments

Description: add engine API to allow user config to use Android cert validation APIs. Risk Level: high Testing: added tests in Http2TestServerTest.java Docs Changes: Release Notes: Fixes #1575 Part of #2144

danzh2010 avatar Sep 06 '22 18:09 danzh2010

/retest

danzh2010 avatar Sep 06 '22 18:09 danzh2010

/retest

danzh2010 avatar Sep 07 '22 02:09 danzh2010

/retest

danzh2010 avatar Sep 07 '22 14:09 danzh2010

/retest

danzh2010 avatar Sep 07 '22 23:09 danzh2010

The java_tests_mac failure depends on the fixed mentioned in https://github.com/envoyproxy/envoy-mobile/pull/2516#issuecomment-1237169849.

danzh2010 avatar Sep 12 '22 19:09 danzh2010

/assign @ggreenway

danzh2010 avatar Sep 12 '22 19:09 danzh2010

/retest

danzh2010 avatar Sep 20 '22 20:09 danzh2010

/retest

danzh2010 avatar Sep 20 '22 22:09 danzh2010

/retest

danzh2010 avatar Sep 21 '22 14:09 danzh2010

@Augustyniak this is good for review.

danzh2010 avatar Sep 21 '22 14:09 danzh2010

/retest

danzh2010 avatar Sep 22 '22 15:09 danzh2010

This is very impressive, and overall looks like a good setup. Thank you, @danzh2010!

I'm not terribly concerned, just curious: is spawning a thread per outstanding cert validation something that Cronet does as well?

goaway avatar Sep 22 '22 19:09 goaway

I'm not terribly concerned, just curious: is spawning a thread per outstanding cert validation something that Cronet does as well?

Nope, AFAIK Cronet coalesce the validations based on host name and uses a thread pool. This will be the next step.

danzh2010 avatar Sep 23 '22 20:09 danzh2010

/retest

danzh2010 avatar Sep 23 '22 20:09 danzh2010

/retest

danzh2010 avatar Oct 05 '22 21:10 danzh2010

/retest

danzh2010 avatar Oct 13 '22 18:10 danzh2010

/retest

Augustyniak avatar Oct 13 '22 18:10 Augustyniak

@goaway can you re-review/approve this PR? I think that your concerns have been addressed

Augustyniak avatar Oct 13 '22 18:10 Augustyniak

Friendly ping on this :)

danzh2010 avatar Oct 17 '22 20:10 danzh2010

@goaway I am going to merge this PR as I think that your concerns with regard to configuration template have been addressed.

Augustyniak avatar Oct 18 '22 12:10 Augustyniak