Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding verifyMode param to safeRequest #74

Merged
merged 1 commit into from
Aug 2, 2024
Merged

adding verifyMode param to safeRequest #74

merged 1 commit into from
Aug 2, 2024

Conversation

miki725
Copy link
Contributor

@miki725 miki725 commented Jul 31, 2024

docker in some cases needs to make a call via TLS however not validate the certificate

@miki725 miki725 requested a review from viega as a code owner July 31, 2024 14:00
ee7
ee7 previously requested changes Aug 1, 2024
Copy link

@ee7 ee7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new getSSLContext parameter is unused, so the CVerifyPeer mode is always used even if the caller tries to use a different mode. This PR writes:

context = getSSLContext(caFile = pinnedCert, verifyMode = verifyMode)

but then it only uses the CVerifyPeer mode because that's hardcoded inside getSSLContext.

nimutils/net.nim Show resolved Hide resolved
docker in some cases needs to make a call via TLS however not validate
the certificate
@ee7 ee7 self-requested a review August 1, 2024 13:34
Copy link

@ee7 ee7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR added verifyMode as a new parameter for createHttpClient, but did not alter newAwsClient proc, which uses createHttpClient:

proc newAwsClient*(creds: AwsCredentials, region,
service: string): AwsClient =
let
# TODO - use some kind of template and compile-time variable to put the correct kernel used to build the sdk in the UA?
httpclient = createHttpClient(
userAgent = "nimaws-sdk/0.3.3; " & defUserAgent.replace(" ", "-").toLower() & "; darwin/16.7.0",
)
scope = AwsScope(date: getAmzDateString(), region: region, service: service)
return AwsClient(httpClient: httpclient, credentials: creds, scope: scope,
key: "", key_expires: getTime())

Should we add a verifyMode parameter to newAwsClient?

@ee7 ee7 dismissed their stale review August 1, 2024 13:53

Resolved.

@miki725
Copy link
Contributor Author

miki725 commented Aug 1, 2024

aws client always needs to verify its peer. no need to customize it

@miki725 miki725 merged commit 7210eca into dev Aug 2, 2024
2 checks passed
@miki725 miki725 deleted the noverify branch August 2, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants