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

cmake options USE_TLS_V1_2 and USE_TLS_V1_3 have no effect #2662

Closed
tmajkowski-ot opened this issue Sep 8, 2023 · 3 comments
Closed

cmake options USE_TLS_V1_2 and USE_TLS_V1_3 have no effect #2662

tmajkowski-ot opened this issue Sep 8, 2023 · 3 comments

Comments

@tmajkowski-ot
Copy link

Describe the bug

The recently added cmake configure options USE_TLS_V1_2 and USE_TLS_V1_3 have no effect
#2604

There are three issues:

  1. cmake -L lists USE_TLS_V1_2 and USE_TLS_V1_3 options, which have no effect as CMakeLists.txt checks for USE_TLS_V2 and USE_TLS_V3
  2. the conditionally compiled code in CurlHttpClient.cpp has no effect with current versions of curl 8 since LIBCURL_VERSION_MINOR would be <34. For curl version range checks LIBCURL_VERSION_NUM might be a better choice https://curl.se/docs/versions.html
  3. CURL_SSLVERSION_TLSv1_3 is available as of curl 7.52 according to curl documentation https://curl.se/libcurl/c/CURLOPT_SSLVERSION.html

Expected Behavior

using the cmake options should enforce the desired TLS version for the selected client

Current Behavior

cmake options USE_TLS_V1_2 and USE_TLS_V1_3 have no effect

Reproduction Steps

cmake <path_to_sdk_source> -DCMAKE_INSTALL_PREFIX=<install_dir> -DBUILD_ONLY=core -DENABLE_TESTING=OFF -DBUILD_SHARED_LIBS=ON -DFORCE_CURL=ON -DCURL_LIBRARY=<path_to_curl_lib> -DCURL_INCLUDE_DIR=<path_to_curl_include> -DUSE_TLS_V1_3=ON
cmake --build . --config=Debug

void UploadFile(const std::string& access_key_id, const std::string& secret_key, const std::string bucket_name, const std::string key_name, std::shared_ptrstd::fstream file_stream)
{
Aws::S3::S3ClientConfiguration config;
config.scheme = Aws::Http::Scheme::HTTPS;
config.region = "us-east-2";

Aws::S3::S3Client client(Aws::Auth::AWSCredentials(access_key_id, secret_key), config, Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::RequestDependent, false);

Aws::S3::Model::PutObjectRequest put_object_request;
put_object_request.SetBucket(bucket_name);
put_object_request.SetKey(key_name);
put_object_request.SetBody(file_stream);
auto outcome(client.PutObject(put_object_request));

std::cout << "Put " << (outcome.IsSuccess() ? "succeeded" : "failed") << std::endl;
}

put brake point before #if defined(ENFORCE_TLS_V1_3) in CurlHttpClient.cpp and see that CURL_SSLVERSION_TLSv1_3 option is not set. Repeat the same for V1_2.

Possible Solution

No response

Additional Information/Context

No response

AWS CPP SDK version used

1.11.159

Compiler and Version used

Visual Studio 2022

Operating System and version

windows 10

@tmajkowski-ot tmajkowski-ot added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 8, 2023
@sbiscigl sbiscigl mentioned this issue Sep 8, 2023
11 tasks
@sbiscigl
Copy link
Contributor

sbiscigl commented Sep 8, 2023

Hey sorry about that, pushing out a change that should address your problems

@yasminetalby yasminetalby added p2 This is a standard priority issue pending-release This issue will be fixed by an approved PR that hasn't been released yet. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 9, 2023
@sbiscigl
Copy link
Contributor

sbiscigl commented Sep 11, 2023

closing issue as it was released and tagged give a shout if you have anymore questions

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@sbiscigl sbiscigl removed bug This issue is a bug. pending-release This issue will be fixed by an approved PR that hasn't been released yet. p2 This is a standard priority issue labels Sep 11, 2023
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

No branches or pull requests

3 participants