Skip to content

CurlHttpClient missing Server Name Indication when doing SSL handshake (Client Hello) #591

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

Closed
PoundKey opened this issue Jun 26, 2017 · 12 comments
Labels
feature-request A feature should be added or improved. help wanted We are asking the community to submit a PR to resolve this issue.

Comments

@PoundKey
Copy link

Hello AWS C++ SDK Team,

We're using the CurlHttpClient to communicate with our ADFS server, and when establishing the SSL connection, we realized that "Client Hello" does not include Server Name Indication in the header extension section.

Server Name Indication (Read more about SNI) is important when establishing SSL connection with the ADFS server.

And on OSX and Windows this piece of information is included in the Client Hello header.

A simple test application that I write to test and http client can be found on the link below, along with some Wireshark log and environment information.

HttpClientTest Source

#include <iostream>
#include <aws/core/Aws.h>
#include <aws/core/utils/StringUtils.h>
#include <aws/core/client/ClientConfiguration.h>
#include <aws/core/http/HttpClient.h>
#include <aws/core/http/HttpClientFactory.h>
#include <aws/core/http/HttpRequest.h>
#include <aws/core/http/HttpResponse.h>

using namespace Aws;
using namespace Aws::Client;
using namespace Aws::Utils;
using namespace Aws::Http;

namespace 
{
    static const char* ADFS_LOG_TAG = "PGOAdfsCredentialsProvider";

    // Additional configuration
    static const int MAX_REDIRECT_LIMIT = 10;
}


int main(int argc, char* argv[]) {
    std::cout << "Initializing HTTPClient..." << std::endl;

    SDKOptions options;
    InitAPI(options);

    ClientConfiguration config;
    
    config.followRedirects = false;
    config.verifySSL = false;

    std::shared_ptr<HttpClient> client = CreateHttpClient(config);
    String uri;
    
    if (argc >= 2) 
    {
 	uri = "https://" + String(argv[1]) + ":443/adfs/ls/IdpInitiatedSignOn.aspx?loginToRp=urn:amazon:webservices";
        
    } 
    else 
    {
	uri = "https://server.adfs.internal:443/adfs/ls/IdpInitiatedSignOn.aspx?loginToRp=urn:amazon:webservices";
    }
    
    std::cout << "Connecting to: " + uri << std::endl;

    std::shared_ptr<HttpRequest> request = CreateHttpRequest(
        URI(uri),
        HttpMethod::HTTP_GET,
        Stream::DefaultResponseStreamFactoryMethod);

    std::cout << "Sent HTTP GET request." << std::endl;
    std::shared_ptr<HttpResponse> response = client->MakeRequest(*request);

    if (!response || response->GetResponseCode() != HttpResponseCode::OK)
    {
        String message;
        if (response)
        {
            message = "Response code: " + std::to_string(static_cast<int>(response->GetResponseCode()));
        }

        std::cout << "[GET] Failed to send request to ADFS. " << message << std::endl;
    }

    std::cout << "Finished testing HttpCLient." << std::endl;
}

Thanks for the help in advance

@JonathanHenson
Copy link
Contributor

what version of libcurl are you using? SNI support was added in 7.18.1.

Also what TLS stack is your libcurl compiled and linked against? OpenSSL, GnuTLS, libNSS etc....

@PoundKey
Copy link
Author

Hello,

This is the environment that I used to compile the CPP SDK:

Compiler requirement: GCC 4.9

AWS C++ SDK version: 1.0.103
OpenSSL version: 1.0.2j
LibCurl version: 7.44.0
Zlib version: 1.2.8

I have also tried using the default OpenSSL, LibCurl and ZLIB specified in build_external.cmake by adding -DBUILD_OPENSSL -DBUILD_CURL and -DBUILD_ZLIB flags when compiling the SDK. Same issue can be reproduced (Missing SNI)

Thanks.

@JonathanHenson
Copy link
Contributor

JonathanHenson commented Jun 28, 2017 via email

@PoundKey
Copy link
Author

Hello Jonathan,

Could you elaborate more on "specifying a host header"?
Do you mean when sending out the HTTP request?

Thanks.

@JonathanHenson
Copy link
Contributor

My guess is that config.verifySSL = false; is turning off SNI on that platform.

If you run curl directly do you see the SNI information being exchanged?

@PoundKey
Copy link
Author

Hello Jonathan,

Yep, runing CURL directly on a Ubuntu/CentOS machine will work.
(OpenSSL version: 1.0.2j, LibCurl version: 7.44.0, Zlib version: 1.2.8)

config.verifySSL is turned off because the server we're trying to GET/POST against is sometimes self-signed and in this case users would still be able to interact with the server.

I can try turn it on and give you another update soon.
It just that the same code and configuration can be ran on OSX platform without any problem, whereas both Linux and OSX should be using the same CurlHttpClient. This is a bit confusing.

Thanks.

@JonathanHenson
Copy link
Contributor

JonathanHenson commented Jul 10, 2017 via email

@PoundKey
Copy link
Author

That I am pretty sure. We statically linked aws-sdk and libcurl against OpenSSL 1.0.2j.

@PoundKey
Copy link
Author

PoundKey commented Jul 10, 2017

Also, there might be a bug in the WinHttpSyncHttpClient.
File link: HERE

Line #99

if (!WinHttpSetOption(GetOpenHandle(), WINHTTP_OPTION_SECURITY_FLAGS, &flags, sizeof(flags))) {
     AWS_LOGSTREAM_FATAL(GetLogTag(), "Failed to turn ssl cert ca verification off.");
}

WINHTTP_OPTION_SECURITY_FLAGS cannot be set using the Session handle returned from GetOpenHandle(). According this the official API HERE. In this case, the code block should always return true.

It can be set using the RequestHandle, which is similar to function void* WinHttpSyncHttpClient::OpenRequest Line# 149

if (!WinHttpSetOption(hHttpRequest, WINHTTP_OPTION_DISABLE_FEATURE, &requestFlags, sizeof(requestFlags))) {
   AWS_LOGSTREAM_FATAL(GetLogTag(), "Failed to turn off redirects!");
}

Thanks

@PoundKey
Copy link
Author

PoundKey commented Aug 3, 2017

Hello Jonathan,

Basically CurlHttpClient cannot be used for any website that requires HTTPS and server certificate verification due to this SNI issue, unless that website has a fallback default server certificate (In this case missing SNI in the request header is fine).
We tried write our own code, linking against the same libcurl, openssl and libz and SNI is set properly in the request header.

config.verifySSL being turned on or off should not not be related to this issue.

I think if you write a simple code that makes a HTTPS GET request to any any HTTPS server, this issue can be reproduced again.

Thanks

@JonathanHenson
Copy link
Contributor

JonathanHenson commented Aug 3, 2017 via email

@PoundKey
Copy link
Author

PoundKey commented Aug 10, 2017

Hello Jonathan,

After further investigation, we realized that this really has to do with the way that we build Libcurl incorrectly. (SNI is only supported in curl 7.18.1 and after).

Sorry for the false positive alert, and thank you for your help along the way.

This ticket can be closed.

Thanks.

@justnance justnance added feature-request A feature should be added or improved. help wanted We are asking the community to submit a PR to resolve this issue. and removed enhancement labels Apr 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. help wanted We are asking the community to submit a PR to resolve this issue.
Projects
None yet
Development

No branches or pull requests

5 participants