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

Unhandled OOM encountered in aws_mem_acquire with allocatorAborted #2556

Closed
csi-amolpawar opened this issue Jun 28, 2023 · 16 comments
Closed
Assignees
Labels
bug This issue is a bug. closed-for-staleness p2 This is a standard priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days.

Comments

@csi-amolpawar
Copy link

csi-amolpawar commented Jun 28, 2023

Describe the bug

The crash is observed when Aws::S3Crt::ClientConfiguration.ca_file is set to ca certs dir path.

Expected Behavior

It should not crash and work as below

  • Should process the request successfully if ca_file assigned dir has the valid certificates
  • Should fail if ca_file assigned dir has the invalid/self signed certificates

Current Behavior

Observed the crash with below message

[root@906f057decec aws]# ./aws_s3_crt_w_ca_file_as_dir 
Unhandled OOM encountered in aws_mem_acquire with allocatorAborted

Reproduction Steps

This behaviour easily reproducible with below code snippet

#include <iostream>
#include <string>
#include <openssl/crypto.h>
#include <aws/core/Aws.h>
#include <aws/core/utils/memory/stl/AWSStringStream.h>
#include <aws/core/utils/logging/CRTLogSystem.h>
#include <aws/s3-crt/S3CrtClient.h>
#include <aws/s3-crt/model/GetObjectRequest.h>

static const char ALLOCATION_TAG[] = "s3_crt_ca_file_with_dir";

std::string get_default_openssl_dir()
{
  const std::string OPENSSLDIR_KEY("OPENSSLDIR: ");

  auto ssl_dir = std::string(SSLeay_version(SSLEAY_DIR));
  auto found = ssl_dir.find(OPENSSLDIR_KEY);
  if(found != std::string::npos)
  {
    ssl_dir = ssl_dir.substr(OPENSSLDIR_KEY.size());
    if(auto s = ssl_dir.size(); ssl_dir.at(0) == '"' && ssl_dir.at(s - 1) == '"')
      ssl_dir = ssl_dir.substr(1, s -2);
  }
  return ssl_dir;
}

int main(int argc, char* argv[])
{
  Aws::SDKOptions options;
  Aws::InitAPI(options);
  {    
    Aws::S3Crt::ClientConfiguration config;
    config.region = Aws::Region::US_EAST_1;

    config.verifySSL = false;
    config.caFile = Aws::String("/tmp/certs");
    config.caPath = Aws::String(get_default_openssl_dir());

    Aws::S3Crt::S3CrtClient s3CrtClient(config, Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::RequestDependent);
    Aws::String bucket("my_bucket");
    Aws::String objectKey("test/my_object");

    Aws::S3Crt::Model::GetObjectRequest request;
    request.SetBucket(bucket);
    request.SetKey(objectKey);
    
    if(auto outcome = s3CrtClient.GetObject(request); outcome.IsSuccess())
      std::cout << outcome.GetResult().GetBody().rdbuf() << std::endl;
    else
      std::cerr << "GetObject error:" << outcome.GetError().GetMessage() << std::endl;
  }
  Aws::ShutdownAPI(options);
  return 0;
}

Possible Solution

NA

Additional Information/Context

NA

AWS CPP SDK version used

aws-cpp-sdk-1.11.100

Compiler and Version used

g++ (GCC) 11.2.0

Operating System and version

Linux 906f057decec 5.14.0-1057-oem #64-Ubuntu SMP Mon Jan 23 17:02:19 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

@csi-amolpawar csi-amolpawar added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 28, 2023
@jmklix jmklix self-assigned this Jun 30, 2023
@jmklix
Copy link
Member

jmklix commented Jul 1, 2023

Thanks for opening this issue. Can you provide the cmake file that you are using with the code you posted above?

@jmklix jmklix added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2023
@github-actions
Copy link

github-actions bot commented Jul 3, 2023

Greetings! It looks like this issue hasn’t been active in longer than a week. We encourage you to check if this is still an issue in the latest release. Because it has been longer than a week since the last update on this, and in the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or add an upvote to prevent automatic closure, or if the issue is already closed, please feel free to open a new one.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jul 3, 2023
@csi-amolpawar
Copy link
Author

@jmklix Please find the build command

# g++ --version
g++ (GCC) 11.2.0
# g++ -std=c++20 -g -o aws_s3_crt_w_ca_file_as_dir aws_s3_crt_w_ca_file_as_dir.cpp -I<Install Prefix Path>/include -L<Install Prefix Path>/lib64 -laws-cpp-sdk-s3-crt -laws-cpp-sdk-core -lssl -lcrypto -lpthread

@csi-amolpawar
Copy link
Author

csi-amolpawar commented Jul 3, 2023

Will retest with latest available version and update

@csi-amolpawar
Copy link
Author

csi-amolpawar commented Jul 3, 2023

I've retested this issue with latest aws-sdk-cpp verion 1.11.112 and is reproducible

[root@906f057decec aws]# ./aws_s3_crt_w_ca_file_as_dir 
Initialised Aws API
Initialised config for ca_file =/tmp/certs/ and ca_path=/etc/pki/tls
Unhandled OOM encountered in aws_mem_acquire with allocatorAborted (core dumped)

when tested with ca.crt file , it returns the exception

[root@906f057decec aws]# ./aws_s3_crt_w_ca_file_as_dir 
Initialised Aws API
Initialised config for ca_file =/tmp/certs/ca.crt and ca_path=/etc/pki/tls
Prepared Get Object Request
GetObject error:crtCode: 1029, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE - TLS (SSL) negotiation failed
Shutdown Aws API

@jmklix jmklix removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. labels Jul 3, 2023
@jmklix
Copy link
Member

jmklix commented Jul 11, 2023

I've retested this issue with latest aws-sdk-cpp verion 1.11.112 and is reproducible

[root@906f057decec aws]# ./aws_s3_crt_w_ca_file_as_dir 
Initialised Aws API
Initialised config for ca_file =/tmp/certs/ and ca_path=/etc/pki/tls
Unhandled OOM encountered in aws_mem_acquire with allocatorAborted (core dumped)

What certificate is your example using for the above test? Did you put it inside the /tmp/certs/ folder

when tested with ca.crt file , it returns the exception

[root@906f057decec aws]# ./aws_s3_crt_w_ca_file_as_dir 
Initialised Aws API
Initialised config for ca_file =/tmp/certs/ca.crt and ca_path=/etc/pki/tls
Prepared Get Object Request
GetObject error:crtCode: 1029, AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE - TLS (SSL) negotiation failed
Shutdown Aws API

How are you generating your ca.crt for this second failure?

@jmklix jmklix added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jul 11, 2023
@csi-amolpawar
Copy link
Author

@jmklix

The caPath=/etc/pki/tls is a system default path which has the trusted certs and caFile=/tmp/certs/ca.crt is set with self signed cert path

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Jul 12, 2023
@jmklix
Copy link
Member

jmklix commented Aug 31, 2023

Do the certs come pre-loaded on the device that you are using?
How are you generating your ca.crt for the second failure?

@jmklix jmklix added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Aug 31, 2023
@csi-amolpawar
Copy link
Author

@jmklix The ca.crt is a self signed certificate which come pre-loaded in our env.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Sep 2, 2023
@DmitriyMusatkin
Copy link
Contributor

caFile expects a file and passing a directory is not a supported behavior. That option is modeled after similar curl option, CURLOPT_CAINFO (if curl is http client, sdk should be passing caFile directly to that curl option), and we are unlikely to make any modifications to how it works.
With that said, passing directory resulting in crash is a bug and it has been fixed in awslabs/aws-c-common#1061, which made it to cpp sdk in 1.11.184. With the fix in place client fails to initialize and error about invalid path is printed to logs.

Second error seems to be due to s3-crt client not respecting verifySSL setting being set to false, as s3 endpoint should fail to validate against self signed cert otherwise? That was fixed in 1.11.218 and should work now as expected.

@DmitriyMusatkin DmitriyMusatkin added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Apr 24, 2024
@csi-amolpawar
Copy link
Author

csi-amolpawar commented Apr 24, 2024

@DmitriyMusatkin Thanks for the update, will check this with latest SDK version and will update you.

@csi-amolpawar
Copy link
Author

csi-amolpawar commented Apr 24, 2024

I've checked this with latest SDK version 1.11.313 and confirming that received different error than the previous Unhandled OOM encountered in aws_mem_acquire with allocatorAborted (core dumped) crash
See below

[root@dab9599299cb aws]# ./aws_s3_crt_w_ca_file_as_dir
Initialised Aws API
Initialised config for ca_file =/tmp/certs and ca_path=/etc/pki/tls verifySSL=1
Prepared Get Object Request
GetObject error:Client is not initialized or already terminated
Shutdown Aws API

@DmitriyMusatkin
Copy link
Contributor

Yes, thats expected. As i mentioned dir path is not valid for ca_file, and with the latest version it will correctly propagate the error and not initialize the client - GetObject error:Client is not initialized or already terminated indicates that. Logs should have an error about file path not being valid.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Apr 25, 2024
@csi-amolpawar
Copy link
Author

csi-amolpawar commented Apr 25, 2024

yes, could see the actual reason in the log file

[2024/04/25 14:07:17:068 UTC] [140575882405440] ERROR load <aws_s3_utils.cpp:104> 1723 [ERROR] 2024-04-25 14:07:17.068 common-io [140575882405440] static: Failed reading file:'/opt/config/' errno:21 aws-error:AWS_ERROR_FILE_INVALID_PATH

@jmklix
Copy link
Member

jmklix commented Apr 25, 2024

When you use the correct file path are you able to get this working?

@jmklix jmklix added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Apr 25, 2024
Copy link

github-actions bot commented May 6, 2024

Greetings! It looks like this issue hasn’t been active in longer than a week. We encourage you to check if this is still an issue in the latest release. Because it has been longer than a week since the last update on this, and in the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or add an upvote to prevent automatic closure, or if the issue is already closed, please feel free to open a new one.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. closed-for-staleness p2 This is a standard priority issue response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days.
Projects
None yet
Development

No branches or pull requests

3 participants