fix(aws): use BuildableClient insead of xhttp.Client

[DmitriyLewen]

Description

There is a problem with xhttp.Client — Trivy aws-sdk cannot connect to IMDS.
That is why some users cannot get credentials.

The aws docs recommend using BuildableClient — https://docs.aws.amazon.com/sdk-for-go/v2/developer-guide/configure-http.html

This PR changes xhttp.Client to BuildableClient with the insecure option.

Related issues

• Close bug(aws): Trivy can't connect to IMDS to get credentials #9437

Related PRs

• fix(image): use standardized HTTP client for ECR authentication #9322

Checklist

• I've read the guidelines for contributing to this repository.
• I've followed the conventions in the PR title.
• I've added tests that prove my fix is effective or that my feature works.
• I've updated the documentation with the relevant information (if needed).
• I've added usage information (if the PR introduces new options)
• I've included a "before" and "after" example to the description (if the PR is a user interface change).

[knqyf263]

@DmitriyLewen Did you figure out how BuidlableClient is different from xhttp.Client? I don't see any critical differences.
https://github.com/aws/aws-sdk-go-v2/blob/1cdc15880e2f5087470b6f813eb0a5cd1451c95f/aws/transport/http/client.go#L180-L198

[DmitriyLewen]

aws-sdk uses different values:
• KeepAlive == 30 sec (we use the default value — 15 sec)
• DualStack == true (but this field shouldn’t matter — // Deprecated: Fast Fallback is enabled by default.)
• MaxIdleConnsPerHost == 10 (we use the default value — 0, no limit)
• ForceAttemptHTTP2 == true
• TLSClientConfig.MinVersion == 771 (we use the default value — 0)

The other fields should match.
But I don’t know how this could affect it…
I can’t reproduce this case, so I can’t identify the field in Trivy’s transport that triggers the issue.

[knqyf263]

I'm confused. The default values I saw are different from the ones you mentioned. Am I missing something?

• KeepAlive == 30 sec (we use the default value — 15 sec)

Isn't the default value 30 sec in DefaultTransport?
https://github.com/golang/go/blob/6447ff409ac7e2a621bc8ca5c44b2eaed751fbaa/src/net/http/transport.go#L50

• MaxIdleConnsPerHost == 10 (we use the default value — 0, no limit)

Isn't it 2?
https://github.com/golang/go/blob/6447ff409ac7e2a621bc8ca5c44b2eaed751fbaa/src/net/http/transport.go#L59-L61

• ForceAttemptHTTP2 == true

We also set true.
https://github.com/golang/go/blob/6447ff409ac7e2a621bc8ca5c44b2eaed751fbaa/src/net/http/transport.go#L52

• TLSClientConfig.MinVersion == 771 (we use the default value — 0)

Also, the minimum version is TLS 1.2 by default.
https://github.com/golang/go/blob/6447ff409ac7e2a621bc8ca5c44b2eaed751fbaa/src/crypto/tls/common.go#L761-L768

[DmitriyLewen]

Isn't the default value 30 sec in DefaultTransport?
golang/go@6447ff4/src/net/http/transport.go#L50

We overwrite this value here:

trivy/pkg/x/http/transport.go

Lines 66 to 68 in aa5b32a

	d := &net.Dialer{
	Timeout: timeout,
	}

Isn't it 2?
golang/go@6447ff4/src/net/http/transport.go#L59-L61

Nice catch. You are right.
https://github.com/golang/go/blob/6447ff409ac7e2a621bc8ca5c44b2eaed751fbaa/src/net/http/transport.go#L1125-L1127

We also set true.
golang/go@6447ff4/src/net/http/transport.go#L52

Oops, my mistake.

Also, the minimum version is TLS 1.2 by default.
golang/go@6447ff4/src/crypto/tls/common.go#L761-L768

I don't know why i didn't check nested package.
You are right.

---

So looks like we have 2 difference fields - KeepAlive and MaxIdleConnsPerHost.

[knqyf263]

looks like we have 2 difference fields - KeepAlive and MaxIdleConnsPerHost

Hmm, but they don't appaear to fail authentication...

[DmitriyLewen]

Can CheckRedirect affects on this?
I found that they wrap Client:
https://github.com/aws/aws-sdk-go-v2/blob/648027edb8aeba036195538174a63cbccaca8c16/aws/transport/http/client.go#L83-L88
https://github.com/aws/aws-sdk-go-v2/blob/648027edb8aeba036195538174a63cbccaca8c16/aws/transport/http/client.go#L266-L305

[knqyf263]

I also discovered that, but since it seems to suppress following redirects (and it only happens with S3?), I thought it might not be related to the current issue. In any case, it’s difficult to fix without being able to reproduce it.

I don’t prefer fixing it without understanding the cause, as that could lead to the issue recurring in the future, but it might be acceptable to temporarily switch to BuildableClient, release it, and then investigate more thoroughly afterward.

[DmitriyLewen]

The users who got this issue are cooperating and trying to help us reproduce the problem.
There are only three of them, so it may be better not to merge this now.
If we don’t find a solution before the release, we’ll merge this PR.

On the other hand, this is the recommended AWS solution, and we’re busy with partner tasks.
So it may indeed make sense to merge this PR.
It’s up to you to decide.

[knqyf263]

By switching to BuildableClient, an easily anticipated issue is that the configurations in x/http will not be applied. The purpose of x/http is to ensure that the same configurations (such as insecure or mTLS settings) are used consistently across all locations in Trivy, but this will no longer be achieved with BuildableClient. Ideally, this auth problem should be fixed by modifying x/http if possible.

I hope to find time to debug it further.

[knqyf263]

I tried again but could not reproduce it. I will merge this PR for now and investigate further later when I have time.