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

OpenSSL 1.1.0 hangs (CPU pegged) when SSL_peek is used with TLSv1 #1563

Closed
alex opened this issue Sep 10, 2016 · 0 comments
Closed

OpenSSL 1.1.0 hangs (CPU pegged) when SSL_peek is used with TLSv1 #1563

alex opened this issue Sep 10, 2016 · 0 comments

Comments

@alex
Copy link
Contributor

alex commented Sep 10, 2016

Code to reproduce:

#include <assert.h>
#include <fcntl.h>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <stdbool.h>
#include <string.h>

#include <openssl/ssl.h>


char SERVER_KEY[] = "-----BEGIN RSA PRIVATE KEY-----\n"
"MIICWwIBAAKBgQC+pvhuud1dLaQQvzipdtlcTotgr5SuE2LvSx0gz/bg1U3u1eQ+\n"
"U5eqsxaEUceaX5p5Kk+QflvW8qdjVNxQuYS5uc0gK2+OZnlIYxCf4n5GYGzVIx3Q\n"
"SBj/TAEFB2WuVinZBiCbxgL7PFM1Kpa+EwVkCAduPpSflJJPwkYGrK2MHQIDAQAB\n"
"AoGAbwuZ0AR6JveahBaczjfnSpiFHf+mve2UxoQdpyr6ROJ4zg/PLW5K/KXrC48G\n"
"j6f3tXMrfKHcpEoZrQWUfYBRCUsGD5DCazEhD8zlxEHahIsqpwA0WWssJA2VOLEN\n"
"j6DuV2pCFbw67rfTBkTSo32ahfXxEKev5KswZk0JIzH3ooECQQDgzS9AI89h0gs8\n"
"Dt+1m11Rzqo3vZML7ZIyGApUzVan+a7hbc33nbGRkAXjHaUBJO31it/H6dTO+uwX\n"
"msWwNG5ZAkEA2RyFKs5xR5USTFaKLWCgpH/ydV96KPOpBND7TKQx62snDenFNNbn\n"
"FwwOhpahld+vqhYk+pfuWWUpQciE+Bu7ZQJASjfT4sQv4qbbKK/scePicnDdx9th\n"
"4e1EeB9xwb+tXXXUo/6Bor/AcUNwfiQ6Zt9PZOK9sR3lMZSsP7rMi7kzuQJABie6\n"
"1sXXjFH7nNJvRG4S39cIxq8YRYTy68II/dlB2QzGpKxV/POCxbJ/zu0CU79tuYK7\n"
"NaeNCFfH3aeTrX0LyQJAMBWjWmeKM2G2sCExheeQK0ROnaBC8itCECD4Jsve4nqf\n"
"r50+LF74iLXFwqysVCebPKMOpDWp/qQ1BbJQIPs7/A==\n"
"-----END RSA PRIVATE KEY-----";

char SERVER_CERT[] = "-----BEGIN CERTIFICATE-----\n"
"MIICKDCCAZGgAwIBAgIJAJn/HpR21r/8MA0GCSqGSIb3DQEBBQUAMFgxCzAJBgNV\n"
"BAYTAlVTMQswCQYDVQQIEwJJTDEQMA4GA1UEBxMHQ2hpY2FnbzEQMA4GA1UEChMH\n"
"VGVzdGluZzEYMBYGA1UEAxMPVGVzdGluZyBSb290IENBMCIYDzIwMDkwMzI1MTIz\n"
"NzUzWhgPMjAxNzA2MTExMjM3NTNaMBgxFjAUBgNVBAMTDWxvdmVseSBzZXJ2ZXIw\n"
"gZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAL6m+G653V0tpBC/OKl22VxOi2Cv\n"
"lK4TYu9LHSDP9uDVTe7V5D5Tl6qzFoRRx5pfmnkqT5B+W9byp2NU3FC5hLm5zSAr\n"
"b45meUhjEJ/ifkZgbNUjHdBIGP9MAQUHZa5WKdkGIJvGAvs8UzUqlr4TBWQIB24+\n"
"lJ+Ukk/CRgasrYwdAgMBAAGjNjA0MB0GA1UdDgQWBBS4kC7Ij0W1TZXZqXQFAM2e\n"
"gKEG2DATBgNVHSUEDDAKBggrBgEFBQcDATANBgkqhkiG9w0BAQUFAAOBgQBh30Li\n"
"dJ+NlxIOx5343WqIBka3UbsOb2kxWrbkVCrvRapCMLCASO4FqiKWM+L0VDBprqIp\n"
"2mgpFQ6FHpoIENGvJhdEKpptQ5i7KaGhnDNTfdy3x1+h852G99f1iyj0RmbuFcM8\n"
"uzujnS8YXWvM7DM1Ilozk4MzPug8jzFp5uhKCQ==\n"
"-----END CERTIFICATE-----";

void assert_errno(bool condition, char *msg) {
    if (!condition) {
        perror(msg);
        exit(1);
    }
}

void assert_openssl(bool condition) {
    if (!condition) {
        printf("OpenSSL error\n");
        exit(1);
    }
}

int get_listening_port(int listener) {
    struct sockaddr_in server_bound_addr;
    socklen_t len = sizeof(server_bound_addr);
    int result = getsockname(listener, (struct sockaddr *)&server_bound_addr, &len);
    assert_errno(result != -1, "getsockname");
    return ntohs(server_bound_addr.sin_port);
}

EVP_PKEY *load_privatekey(char *key_bytes) {
    BIO *bio = BIO_new_mem_buf(key_bytes, strlen(key_bytes));
    EVP_PKEY *key = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
    assert_openssl(key != NULL);
    return key;
}

X509 *load_certificate(char *cert_bytes) {
    BIO *bio = BIO_new_mem_buf(cert_bytes, strlen(cert_bytes));
    X509 *cert = PEM_read_bio_X509(bio, NULL, NULL, NULL);
    assert_openssl(cert != NULL);
    return cert;
}

void create_loopback(SSL **server, SSL **client) {
    int result;

    int listener = socket(AF_INET, SOCK_STREAM, 0);
    assert_errno(listener != -1, "socket");
    struct sockaddr_in server_addr;
    server_addr.sin_port = 0;
    server_addr.sin_addr.s_addr = INADDR_ANY;
    server_addr.sin_family = AF_INET;
    result = bind(listener, (struct sockaddr *)&server_addr, sizeof(server_addr));
    assert_errno(result != -1, "bind");
    result = listen(listener, 1);
    assert_errno(result != -1, "listen");

    int client_sock = socket(AF_INET, SOCK_STREAM, 0);
    assert_errno(client_sock != -1, "socket");
    struct sockaddr_in dest_addr;
    dest_addr.sin_port = htons(get_listening_port(listener));
    inet_aton("127.0.0.1", &dest_addr.sin_addr);
    dest_addr.sin_family = AF_INET;
    result = connect(client_sock, (struct sockaddr *)&dest_addr, sizeof(dest_addr));
    assert_errno(result != -1, "connect");

    int server_sock = accept(listener, NULL, NULL);
    assert_errno(server_sock != -1, "accept");

    SSL_CTX *server_ctx = SSL_CTX_new(TLSv1_method());
    assert_openssl(server_ctx != NULL);
    SSL_CTX_use_PrivateKey(server_ctx, load_privatekey(SERVER_KEY));
    SSL_CTX_use_certificate(server_ctx, load_certificate(SERVER_CERT));
    *server = SSL_new(server_ctx);
    assert_openssl(*server != NULL);
    SSL_set_fd(*server, server_sock);
    SSL_set_accept_state(*server);

    SSL_CTX *client_ctx = SSL_CTX_new(TLSv1_method());
    assert_openssl(client_ctx != NULL);
    *client = SSL_new(client_ctx);
    assert_openssl(*client != NULL);
    SSL_set_fd(*client, client_sock);
    SSL_set_connect_state(*client);
}

void set_blocking(SSL *s, bool blocking) {
    int fd = SSL_get_fd(s);
    int flags = fcntl(fd, F_GETFL, 0);
    assert_errno(flags != -1, "fcntl");
    if (blocking) {
        flags = flags & (~O_NONBLOCK);
    } else {
        flags |= O_NONBLOCK;
    }
    flags = fcntl(fd, F_SETFL, flags);
    assert_errno(flags != -1, "fcntl");
}

void handshake(SSL *client, SSL *server) {
    set_blocking(client, false);
    set_blocking(server, false);

    SSL *conns[] = {client, server};
    int nconns = 2;
    while (nconns) {
        for (size_t i = 0; i < 2; i++) {
            if (conns[i] == NULL) {
                continue;
            }
            int result = SSL_do_handshake(conns[i]);
            int error = SSL_get_error(conns[i], result);
            if (error == SSL_ERROR_NONE) {
                conns[i] = NULL;
                nconns--;
            } else if (error != SSL_ERROR_WANT_READ) {
                assert_openssl(false);
            }
        }
    }

    set_blocking(client, true);
    set_blocking(server, true);
}

int main() {
    SSL *server, *client;
    create_loopback(&server, &client);

    handshake(client, server);

    printf("Writing...\n");
    int result = SSL_write(server, "xy", 2);
    assert_openssl(result == 2);

    char out[3];
    printf("Peeking...\n");
    result = SSL_peek(client, out, 2);
    assert_openssl(result == 2);
    printf("Result: %s\n", out);

    printf("Peeking...\n");
    result = SSL_peek(client, out, 2);
    assert_openssl(result == 2);
    printf("Result: %s\n", out);

    printf("Reading...\n");
    result = SSL_read(client, out, 2);
    assert_openssl(result == 2);
    printf("Result: %s\n", out);
}

Reproduces 100% reliably for me. Switching SSL_peek to SSL_read fixes it, and switching TLSv1_method to TLS_method or TLSv1_1_method also fixes it.

Therefore I believe, but have not confirmed, that https://github.com/openssl/openssl/blob/master/ssl/record/rec_layer_s3.c#L1120-L1144 is the loop that's spinning.

Demonstration of what this looke like:

$ clang -L/usr/local/opt/openssl@1.1/lib -I/usr/local/opt/openssl@1.1/include t.c -lcrypto -lssl
t.c:104:39: warning: 'TLSv1_method' is deprecated [-Wdeprecated-declarations]
    SSL_CTX *server_ctx = SSL_CTX_new(TLSv1_method());
                                      ^
/usr/local/opt/openssl@1.1/include/openssl/ssl.h:1596:45: note: 'TLSv1_method' has been explicitly marked deprecated here
DEPRECATEDIN_1_1_0(__owur const SSL_METHOD *TLSv1_method(void)) /* TLSv1.0 */
                                            ^
t.c:113:39: warning: 'TLSv1_method' is deprecated [-Wdeprecated-declarations]
    SSL_CTX *client_ctx = SSL_CTX_new(TLSv1_method());
                                      ^
/usr/local/opt/openssl@1.1/include/openssl/ssl.h:1596:45: note: 'TLSv1_method' has been explicitly marked deprecated here
DEPRECATEDIN_1_1_0(__owur const SSL_METHOD *TLSv1_method(void)) /* TLSv1.0 */
                                            ^
2 warnings generated.
$ ./a.out
Writing...
Peeking...
levitte pushed a commit that referenced this issue Sep 22, 2016
If while calling SSL_peek() we read an empty record then we go into an
infinite loop, continually trying to read data from the empty record and
never making any progress. This could be exploited by a malicious peer in
a Denial Of Service attack.

CVE-2016-6305

GitHub Issue #1563

Reviewed-by: Rich Salz <rsalz@openssl.org>
levitte pushed a commit that referenced this issue Sep 22, 2016
If while calling SSL_peek() we read an empty record then we go into an
infinite loop, continually trying to read data from the empty record and
never making any progress. This could be exploited by a malicious peer in
a Denial Of Service attack.

CVE-2016-6305

GitHub Issue #1563

Reviewed-by: Rich Salz <rsalz@openssl.org>
agl pushed a commit to google/boringssl that referenced this issue Sep 22, 2016
SSL_peek works fine for us, but OpenSSL 1.1.0 regressed this
(openssl/openssl#1563), and we don't have
tests either. Fix this.

SSL_peek can handle all weird events that SSL_read can, so use runner
and tell bssl_shim to do a SSL_peek + SSL_peek + SSL_read instead of
SSL_read. Then add tests for all the events we may discover.

Change-Id: I9e8635e3ca19653a02a883f220ab1332d4412f98
Reviewed-on: https://boringssl-review.googlesource.com/11090
Reviewed-by: Adam Langley <agl@google.com>
@alex alex closed this as completed Sep 24, 2016
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

1 participant