Add IMDSv2 support to AwsCandidateHarvester

jitsi / ice4j

A Java implementation of the ICE protocol

Apache License 2.0

437 stars 232 forks source link

Add IMDSv2 support to AwsCandidateHarvester #294

Closed dsmeytis closed 2 days ago

dsmeytis commented 1 month ago

Hello, AWS recommend to disable IMDSv1 support for EC2 instances: https://aws.amazon.com/blogs/security/get-the-full-benefits-of-imdsv2-and-disable-imdsv1-across-your-aws-infrastructure/ However it breaks AwsCandidateHarvester functionality because REST endpoints it uses to detect EC2 and obtain private/public IPs become unavailable. I would like to propose the fix that will work for both IMDSv1 and IMDSv2:

doTestEc2() will now try to obtain an API token as EC2_TEST_URL is unavailable without a token.
added parameters to fetch() to set a method and pass required headers.

jitsi-jenkins commented 1 month ago

Hi, thanks for your contribution! If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

dsmeytis commented 1 month ago

already signed

bgrozev commented 4 days ago

Thanks for the contribution! I left a couple of minor requests. I'll run a manual test and let you know if I have more.

Confirmed CLA. Jenkins, please add to whitelist

bgrozev commented 4 days ago

Looks good after testing

dsmeytis commented 4 days ago

hi @bgrozev, thanks for the review! Actually for my deployment I ended up with the java.net.http.HttpClient implementation. If you wish I can push updated solution, otherwise I'll address your requests and we'll proceed with it.

bgrozev commented 4 days ago

Sure, unless it's a large change let's go with HttpClient

dsmeytis commented 3 days ago

@bgrozev please take a look at https://github.com/jitsi/ice4j/pull/297