Sentry JWKS validator: add support for passing root CA certs

[ItalyPaleAle]

Allows fetching a JWKS from a HTTPS server that uses a custom CA certificate

[JoshVanL]

Moving to v1.14 milestone as we are beyond v1.13 feature freeze and imminently going into code freeze.

[ItalyPaleAle]

This is a P0 for Microsoft for 1.13. It's only 5 LOCs if you exclude tests.

[JoshVanL]

@ItalyPaleAle this has not been discussed with the project more widely and cannot be pushed through without consensus. Please create an issue as to why we need an exception to be included and the bug this is fixing. New features will not be accepted at this stage.

[ItalyPaleAle]

@ItalyPaleAle this has not been discussed with the project more widely and cannot be pushed through without consensus. Please create an issue as to why we need an exception to be included and the bug this is fixing. New features will not be accepted at this stage.

@mukundansundar is aware and agreed.

Reason is pretty self-evident and it's related to security, we need to set the CA cert for when fetching a JWKS from a HTTPS endpoint. It's required for security otherwise the only option is HTTP, which doesn't allow authenticating the origin server, so the JWKS could be spoofed.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (38b0511) 64.42% compared to head (1535370) 64.43%.

❗ Current head 1535370 differs from pull request most recent head 3106a14. Consider uploading reports for the commit 3106a14 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7366      +/-   ##
==========================================
+ Coverage   64.42%   64.43%   +0.01%     
==========================================
  Files         238      238              
  Lines       21833    21833              
==========================================
+ Hits        14066    14069       +3     
+ Misses       6560     6553       -7     
- Partials     1207     1211       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yaron2]

I agree that this is a feature that needs an exception and should be treated as such. We should always try to bring these up ahead of time. With that, judging by the small change-set and scope of the PR and coupled with the fact it improves our security posture, I support accepting this into the 1.13 release and have no issues getting this merged.

[ItalyPaleAle]

Thank you @yaron2.

I agree that sooner would have been better. We didn't realize this was needed this till yesterday.

[artursouza]

I agree to accept this change. I also recommend vendors to use internal fork to have their urgent needs met without the urge to workaround the OSS release process.