advancedtls: populate verified chains when using custom buildVerifyFunc

[mudhireddy]

This request is to address the issue opened #7176

Basically this patch will set the verifiedChains correctly when advancedtls package is used with buildVerifyFunc() and applications need to access TLSInfo.State.VerifiedChains in their logic.

RELEASE NOTES: none

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mudhireddy / name: Ramesh M (188bbcc, 54a29d7, ce997a6, 6653a37, f512e23, 8147924, 01160d4, 39011ab)

[mudhireddy]

My local testing shows it ok though

grpc-go/test/xds @ramesh-937457-0.sjc% go test
PASS
ok google.golang.org/grpc/test/xds 11.131s

Is there a way to trigger the re-test of it?

[mudhireddy]

@purnesh42H any documentation on who and how to add LABEL, MILESTONE so that all checks on this pull request will pass ? right now Mergeable is failing for missing LABEL, MILESTONE

[purnesh42H]

@mudhireddy I have assigned it to myself. I will discuss with other maintainers and update you back. Thanks!

[gtcooke94]

Thanks for the PR! In general looks good, one big comment about where we set the value.
I'm reasoning through a few things on the overall code flow to make sure this wouldn't have any risk of leaking cert chains around if someone used the same credential with multiple connections - I'll update here when I'm done looking through that.

In the meanwhile, could you add tests that validate the expected behaviors here?
Also just curious, what is your use case for this?

[mudhireddy]

Thanks for the PR! In general looks good, one big comment about where we set the value. I'm reasoning through a few things on the overall code flow to make sure this wouldn't have any risk of leaking cert chains around if someone used the same credential with multiple connections - I'll update here when I'm done looking through that.

In the meanwhile, could you add tests that validate the expected behaviors here? Also just curious, what is your use case for this?

Added tests. Reg use case we use the CommonName details from VerifiedChains

[gtcooke94]

This looks good, thanks again for the PR!
We'll get one more reviewer to check it out

[gtcooke94]

@dfawley can you review this or assign to another go team member?

[mudhireddy]

@dfawley @gtcooke94, can I please get approval for this PR to merge ?

Q above

[gtcooke94]

Giving the type a name makes the code a little more readable, I think it's a good change

[gtcooke94]

Sorry meant to not approve yet because I don't want the shortened name

Didn't mean to approve

[gtcooke94]

I think this is looking good - it would be nice to be able to replace and use CertificateChains everywhere, but we could potentially just do that in a separate PR

[mudhireddy]

Thank you guys for the review and approval.

@gtcooke94, I see that it is still open, is it going to auto merge or do I need to do anything for it to merge ?