Validate websocket handshake response (#410)

**Issue:**
The websocket client did not fully validate the server's response, as specified in [RFC-6455 Section 4.1](https://www.rfc-editor.org/rfc/rfc6455#section-4.1)

The client was doing 1/6 required steps (checking the status-code, but not checking any headers)

**Description of changes:**
Validate the "Upgrade", "Connection", and "Sec-WebSocket-Accept" headers.

Now we're doing 4/6 required steps. Still need to check "Sec-WebSocket-Extensions" and "Sec-WebSocket-Protocol". That will be in a followup PR.

Author: graebm

include/aws/http/websocket.h

@@ -245,14 +245,14 @@ struct aws_websocket_client_connection_options {
 
     /**
      * Called repeatedly as payload data arrives.
-     * Required if `on_incoming_frame_begin` is set.
+     * Optional.
      * See `aws_websocket_on_incoming_frame_payload_fn`.
      */
     aws_websocket_on_incoming_frame_payload_fn *on_incoming_frame_payload;
 
     /**
      * Called when done processing an incoming frame.
-     * Required if `on_incoming_frame_begin` is set.
+     * Optional.
      * See `aws_websocket_on_incoming_frame_complete_fn`.
      */
     aws_websocket_on_incoming_frame_complete_fn *on_incoming_frame_complete;

source/websocket.c

@@ -1373,16 +1373,14 @@ static int s_decoder_on_payload(struct aws_byte_cursor data, void *user_data) {
 
 /* Invoke user cb */
 static int s_decoder_on_user_payload(struct aws_websocket *websocket, struct aws_byte_cursor data) {
-    if (!websocket->on_incoming_frame_payload) {
-        return AWS_OP_SUCCESS;
-    }
+    if (websocket->on_incoming_frame_payload) {
+        if (!websocket->on_incoming_frame_payload(
+                websocket, websocket->thread_data.current_incoming_frame, data, websocket->user_data)) {
 
-    if (!websocket->on_incoming_frame_payload(
-            websocket, websocket->thread_data.current_incoming_frame, data, websocket->user_data)) {
-
-        AWS_LOGF_ERROR(
-            AWS_LS_HTTP_WEBSOCKET, "id=%p: Incoming payload callback has reported a failure.", (void *)websocket);
-        return aws_raise_error(AWS_ERROR_HTTP_CALLBACK_FAILURE);
+            AWS_LOGF_ERROR(
+                AWS_LS_HTTP_WEBSOCKET, "id=%p: Incoming payload callback has reported a failure.", (void *)websocket);
+            return aws_raise_error(AWS_ERROR_HTTP_CALLBACK_FAILURE);
+        }
     }
 
     /* If this is a "data" frame's payload, let the window shrink */

source/websocket_bootstrap.c

@@ -3,9 +3,12 @@
  * SPDX-License-Identifier: Apache-2.0.
  */
 
+#include <aws/cal/hash.h>
+#include <aws/common/encoding.h>
 #include <aws/common/logging.h>
 #include <aws/http/connection.h>
 #include <aws/http/private/http_impl.h>
+#include <aws/http/private/strutil.h>
 #include <aws/http/private/websocket_impl.h>
 #include <aws/http/request_response.h>
 #include <aws/http/status_code.h>
@@ -66,6 +69,10 @@ struct aws_websocket_client_bootstrap {
     /* Handshake request data */
     struct aws_http_message *handshake_request;
 
+    /* Given the "Sec-WebSocket-Key" from the request,
+     * this is what we expect the response's "Sec-WebSocket-Accept" to be */
+    struct aws_byte_buf expected_sec_websocket_accept;
+
     /* Handshake response data */
     int response_status;
     struct aws_http_headers *response_headers;
@@ -78,6 +85,10 @@ struct aws_websocket_client_bootstrap {
 };
 
 static void s_ws_bootstrap_destroy(struct aws_websocket_client_bootstrap *ws_bootstrap);
+static int s_ws_bootstrap_calculate_sec_websocket_accept(
+    struct aws_byte_cursor sec_websocket_key,
+    struct aws_byte_buf *out_buf,
+    struct aws_allocator *alloc);
 static void s_ws_bootstrap_cancel_setup_due_to_err(
     struct aws_websocket_client_bootstrap *ws_bootstrap,
     struct aws_http_connection *http_connection,
@@ -125,24 +136,19 @@ int aws_websocket_client_connect(const struct aws_websocket_client_connection_op
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
-    bool all_frame_callbacks_set =
-        options->on_incoming_frame_begin && options->on_incoming_frame_payload && options->on_incoming_frame_begin;
-
-    bool no_frame_callbacks_set =
-        !options->on_incoming_frame_begin && !options->on_incoming_frame_payload && !options->on_incoming_frame_begin;
-
-    if (!(all_frame_callbacks_set || no_frame_callbacks_set)) {
+    if (!options->handshake_request) {
         AWS_LOGF_ERROR(
             AWS_LS_HTTP_WEBSOCKET_SETUP,
-            "id=static: Invalid websocket connection options,"
-            " either all frame-handling callbacks must be set, or none must be set.");
+            "id=static: Invalid connection options, missing required request for websocket client handshake.");
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
-    if (!options->handshake_request) {
+    const struct aws_http_headers *request_headers = aws_http_message_get_headers(options->handshake_request);
+    struct aws_byte_cursor sec_websocket_key;
+    if (aws_http_headers_get(request_headers, aws_byte_cursor_from_c_str("Sec-WebSocket-Key"), &sec_websocket_key)) {
         AWS_LOGF_ERROR(
             AWS_LS_HTTP_WEBSOCKET_SETUP,
-            "id=static: Invalid connection options, missing required request for websocket client handshake.");
+            "id=static: Websocket handshake request is missing required 'Sec-WebSocket-Key' header");
         return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
     }
 
@@ -164,6 +170,11 @@ int aws_websocket_client_connect(const struct aws_websocket_client_connection_op
     ws_bootstrap->response_headers = aws_http_headers_new(ws_bootstrap->alloc);
     aws_byte_buf_init(&ws_bootstrap->response_body, ws_bootstrap->alloc, 0);
 
+    if (s_ws_bootstrap_calculate_sec_websocket_accept(
+            sec_websocket_key, &ws_bootstrap->expected_sec_websocket_accept, ws_bootstrap->alloc)) {
+        goto error;
+    }
+
     /* Initiate HTTP connection */
     struct aws_http_client_connection_options http_options = AWS_HTTP_CLIENT_CONNECTION_OPTIONS_INIT;
     http_options.allocator = ws_bootstrap->alloc;
@@ -203,7 +214,7 @@ int aws_websocket_client_connect(const struct aws_websocket_client_connection_op
             "id=static: Websocket failed to initiate HTTP connection, error %d (%s)",
             aws_last_error(),
             aws_error_name(aws_last_error()));
-        goto error_already_logged;
+        goto error;
     }
 
     /* Success! (so far) */
@@ -217,7 +228,7 @@ int aws_websocket_client_connect(const struct aws_websocket_client_connection_op
 
     return AWS_OP_SUCCESS;
 
-error_already_logged:
+error:
     s_ws_bootstrap_destroy(ws_bootstrap);
     return AWS_OP_ERR;
 }
@@ -229,11 +240,96 @@ static void s_ws_bootstrap_destroy(struct aws_websocket_client_bootstrap *ws_boo
 
     aws_http_message_release(ws_bootstrap->handshake_request);
     aws_http_headers_release(ws_bootstrap->response_headers);
+    aws_byte_buf_clean_up(&ws_bootstrap->expected_sec_websocket_accept);
     aws_byte_buf_clean_up(&ws_bootstrap->response_body);
 
     aws_mem_release(ws_bootstrap->alloc, ws_bootstrap);
 }
 
+/* Given the handshake request's "Sec-WebSocket-Key" value,
+ * calculate the expected value for the response's "Sec-WebSocket-Accept".
+ * RFC-6455 Section 4.1:
+ *      base64-encoded SHA-1 of the concatenation of the |Sec-WebSocket-Key|
+ *      (as a string, not base64-decoded) with the string
+ *      "258EAFA5-E914-47DA-95CA-C5AB0DC85B11" but ignoring any leading and
+ *      trailing whitespace
+ */
+static int s_ws_bootstrap_calculate_sec_websocket_accept(
+    struct aws_byte_cursor sec_websocket_key,
+    struct aws_byte_buf *out_buf,
+    struct aws_allocator *alloc) {
+
+    AWS_ASSERT(out_buf && !out_buf->allocator && out_buf->len == 0); /* expect buf to be uninitialized */
+
+    /* note: leading and trailing whitespace was already trimmed by aws_http_headers */
+
+    /* optimization: skip concatenating Sec-WebSocket-Key and the magic string.
+     * just run the SHA1 over the first string, and then the 2nd. */
+
+    bool success = false;
+    struct aws_byte_cursor magic_string = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL("258EAFA5-E914-47DA-95CA-C5AB0DC85B11");
+
+    /* SHA-1 */
+    struct aws_hash *sha1 = aws_sha1_new(alloc);
+    if (!sha1) {
+        AWS_LOGF_ERROR(
+            AWS_LS_HTTP_WEBSOCKET_SETUP,
+            "id=static: Failed to initiate SHA1, error %d (%s)",
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
+        goto cleanup;
+    }
+
+    if (aws_hash_update(sha1, &sec_websocket_key) || aws_hash_update(sha1, &magic_string)) {
+        AWS_LOGF_ERROR(
+            AWS_LS_HTTP_WEBSOCKET_SETUP,
+            "id=static: Failed to update SHA1, error %d (%s)",
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
+        goto cleanup;
+    }
+
+    uint8_t sha1_storage[AWS_SHA1_LEN];
+    struct aws_byte_buf sha1_buf = aws_byte_buf_from_empty_array(sha1_storage, sizeof(sha1_storage));
+    if (aws_hash_finalize(sha1, &sha1_buf, 0)) {
+        AWS_LOGF_ERROR(
+            AWS_LS_HTTP_WEBSOCKET_SETUP,
+            "id=static: Failed to finalize SHA1, error %d (%s)",
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
+        goto cleanup;
+    }
+
+    /* base64-encoded SHA-1 (clear out_buf, and write to it again) */
+    size_t base64_encode_sha1_len;
+    if (aws_base64_compute_encoded_len(sha1_buf.len, &base64_encode_sha1_len)) {
+        AWS_LOGF_ERROR(
+            AWS_LS_HTTP_WEBSOCKET_SETUP,
+            "id=static: Failed to determine Base64-encoded length, error %d (%s)",
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
+        goto cleanup;
+    }
+    aws_byte_buf_init(out_buf, alloc, base64_encode_sha1_len);
+
+    struct aws_byte_cursor sha1_cursor = aws_byte_cursor_from_buf(&sha1_buf);
+    if (aws_base64_encode(&sha1_cursor, out_buf)) {
+        AWS_LOGF_ERROR(
+            AWS_LS_HTTP_WEBSOCKET_SETUP,
+            "id=static: Failed to Base64-encode, error %d (%s)",
+            aws_last_error(),
+            aws_error_name(aws_last_error()));
+        goto cleanup;
+    }
+
+    success = true;
+cleanup:
+    if (sha1) {
+        aws_hash_destroy(sha1);
+    }
+    return success ? AWS_OP_SUCCESS : AWS_OP_ERR;
+}
+
 /* Called if something goes wrong after an HTTP connection is established.
  * The HTTP connection is closed.
  * We must wait for its shutdown to complete before informing user of the failed websocket setup. */
@@ -457,15 +553,83 @@ static int s_ws_bootstrap_on_handshake_response_headers(
     return AWS_OP_SUCCESS;
 }
 
+static int s_ws_bootstrap_validate_header(
+    struct aws_websocket_client_bootstrap *ws_bootstrap,
+    const char *name,
+    struct aws_byte_cursor expected_value,
+    bool case_sensitive) {
+
+    struct aws_byte_cursor actual_value;
+    if (aws_http_headers_get(ws_bootstrap->response_headers, aws_byte_cursor_from_c_str(name), &actual_value)) {
+        AWS_LOGF_ERROR(
+            AWS_LS_HTTP_WEBSOCKET_SETUP, "id=%p: Response lacks required '%s' header", (void *)ws_bootstrap, name);
+        return aws_raise_error(AWS_ERROR_HTTP_WEBSOCKET_UPGRADE_FAILURE);
+    }
+
+    bool matches = case_sensitive ? aws_byte_cursor_eq(&expected_value, &actual_value)
+                                  : aws_byte_cursor_eq_ignore_case(&expected_value, &actual_value);
+    if (!matches) {
+        AWS_LOGF_ERROR(
+            AWS_LS_HTTP_WEBSOCKET_SETUP,
+            "id=%p: Response '%s' header has wrong value. Expected '" PRInSTR "'. Received '" PRInSTR "'",
+            (void *)ws_bootstrap,
+            name,
+            AWS_BYTE_CURSOR_PRI(expected_value),
+            AWS_BYTE_CURSOR_PRI(actual_value));
+        return aws_raise_error(AWS_ERROR_HTTP_WEBSOCKET_UPGRADE_FAILURE);
+    }
+
+    return AWS_OP_SUCCESS;
+}
+
 /* OK, we've got all the headers for the 101 Switching Protocols response.
- * Verify handshake response according to RFC-6455 Section 1.3,
- * install the websocket handler into the channel,
+ * Validate the handshake response, install the websocket handler into the channel,
  * and invoke the on_connection_setup callback. */
 static int s_ws_bootstrap_validate_response_and_install_websocket_handler(
     struct aws_websocket_client_bootstrap *ws_bootstrap,
     struct aws_http_connection *http_connection) {
 
-    /* TODO: validate Sec-WebSocket-Accept header */
+    /* RFC-6455 Section 4.1 - The client MUST validate the server's response as follows... */
+
+    /* (we already checked step 1, that status code is 101) */
+    AWS_FATAL_ASSERT(ws_bootstrap->response_status == AWS_HTTP_STATUS_CODE_101_SWITCHING_PROTOCOLS);
+
+    /* 2.   If the response lacks an |Upgrade| header field or the |Upgrade|
+     *      header field contains a value that is not an ASCII case-
+     *      insensitive match for the value "websocket", the client MUST
+     *      _Fail the WebSocket Connection_. */
+    if (s_ws_bootstrap_validate_header(
+            ws_bootstrap, "Upgrade", aws_byte_cursor_from_c_str("websocket"), false /*case_sensitive*/)) {
+        goto error;
+    }
+
+    /* 3.   If the response lacks a |Connection| header field or the
+     *      |Connection| header field doesn't contain a token that is an
+     *      ASCII case-insensitive match for the value "Upgrade", the client
+     *      MUST _Fail the WebSocket Connection_. */
+    if (s_ws_bootstrap_validate_header(
+            ws_bootstrap, "Connection", aws_byte_cursor_from_c_str("Upgrade"), false /*case_sensitive*/)) {
+        goto error;
+    }
+
+    /* 4.   If the response lacks a |Sec-WebSocket-Accept| header field or
+     *      the |Sec-WebSocket-Accept| contains a value other than the
+     *      base64-encoded SHA-1 of the concatenation of the |Sec-WebSocket-
+     *      Key| (as a string, not base64-decoded) with the string "258EAFA5-
+     *      E914-47DA-95CA-C5AB0DC85B11" but ignoring any leading and
+     *      trailing whitespace, the client MUST _Fail the WebSocket
+     *      Connection_. */
+    if (s_ws_bootstrap_validate_header(
+            ws_bootstrap,
+            "Sec-WebSocket-Accept",
+            aws_byte_cursor_from_buf(&ws_bootstrap->expected_sec_websocket_accept),
+            true /*case_sensitive*/)) {
+        goto error;
+    }
+
+    /* TODO: validate Sec-WebSocket-Extensions */
+
+    /* TODO: validate Sec-WebSocket-Protocol */
 
     /* Insert websocket handler into channel */
     struct aws_channel *channel = s_system_vtable->aws_http_connection_get_channel(http_connection);

tests/CMakeLists.txt

@@ -228,6 +228,10 @@ add_test_case(websocket_boot_report_unexpected_http_shutdown)
 add_test_case(websocket_boot_fail_from_handshake_rejection)
 add_test_case(websocket_boot_fail_before_handshake_rejection_body)
 add_test_case(websocket_boot_fail_before_handshake_rejection_stream_complete)
+add_test_case(websocket_boot_fail_from_invalid_upgrade_header)
+add_test_case(websocket_boot_fail_from_missing_upgrade_header)
+add_test_case(websocket_boot_fail_from_invalid_connection_header)
+add_test_case(websocket_boot_fail_from_invalid_sec_websocket_upgrade_header)
 add_test_case(websocket_handshake_key_max_length)
 add_test_case(websocket_handshake_key_randomness)