Miscellaneous updates

Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #19769)
openssl · Jun 27, 2023 · 64aa8ea · 64aa8ea
1 parent e4c2988
commit 64aa8ea
Show file tree

Hide file tree

Showing 2 changed files with 55 additions and 115 deletions.
diff --git a/doc/designs/quic-design/quic-api-ssl-funcs.md b/doc/designs/quic-design/quic-api-ssl-funcs.md
@@ -133,7 +133,7 @@ Notes:
 | `DTLSv1_2_client_method` | Global | 🟩U | 🟦U | 🟩NC | 🟢Done |
 | `DTLSv1_2_server_method` | Global | 🟩U | 🟦U | 🟩NC | 🟢Done |
 | `OSSL_QUIC_client_method` | Global | 🟩U | 🟦U | 🟥QSA | 🟢Done |
-| `OSSL_QUIC_client_thread_method` | Global | 🟩U | 🟦U | 🟥QSA | 🟠Design TBD |
+| `OSSL_QUIC_client_thread_method` | Global | 🟩U | 🟦U | 🟥QSA | 🟢Done |
 | `OSSL_QUIC_server_method` | Global | 🟩U | 🟦U | 🟥QSA | 🟠Design TBD |
 | **⇒ Instantiation** | |
 | `BIO_f_ssl` | Object | 🟩U | 🟩A | 🟩NC | 🟢Done |
@@ -189,9 +189,9 @@ Notes:
 | `SSL_SESSION_set1_alpn_selected` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟡TODO |
 | `SSL_SESSION_get0_alpn_selected` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟡TODO |
 | `SSL_CTX_set_alpn_select_cb` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟡TODO |
-| `SSL_set_alpn_protos` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟡TODO |
-| `SSL_get0_alpn_selected` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟡TODO |
-| `SSL_CTX_set_alpn_protos` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟡TODO |
+| `SSL_set_alpn_protos` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟢Done |
+| `SSL_get0_alpn_selected` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟢Done |
+| `SSL_CTX_set_alpn_protos` | HL | 🟩U | 🟩A | 🟨C\* †2 | 🟢Done |
 | **⇒ NPN** | †3 |
 | `SSL_CTX_set_next_proto_select_cb` | HL | 🟩U | 🟥FC | 🟨C\* †3 | 🟢Done |
 | `SSL_CTX_set_next_protos_advertised_cb` | HL | 🟩U | 🟥FC | 🟨C\* †3 | 🟢Done |
@@ -648,9 +648,7 @@ Notes:
 | `SSL_accept_stream` | CSSM | 🟦N | 🟩A | 🟥QSA | 🟡TODO |
 | `SSL_get_accept_stream_queue_len` | CSSM | 🟦N | 🟩A | 🟥QSA | 🟡TODO |
 | `SSL_set_default_stream_mode` | CSSM | 🟦N | 🟩A | 🟥QSA | 🟡TODO |
-| `SSL_set_incoming_stream_reject_policy` | CSSM | 🟦N | 🟩A | 🟥QSA | 🟡TODO |
-| `SSL_detach_stream` | CSSM | 🟦N | 🟩A | 🟥QSA | 🟡TODO |
-| `SSL_attach_stream` | CSSM | 🟦N | 🟩A | 🟥QSA | 🟡TODO |
+| `SSL_set_incoming_stream_policy` | CSSM | 🟦N | 🟩A | 🟥QSA | 🟡TODO |
 | **⇒ Currently Not Supported** | |
 | `SSL_copy_session_id` | Special | 🟩U | 🟥FC | 🟨C* | 🟢Done |
 | `BIO_ssl_copy_session_id` | Special | 🟩U | 🟥FC | 🟨C* | 🟢Done |
@@ -842,23 +840,18 @@ encryption is mostly for protection against accidential modification and not
 active yet. An application using QUIC can always interpret NULL as meaning
 AES-128-GCM is being used if needed as this is implied by using QUIC.
 
+A. We return NULL here, because it allows applications to detect if a
+ciphersuite has been negotiated and NULL can be used to infer that Initial
+encryption is still being used. This also minimises the changes needed to the
+implementation.
+
 ### What should `SSL_CTX_set_cipher_list` do?
 
 Since this function configures the cipher list for TLSv1.2 and below only, there
 is no need to restrict it as TLSv1.3 is required for QUIC. For the sake of
 application compatibility, applications can still configure the TLSv1.2 cipher
-list; it will always be ignored.
-
-### What should `SSL_get_current_cipher` and similar do?
-
-QUIC always uses AES-128-GCM encryption initially, so we could either return
-AES-128-GCM where the handshake has not yet negotiated another algorithm or
-return NULL here.
-
-A. We return NULL here, because it allows applications to detect if a
-ciphersuite has been negotiated and NULL can be used to infer that Initial
-encryption is still being used. This also minimises the changes needed to the
-implementation.
+list; it will always be ignored. This function can still be used to set the
+SECLEVEL; no changes are needed to facilitate this.
 
 ### What SSL options should be supported?
 
@@ -870,10 +863,10 @@ Options we explicitly want to support:
 - `SSL_OP_NO_RX_CERTIFICATE_COMPRESSION`
 - `SSL_OP_PRIORITIZE_CHACHA`
 - `SSL_OP_NO_TICKET`
+- `SSL_OP_CLEANSE_PLAINTEXT`
 
 Options we do not yet support but could support in the future, currently no-ops:
 
-- `SSL_OP_CLEANSE_PLAINTEXT`
 - `SSL_OP_NO_QUERY_MTU`
 - `SSL_OP_NO_ANTI_REPLAY`
 
@@ -983,12 +976,11 @@ This is a deprecated function, so it needn't be supported for QUIC. Fail closed.
 
 ### What should `SSL_set_ssl_method` do?
 
-For now we can avoid supporting this for QUIC. Supporting this would be rather
-hairy.
+We do not currently support this for QUIC.
 
 ### What should `SSL_set_shutdown` do?
 
-TBD.
+This is not supported and is a no-op for QUIC.
 
 ### What should `SSL_dup` and `SSL_clear` do?
 

diff --git a/doc/designs/quic-design/quic-api.md b/doc/designs/quic-design/quic-api.md
@@ -30,11 +30,11 @@ designs and the relevant design decisions.
       - [`SSL_CTRL_MODE`, `SSL_CTRL_CLEAR_MODE`](#-ssl-ctrl-mode----ssl-ctrl-clear-mode-)
       - [SSL Modes](#ssl-modes)
     + [New APIs for Single-Stream Operation](#new-apis-for-single-stream-operation)
-      - [`SSL_tick`](#-ssl-tick-)
-      - [`SSL_get_tick_timeout`](#-ssl-get-tick-timeout-)
+      - [`SSL_handle_events`](#-ssl-handle-events-)
+      - [`SSL_get_event_timeout`](#-ssl-get-event-timeout-)
       - [`SSL_set_blocking_mode`, `SSL_get_blocking_mode`](#-ssl-set-blocking-mode----ssl-get-blocking-mode-)
       - [`SSL_get_rpoll_descriptor`, `SSL_get_wpoll_descriptor`](#-ssl-get-rpoll-descriptor----ssl-get-wpoll-descriptor-)
-      - [`SSL_want_net_read`, `SSL_want_net_write`](#-ssl-want-net-read----ssl-want-net-write-)
+      - [`SSL_net_read_desired`, `SSL_net_write_desired`](#-ssl-want-net-read----ssl-want-net-write-)
       - [`SSL_want`, `SSL_want_read`, `SSL_want_write`](#-ssl-want----ssl-want-read----ssl-want-write-)
       - [`SSL_set_initial_peer_addr`, `SSL_get_initial_peer_addr`](#-ssl-set-initial-peer-addr----ssl-get-initial-peer-addr-)
       - [`SSL_shutdown_ex`](#-ssl-shutdown-ex-)
@@ -54,10 +54,8 @@ designs and the relevant design decisions.
       - [`SSL_new_stream`](#-ssl-new-stream-)
       - [`SSL_accept_stream`](#-ssl-accept-stream-)
       - [`SSL_get_accept_stream_queue_len`](#-ssl-get-accept-stream-queue-len-)
-      - [`SSL_set_incoming_stream_reject_policy`](#-ssl-set-incoming-stream-reject-policy-)
+      - [`SSL_set_incoming_stream_policy`](#-ssl-set-incoming-stream-policy-)
       - [`SSL_set_default_stream_mode`](#-ssl-set-default-stream-mode-)
-      - [`SSL_detach_stream`](#-ssl-detach-stream-)
-      - [`SSL_attach_stream`](#-ssl-attach-stream-)
     + [Future APIs](#future-apis)
   * [BIO Objects](#bio-objects)
     + [Existing APIs](#existing-apis-1)
@@ -143,18 +141,16 @@ Each API listed below has an information table with the following fields:
     - **Error**: Non-`WANT_READ`/`WANT_WRITE` errors can be raised.
     - **Want**: `WANT_READ`/`WANT_WRITE` can be raised.
 
-- **Can Tick?**: Whether this function is allowed to tick the QUIC state
-  machine and potentially perform network I/O.
+- **Can Tick?**: Whether this function is allowed to perform event processing
+  for the QUIC state machine and potentially perform network I/O.
 
 - **CSHL:** Connection/Stream/Handshake Layer classification.
   This can be one of:
 
     - **HL:** This is a handshake layer related call. It should be supported
       on a QUIC connection SSL object, forwarding to the handshake layer
-      SSL object.
-
-      Whether we allow QUIC stream SSL objects to have these calls forwarded is
-      TBD.
+      SSL object. QUIC stream SSL objects do not allow these calls to be
+      forwarded.
 
     - **HL-Forbidden:** This is a handshake layer related call, but it is
       inapplicable to QUIC, so it is not supported.
@@ -430,9 +426,7 @@ Should not require any changes.
 
 ### New APIs for Single-Stream Operation
 
-TBD: Should any of these be implemented as ctrls rather than actual functions?
-
-#### `SSL_tick`
+#### `SSL_handle_events`
 
 | Semantics | `SSL_get_error` | Can Tick? | CSHL          |
 | --------- | ------------- | --------- | ------------- |
@@ -442,10 +436,7 @@ Advances the QUIC state machine to the extent feasible, potentially performing
 network I/O. Also compatible with DTLSv1 and supercedes `DTLSv1_handle_timeout`
 for all use cases.
 
-TBD: Deprecate `DTLSv1_get_timeout`?
-TBD: Deprecate `DTLSv1_handle_timeout`?
-
-#### `SSL_get_tick_timeout`
+#### `SSL_get_event_timeout`
 
 | Semantics | `SSL_get_error` | Can Tick? | CSHL          |
 | --------- | ------------- | --------- | ------------- |
@@ -460,14 +451,11 @@ protocol-agnostic API for this purpose, superceding `DTLSv1_get_timeout` for all
 use cases.
 
 The design is similar to that of `DTLSv1_get_timeout` and uses a `struct
-timeval`. However, this function represents an infinite timeout (i.e., no
-timeout) using `tv_sec == -1`, whereas `DTLSv1_get_timeout` represents an
+timeval`. However, this function can also output an infinite timeout using the
+`is_infinite` argument, whereas whereas `DTLSv1_get_timeout` represents an
 infinite timeout using a 0 return value, which does not allow a failure
 condition to be distinguished.
 
-TBD: Should we just map this to DTLS_CTRL_GET_TIMEOUT internally (and maybe
-alias the CTRL #define)?
-
 #### `SSL_set_blocking_mode`, `SSL_get_blocking_mode`
 
 | Semantics | `SSL_get_error` | Can Tick? | CSHL          |
@@ -491,18 +479,18 @@ Not supported for non-QUIC SSL objects.
 | --------- | ------------- | --------- | ------------- |
 | New       | Never         | No        | CS            |
 
-These functions output poll descriptors which can be used to determine
-when the QUIC state machine should next be ticked. `SSL_get_rpoll_descriptor` is
-relevant if `SSL_want_net_read` returns 1, and `SSL_get_wpoll_descriptor` is
-relevant if `SSL_want_net_write` returns 1.
+These functions output poll descriptors which can be used to determine when the
+QUIC state machine next needs to have events handled. `SSL_get_rpoll_descriptor`
+is relevant if `SSL_net_read_desired` returns 1, and `SSL_get_wpoll_descriptor`
+is relevant if `SSL_net_write_desired` returns 1.
 
 The implementation of these functions is a simple forward to
 `BIO_get_rpoll_descriptor` and `BIO_get_wpoll_descriptor` on the underlying
 network BIOs.
 
 TODO: Support these for non-QUIC SSL objects
 
-#### `SSL_want_net_read`, `SSL_want_net_write`
+#### `SSL_net_read_desired`, `SSL_net_write_desired`
 
 | Semantics | `SSL_get_error` | Can Tick? | CSHL          |
 | --------- | ------------- | --------- | ------------- |
@@ -511,11 +499,11 @@ TODO: Support these for non-QUIC SSL objects
 These calls return 1 if the QUIC state machine is interested in receiving
 further data from the network, or writing to the network, respectively. The
 return values of these calls should be used to determine which wakeup events
-should cause an application to call `SSL_tick`. These functions do not mutate
-any state, and their return values may change after a call to any SSL function
-other than `SSL_want_net_read`, `SSL_want_net_write`,
+should cause an application to call `SSL_handle_events`. These functions do not
+mutate any state, and their return values may change after a call to any SSL
+function other than `SSL_net_read_desired`, `SSL_net_write_desired`,
 `SSL_get_rpoll_descriptor`, `SSL_get_wpoll_descriptor` and
-`SSL_get_tick_timeout`.
+`SSL_get_event_timeout`.
 
 TODO: Support these for non-QUIC SSL objects, turning this into a unified
 replacement for `SSL_want`
@@ -528,8 +516,8 @@ non-blocking mode due to a desire to read from or write to the underlying
 network BIO. However, this API is unsuitable for use with QUIC because the
 return value of `SSL_want` can only express one I/O direction at a time (read or
 write), not both. This call will not be implemented for QUIC (e.g. always
-returns `SSL_NOTHING`) and `SSL_want_net_read` and `SSL_want_net_write` will be
-used instead.
+returns `SSL_NOTHING`) and `SSL_net_read_desired` and `SSL_net_write_desired`
+will be used instead.
 
 #### `SSL_set_initial_peer_addr`, `SSL_get_initial_peer_addr`
 
@@ -623,17 +611,17 @@ A QUIC connection can be shut down using this function in two different ways:
   an I/O would-block condition.
 
 It is permissible for an application to implement a hybrid approach, for example
-by initiating a rapid or non-blocking shutdown and continuing to call `SSL_tick`
-for a duration it chooses.
+by initiating a rapid or non-blocking shutdown and continuing to call
+`SSL_handle_events` for a duration it chooses.
 
 If `SSL_SHUTDOWN_FLAG_RAPID` is specified in `flags`, a rapid shutdown is
 performed, otherwise an RFC-compliant shutdown is performed. The principal
 effect of this flag is to partially disable blocking behaviour in blocking mode,
 and the QUIC implementation will still attempt to implement the Terminating
-state semantics if the application happens to tick it, until it reaches the
-Terminated state or is freed. An application can change its mind about
-performing a rapid shutdown by making a subsequent call to `SSL_shutdown_ex`
-without the flag set.
+state semantics if the application happens to call `SSL_handle_events`, until it
+reaches the Terminated state or is freed. An application can change its mind
+about performing a rapid shutdown by making a subsequent call to
+`SSL_shutdown_ex` without the flag set.
 
 Calling `SSL_shutdown_ex` on a QUIC stream SSL object is not valid; such a call
 will fail and has no effect. The rationale for this is that an application may
@@ -850,8 +838,8 @@ typedef struct ssl_conn_close_info_st {
     uint64_t error_code;
     char     *reason;
     size_t   reason_len;
-    char     is_local;
-    char     is_transport;
+    int      is_local;
+    int      is_transport;
 } SSL_CONN_CLOSE_INFO;
 
 int SSL_get_conn_close_info(SSL *ssl,
@@ -1080,7 +1068,7 @@ SSL *SSL_accept_stream(SSL *ssl, uint64_t flags);
 size_t SSL_get_accept_stream_queue_len(SSL *ssl);
 ```
 
-#### `SSL_set_incoming_stream_reject_policy`
+#### `SSL_set_incoming_stream_policy`
 
 | Semantics | `SSL_get_error` | Can Tick? | CSHL          |
 | --------- | ------------- | --------- | ------------- |
@@ -1090,9 +1078,7 @@ size_t SSL_get_accept_stream_queue_len(SSL *ssl);
 /*
  * Sets the policy for incoming streams. If `policy` is `AUTO` (the default):
  *
- *   - if `SSL_detach_stream` has been used, this is equivalent to `ACCEPT`;
- *
- *   - otherwise, if the default stream mode is
+ *   - if the default stream mode is
  *     `SSL_DEFAULT_STREAM_MODE_AUTO_BIDI` or
  *     `SSL_DEFAULT_STREAM_MODE_AUTO_UNI`, this is equivalent to `REJECT`;
  *
@@ -1107,11 +1093,11 @@ size_t SSL_get_accept_stream_queue_len(SSL *ssl);
  * used for the purposes of this termination. The default AEC value used if this
  * function is never called is 0.
  */
-#define SSL_INCOMING_STREAM_REJECT_POLICY_AUTO      0
-#define SSL_INCOMING_STREAM_REJECT_POLICY_ACCEPT    1
-#define SSL_INCOMING_STREAM_REJECT_POLICY_REJECT    2
+#define SSL_INCOMING_STREAM_POLICY_AUTO      0
+#define SSL_INCOMING_STREAM_POLICY_ACCEPT    1
+#define SSL_INCOMING_STREAM_POLICY_REJECT    2
 
-int SSL_set_incoming_stream_reject_policy(SSL *ssl, int policy, uint64_t aec);
+int SSL_set_incoming_stream_policy(SSL *ssl, int policy, uint64_t aec);
 ```
 
 #### `SSL_set_default_stream_mode`
@@ -1162,8 +1148,7 @@ int SSL_set_incoming_stream_reject_policy(SSL *ssl, int policy, uint64_t aec);
  *
  * This function must be called before a default stream object is created, for
  * example before initiating a connection. If the function is too late to have
- * an effect, this function fails and returns 0. To switch to multi-stream
- * operation after a default stream has been created, use `SSL_detach_stream`.
+ * an effect, this function fails and returns 0.
  */
 #define SSL_DEFAULT_STREAM_MODE_NONE                0
 #define SSL_DEFAULT_STREAM_MODE_AUTO_BIDI           1
@@ -1172,44 +1157,6 @@ int SSL_set_incoming_stream_reject_policy(SSL *ssl, int policy, uint64_t aec);
 __owur int SSL_set_default_stream_mode(SSL *ssl, uint32_t mode);
 ```
 
-#### `SSL_detach_stream`
-
-| Semantics | `SSL_get_error` | Can Tick? | CSHL          |
-| --------- | ------------- | --------- | ------------- |
-| New       | Never         | No        | C             |
-
-```c
-/*
- * Detaches a default stream from a QUIC connection object. If the
- * QUIC connection object does not contain a default stream, returns NULL.
- * After calling this, calling SSL_get_stream_type on the connection object
- * returns SSL_STREAM_TYPE_NONE. Always returns NULL for non-QUIC connections.
- *
- * Calling this function automatically inhibits default stream creation;
- * though, after calling this function, a QUIC connection SSL object will no
- * longer have a stream attached to it, calling SSL_read or SSL_write on
- * that QUIC connection SSL object will not automatically create a new
- * default stream. Default stream creation only occurs at most a single time per
- * connection.
- */
-SSL *SSL_detach_stream(SSL *ssl);
-```
-
-#### `SSL_attach_stream`
-
-| Semantics | `SSL_get_error` | Can Tick? | CSHL          |
-| --------- | ------------- | --------- | ------------- |
-| New       | Never         | No        | C             |
-
-```c
-/*
- * Attaches a default stream to a QUIC connection object. If the conn object is
- * not a QUIC connection object, or already has a default stream, this function
- * fails. The stream must belong to the same connection, or this function fails.
- */
-__owur int SSL_attach_stream(SSL *conn, SSL *stream);
-```
-
 ### Future APIs
 
 A custom poller interface may be provided in the future. For more information,
@@ -1576,7 +1523,8 @@ draining state is relevant. Since we conclude above that we do not need to
 implement the draining state on the client side, this means that connection
 closure can be completed immediately in the case of a remote closure.
 
-**Q. Should we just map `SSL_tick` to `DTLS_CTRL_HANDLE_TIMEOUT` internally?**
+**Q. Should we just map `SSL_handle_events` to `DTLS_CTRL_HANDLE_TIMEOUT`
+internally?**
 
 A. No, since the infinite time representation is different between the two
 calls.