fixes

goatgoose · Sep 21, 2023 · b4ecdfc · b4ecdfc
1 parent b89289f
commit b4ecdfc
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 23 deletions.
diff --git a/api/s2n.h b/api/s2n.h
@@ -1522,7 +1522,7 @@ S2N_API extern int s2n_client_hello_get_session_id_length(struct s2n_client_hell
 S2N_API extern int s2n_client_hello_get_session_id(struct s2n_client_hello *ch, uint8_t *out, uint32_t *out_length, uint32_t max_length);
 
 /**
- * Retrieves the supported groups received in the supported groups extension.
+ * Retrieves the supported groups received from the peer in the supported groups extension.
  *
  * IANA values for each of the received supported groups are written to the provided
  * `supported_groups` array, and `supported_groups_count` is set to the number of received
@@ -1532,16 +1532,16 @@ S2N_API extern int s2n_client_hello_get_session_id(struct s2n_client_hello *ch,
  * `max_count` is less than the number of received supported groups, this function will error. To
  * determine how large `supported_groups` should be in advance, use
  * `s2n_client_hello_get_extension_length()` with the S2N_EXTENSION_SUPPORTED_GROUPS extension
- * type.
+ * type, and divide the value by 2.
  *
  * If no supported groups extension was received from the peer, or the received supported groups
  * extension is malformed, this function will error.
  *
  * @param ch A pointer to the ClientHello. Can be retrieved from a connection via
  * `s2n_connection_get_client_hello()`.
- * @param supported_groups An array that will be filled with the received supported groups.
- * @param supported_groups_count Set to the number of received supported groups.
- * @param max_count The maximum number of supported groups that can fit in `supported_groups`.
+ * @param supported_groups The array to populate with the received supported groups.
+ * @param supported_groups_count Returns the number of received supported groups.
+ * @param max_count The maximum number of supported groups that can fit in the `supported_groups` array.
  * @returns S2N_SUCCESS on success. S2N_FAILURE on failure.
  */
 S2N_API extern int s2n_client_hello_get_supported_groups(struct s2n_client_hello *ch, uint16_t *supported_groups,

diff --git a/tests/unit/s2n_client_hello_get_supported_groups_test.c b/tests/unit/s2n_client_hello_get_supported_groups_test.c
@@ -45,13 +45,6 @@ int s2n_check_received_supported_groups_cb(struct s2n_connection *conn, void *ct
     struct s2n_client_hello *client_hello = s2n_connection_get_client_hello(conn);
     EXPECT_NOT_NULL(client_hello);
 
-    bool supported_groups_received = false;
-    EXPECT_SUCCESS(s2n_client_hello_has_extension(client_hello, S2N_EXTENSION_SUPPORTED_GROUPS,
-            &supported_groups_received));
-    if (!supported_groups_received) {
-        return S2N_SUCCESS;
-    }
-
     uint16_t supported_groups[S2N_TEST_SUPPORTED_GROUPS_LIST_COUNT] = { 0 };
     uint16_t supported_groups_count = 0;
     EXPECT_SUCCESS(s2n_client_hello_get_supported_groups(client_hello, supported_groups,
@@ -135,21 +128,21 @@ int main(int argc, char **argv)
         uint16_t supported_groups[S2N_TEST_SUPPORTED_GROUPS_LIST_COUNT] = { 0 };
         uint16_t supported_groups_count = 0;
 
-        /* s2n_client_hello_get_supported_groups should fail if the provided buffer is too small. */
+        /* Fail if the provided buffer is too small. */
         EXPECT_FAILURE_WITH_ERRNO(s2n_client_hello_get_supported_groups(&client_hello, supported_groups,
                                           &supported_groups_count, S2N_TEST_SUPPORTED_GROUPS_LIST_COUNT - 1),
                 S2N_ERR_SAFETY);
         EXPECT_EQUAL(supported_groups_count, 0);
 
         EXPECT_SUCCESS(s2n_stuffer_reread(&extension_stuffer));
 
-        /* s2n_client_hello_get_supported_groups should succeed with a correctly sized buffer. */
+        /* Succeed with a correctly sized buffer. */
         EXPECT_SUCCESS(s2n_client_hello_get_supported_groups(&client_hello, supported_groups, &supported_groups_count,
-                s2n_array_len(supported_groups)));
-        EXPECT_EQUAL(supported_groups_count, s2n_array_len(supported_groups));
+                S2N_TEST_SUPPORTED_GROUPS_LIST_COUNT));
+        EXPECT_EQUAL(supported_groups_count, S2N_TEST_SUPPORTED_GROUPS_LIST_COUNT);
     }
 
-    /* Ensure that s2n_client_hello_get_supported_groups fails if the client hello isn't parsed yet. */
+    /* Error if the client hello isn't parsed yet. */
     {
         struct s2n_client_hello client_hello = { 0 };
 
@@ -160,9 +153,7 @@ int main(int argc, char **argv)
                 S2N_ERR_EXTENSION_NOT_RECEIVED);
     }
 
-    /* Ensure that s2n_client_hello_get_supported_groups fails if a supported groups extension
-     * wasn't received.
-     */
+    /* Error if a supported groups extension wasn't received. */
     for (int disable_ecc = 0; disable_ecc <= 1; disable_ecc++) {
         DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
         EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, chain_and_key));
@@ -289,9 +280,7 @@ int main(int argc, char **argv)
         }
     }
 
-    /* Ensure that s2n_client_hello_get_supported_groups writes what is contained in the parsed
-     * supported groups extension in the client hello.
-     */
+    /* Ensure that the supported groups in the client hello are written to the output array. */
     {
         struct s2n_client_hello client_hello = { 0 };