Repository: randombit/botan
Pull request #2988 (2022-06-07 12:15:11)

[TLS 1.3] Overhaul TLS::Callbacks
## Status

This is meant to collect changes to the TLS callbacks (i.e. user-facing API changes). Currently, it is based on `master` where all client-side TLS 1.3 feature branches are merged. Changes required for [the server implementation](https://github.com/randombit/botan/pull/3053) are additionally cherry-picked here. Note that this builds on the ideas outlined [here](https://github.com/randombit/botan/issues/2714#issuecomment-1057175631).

## Callbacks that need attention

Here is a collection of insights we had during implementation with suggestions how to tackle the new requirements TLS 1.3 brings to the implementations. Those changes are not yet implemented and might require some discussion.

### A word on 0-RTT

We don't have immediate plans to implement it. However, it would certainly have user-facing API implications. I have some hope to be able restrict the changes to _additional API_, rather than breaking changes, though. I.e. deferring 0-RTT support to after Botan 3.0 is released seems feasible to me.

### `tls_session_established(const Session&)` (mandatory callback)

Currently, applications are required to override that. The TLS 1.2 implementation calls it once a session is established, provides the established session's descriptor and applications must decide whether to persist the session for resumption or discard it. TLS 1.3 got rid of this type of session resumption. Instead it exclusively uses session tickets and allows multiple such tickets per connection (see `Callbacks::tls_session_ticket_received`).

I don't see a reasonable way to serve this callback with TLS 1.3. @randombit

Ideas:

* **replace `tls_session_established` using a dedicated "session descriptor"**
  * deprecate the old callback and keep serving it from TLS 1.2 for now
  * use `tls_session_ticket_received` for TLS 1.2 session resumption storage decision
    * might mandate a different name (`tls_session_should_be_stored` ?)
  * new `tls_session_established`
    * pass a `TLS::Summary` object, containing info about the new session, e.g
      * protocol version
      * negotiated ciphersuite
      * extended master secret?
      * encrypt-then-MAC?
      * ...
    * would be purely "informational" (callback returns `void`)
    * might obsolete `tls_session_activated()` as it carries roughly the same info without the `TLS::Summary`
* **rename `tls_session_established()` to `tls_12_session_established()`**
  * probably shouldn't be mandatory
    * default implementation always returns `true`, I guess
  * probably also needs a way to get a connection summary (`TLS::Summary` as described above)

#### Side-Note: New `TLS::Server` API to send new tickets

[TLS 1.3 allows for an arbitrary number of tickets at any time after the handshake](https://www.rfc-editor.org/rfc/rfc8446.html#section-4.6.1) not just exactly one at the end of the handshake. Hence, we'll need something like `TLS::Server::send_new_session_ticket(std::chrono::seconds lifetime)` allowing applications to send tickets at their own discretion.

#### Side-Note: Application-defined Session Ticket content

[RFC 8446 4.6.1](https://www.rfc-editor.org/rfc/rfc8446#section-4.6.1): "The ticket itself is an opaque label. It MAY be either a database lookup key or a self-encrypted and self-authenticated value." And I think we should allow the application to decide that.

Given the current `Session_Manager` API, we might extend the sematics of `::save(const Session&)` to return the "ticket" where applications could return a lookup handle or the result of `Session::encrypt()`. `Session_Manager::load_from_session_id()` already has the right function signature to act as a counter-part. Nevertheless, I'd suggest to rename it to something like `restore_from_ticket()` for clarity.

### `tls_dh_agree()` / `tls_ecdh_agree()` along with `tls_decode_group_param()`

The agreement callbacks receive a public value along with a group specification. They generate a new keypair, perform an agreement and return a tuple of the shared secret and the public value to pass to the peer.

TLS 1.3 encourages clients to offer initial key share values. That allows the server to encrypt most of its first flight. In contrast to TLS 1.2 where the initial key offer is provided by the server in its first flight.

Technically, we could use the above-mentioned callbacks for TLS 1.3 as well. Main difference: The `tls_xx_agree()` callbacks would be called *in clients* in a TLS 1.2 session establishment and *in servers* in a TLS 1.3 session establishment. That might cause confusion.
Additionally, the callbacks are currently not used for the key agreement in the opposite direction. Instead the `PK_Key_Agreement` is hard-coded.

With KEMs entering the game for PQC-ready TLS we would likely want to add a `tls_kem_agree()` if we decide to keep those callbacks at all.

If we decide to keep these callbacks, I would suggest to separate ephemeral key generation from the actual agreement. Then both client and server as well as TLS 1.2 and 1.3 could make use of the same callback. To really be useful, one would probably also want to extract the key generation into a callback. Also, we might even abstract from the DH, ECDH (and KEM) differentiation and just have two callbacks, like so:

```C++
struct {
  std::vector<uint8_t> public_value;
  std::unique_ptr<Private_Key> private_key;
} tls_generate_ephemeral_key(Group_Params params,
                             bool compressed_public_value,
                             RandomNumberGenerator& rng, ...);

secure_vector<uint8_t>
tls_key_agreement(Group_Params         params,
                  std::vector<uint8_t> peer_public_value,
                  Private_Key& local_private_key, 
                  ...);
```

Does that sound about right? Somehow, I feel I'm missing the gist of that interface. @randombit 

### `tls_log_error()`, `...debug()` and `debug_bin()`

Not actually related to TLS 1.3; though none of these callbacks are used. Now might be a good time to remove them. YAGNI. ;)

## Modifications at a Glance

Those modifications are already implemented and merged or are proposed as working implementations in this pull request.

* **tls_modify_extensions**
  Now has an additional parameter (`which_message`) that provides context on what handshake message contains the extensions to be modified. The new overload is called for extensible messages introduced in TLS 1.3. The old overload is marked *deprecated* and is called by the default implementation of the new overload (after filtering out new TLS 1.3 extensible messages to keep its behaviour as close to legacy as possible).

* **tls_examine_extensions**
  Analogous to the modify callback above.

* **tls_session_ticket_received**
  Added for TLS 1.3 session tickets. Not touched in this pull request.

* **tls_parse_ocsp_response**
  Added to influence the OCSP parsing behaviour. Mainly used to better support BoGo tests which send invalid responses that need to be ignored. Not touched in this pull request.

* **tls_current_timestamp**
  Added to inject user-defined timestamps. Mainly used for deterministic testing (using test vectors from RFC 8448) and certain BoGo tests that define a specific waiting period. Not touched in this pull request.

* **tls_peer_closed_connection**
  In contrast to older revisions, TLS 1.3 does not mandate an immediate in-kind response when receiving "close_notify". Applications can override this callback to get notified that no more data is to be expected from the peer. With TLS 1.3 they might legally continue sending data and finish with `Channel::close()` at their own discretion. Returning `true` from this callback will make TLS 1.3 behave just like 1.2 and reply with a "close_notify" immediately. This is the default behaviour.

* **tls_provide_cert_chain_status**
  This allows applications to provide stapled OCSP responses for all (or some) intermediate certificates added to the Certificate handshake message. To the best of my understanding, previous versions of TLS allowed stapled OCSP responses for the leaf certificate, only.
By default, this callback invokes the old tls_provide_cert_status() callback: mimicking the behavior of TLS 1.2 and stapling an OCSP response for the leaf certificate, only.

codecov-commenter: # [Codecov](https://codecov.io/gh/randombit/botan/pull/2988?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd) Report
Base: **92.55**% // Head: **92.59**% // Increases project coverage by **`+0.03%`** :tada:
> Coverage data is based on head [(`8d8f518`)](https://codecov.io/gh/randombit/botan/pull/2988?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd) compared to base [(`864cbf2`)](https://codecov.io/gh/randombit/botan/commit/864cbf2760c862ed6ff1b15d9f046dcad4cbd6d0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #2988      +/-   ##
==========================================
+ Coverage   92.55%   92.59%   +0.03%     
==========================================
  Files         601      602       +1     
  Lines       70608    70702      +94     
  Branches     6666     6669       +3     
==========================================
+ Hits        65353    65468     +115     
+ Misses       5225     5204      -21     
  Partials       30       30              
```


| [Impacted Files](https://codecov.io/gh/randombit/botan/pull/2988?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd) | Coverage Δ | |
|---|---|---|
| [src/lib/tls/tls13/tls\_channel\_impl\_13.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvdGxzMTMvdGxzX2NoYW5uZWxfaW1wbF8xMy5jcHA=) | `94.85% <ø> (+0.27%)` | :arrow_up: |
| [src/lib/tls/msg\_client\_hello.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvbXNnX2NsaWVudF9oZWxsby5jcHA=) | `92.72% <100.00%> (ø)` | |
| [src/lib/tls/msg\_server\_hello.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvbXNnX3NlcnZlcl9oZWxsby5jcHA=) | `93.36% <100.00%> (+0.41%)` | :arrow_up: |
| [src/lib/tls/tls12/tls\_channel\_impl\_12.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvdGxzMTIvdGxzX2NoYW5uZWxfaW1wbF8xMi5jcHA=) | `88.82% <100.00%> (+0.03%)` | :arrow_up: |
| [src/lib/tls/tls12/tls\_client\_impl\_12.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvdGxzMTIvdGxzX2NsaWVudF9pbXBsXzEyLmNwcA==) | `91.54% <100.00%> (ø)` | |
| [src/lib/tls/tls12/tls\_server\_impl\_12.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvdGxzMTIvdGxzX3NlcnZlcl9pbXBsXzEyLmNwcA==) | `88.41% <100.00%> (ø)` | |
| [src/lib/tls/tls13/msg\_certificate\_13.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvdGxzMTMvbXNnX2NlcnRpZmljYXRlXzEzLmNwcA==) | `91.86% <100.00%> (+0.29%)` | :arrow_up: |
| [src/lib/tls/tls13/tls\_client\_impl\_13.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvdGxzMTMvdGxzX2NsaWVudF9pbXBsXzEzLmNwcA==) | `95.78% <100.00%> (+0.48%)` | :arrow_up: |
| [src/lib/tls/tls\_callbacks.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL2xpYi90bHMvdGxzX2NhbGxiYWNrcy5jcHA=) | `84.53% <100.00%> (+3.52%)` | :arrow_up: |
| [src/tests/test\_tls\_rfc8448.cpp](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd#diff-c3JjL3Rlc3RzL3Rlc3RfdGxzX3JmYzg0NDguY3Bw) | `90.00% <100.00%> (+0.56%)` | :arrow_up: |
| ... and [20 more](https://codecov.io/gh/randombit/botan/pull/2988/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd) | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/randombit/botan/pull/2988?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jack+Lloyd).


Patch:
diff --git a/src/lib/tls/asio/asio_stream.h b/src/lib/tls/asio/asio_stream.h
index 3da047155..abd99119f 100644
--- a/src/lib/tls/asio/asio_stream.h
+++ b/src/lib/tls/asio/asio_stream.h
@@ -574,6 +574,9 @@ class Stream
       //! @}
 
       //! @brief Indicates whether a close_notify alert has been received from the peer.
+      //!
+      //! Note that we cannot m_core.is_closed_for_reading() because this wants to
+      //! explicitly check that the peer sent close_notify.
       bool shutdown_received() const
          {
          return m_core.shutdown_received;
@@ -613,6 +616,13 @@ class Stream
                );
                }
 
+            bool tls_peer_closed_connection() override
+               {
+               // Instruct the TLS implementation to reply with our close_notify to obtain
+               // the same behaviour for TLS 1.2 and TLS 1.3.
+               return true;
+               }
+
             void tls_alert(TLS::Alert alert) override
                {
                if(alert.type() == TLS::Alert::CLOSE_NOTIFY)
diff --git a/src/lib/tls/msg_client_hello.cpp b/src/lib/tls/msg_client_hello.cpp
index 0675fd97f..4ceb0afa9 100644
--- a/src/lib/tls/msg_client_hello.cpp
+++ b/src/lib/tls/msg_client_hello.cpp
@@ -414,7 +414,7 @@ Client_Hello_12::Client_Hello_12(Handshake_IO& io,
    if(m_legacy_version.is_datagram_protocol())
       { m_extensions.add(new SRTP_Protection_Profiles(policy.srtp_profiles())); }
 
-   cb.tls_modify_extensions(m_extensions, CLIENT);
+   cb.tls_modify_extensions(m_extensions, CLIENT, type());
 
    hash.update(io.send(*this));
    }
@@ -477,7 +477,7 @@ Client_Hello_12::Client_Hello_12(Handshake_IO& io,
    if(reneg_info.empty() && !next_protocols.empty())
       { m_extensions.add(new Application_Layer_Protocol_Notification(next_protocols)); }
 
-   cb.tls_modify_extensions(m_extensions, CLIENT);
+   cb.tls_modify_extensions(m_extensions, CLIENT, type());
 
    hash.update(io.send(*this));
    }
@@ -567,7 +567,7 @@ Client_Hello_13::Client_Hello_13(const Policy& policy,
    // TODO: Some extensions require a certain order or pose other assumptions.
    //       We should check those after the user was allowed to make changes to
    //       the extensions.
-   cb.tls_modify_extensions(m_extensions, CLIENT);
+   cb.tls_modify_extensions(m_extensions, CLIENT, type());
 
    // RFC 8446 4.2.11
    //    The "pre_shared_key" extension MUST be the last extension in the
@@ -610,10 +610,11 @@ void Client_Hello_13::retry(const Hello_Retry_Request& hrr,
       m_extensions.add(new Cookie(hrr.extensions().get<Cookie>()->get_cookie()));
       }
 
-   // TODO: the consumer of the TLS implementation won't be able to distinguish
+   // Note: the consumer of the TLS implementation won't be able to distinguish
    //       invocations to this callback due to the first Client_Hello or the
-   //       retried Client_Hello after receiving a Hello_Retry_Request.
-   cb.tls_modify_extensions(m_extensions, CLIENT);
+   //       retried Client_Hello after receiving a Hello_Retry_Request. We assume
+   //       that the user keeps and detects this state themselves.
+   cb.tls_modify_extensions(m_extensions, CLIENT, type());
 
    auto psk = m_extensions.get<PSK>();
    if(psk)
diff --git a/src/lib/tls/msg_server_hello.cpp b/src/lib/tls/msg_server_hello.cpp
index 491d118dc..397203018 100644
--- a/src/lib/tls/msg_server_hello.cpp
+++ b/src/lib/tls/msg_server_hello.cpp
@@ -291,7 +291,7 @@ Server_Hello_12::Server_Hello_12(Handshake_IO& io,
          }
       }
 
-   cb.tls_modify_extensions(m_data->extensions, SERVER);
+   cb.tls_modify_extensions(m_data->extensions, SERVER, type());
 
    hash.update(io.send(*this));
    }
@@ -348,7 +348,7 @@ Server_Hello_12::Server_Hello_12(Handshake_IO& io,
       m_data->extensions.add(new Session_Ticket());
       }
 
-   cb.tls_modify_extensions(m_data->extensions, SERVER);
+   cb.tls_modify_extensions(m_data->extensions, SERVER, type());
 
    hash.update(io.send(*this));
    }
diff --git a/src/lib/tls/tls12/tls_channel_impl_12.cpp b/src/lib/tls/tls12/tls_channel_impl_12.cpp
index 49a394a77..3a9a0b410 100644
--- a/src/lib/tls/tls12/tls_channel_impl_12.cpp
+++ b/src/lib/tls/tls12/tls_channel_impl_12.cpp
@@ -501,7 +501,12 @@ void Channel_Impl_12::process_alert(const secure_vector<uint8_t>& record)
        }
 
     if(alert_msg.type() == Alert::CLOSE_NOTIFY)
+       {
+       // TLS 1.2 requires us to immediately react with our "close_notify",
+       // the return value of the application's callback has no effect on that.
+       callbacks().tls_peer_closed_connection();
        send_warning_alert(Alert::CLOSE_NOTIFY); // reply in kind
+       }
 
     if(alert_msg.type() == Alert::CLOSE_NOTIFY || alert_msg.is_fatal())
        {
diff --git a/src/lib/tls/tls12/tls_client_impl_12.cpp b/src/lib/tls/tls12/tls_client_impl_12.cpp
index 65c34e51d..d8ed2f823 100644
--- a/src/lib/tls/tls12/tls_client_impl_12.cpp
+++ b/src/lib/tls/tls12/tls_client_impl_12.cpp
@@ -352,7 +352,7 @@ void Client_Impl_12::process_handshake_msg(const Handshake_State* active_state,
                                 "Server replied with DTLS-SRTP alg we did not send");
          }
 
-      callbacks().tls_examine_extensions(state.server_hello()->extensions(), SERVER);
+      callbacks().tls_examine_extensions(state.server_hello()->extensions(), SERVER, Handshake_Type::SERVER_HELLO);
 
       state.set_version(state.server_hello()->legacy_version());
       m_application_protocol = state.server_hello()->next_protocol();
diff --git a/src/lib/tls/tls12/tls_server_impl_12.cpp b/src/lib/tls/tls12/tls_server_impl_12.cpp
index 5ee63a9e9..8f9ab008f 100644
--- a/src/lib/tls/tls12/tls_server_impl_12.cpp
+++ b/src/lib/tls/tls12/tls_server_impl_12.cpp
@@ -483,7 +483,7 @@ void Server_Impl_12::process_client_hello_msg(const Handshake_State* active_stat
 
    secure_renegotiation_check(pending_state.client_hello());
 
-   callbacks().tls_examine_extensions(pending_state.client_hello()->extensions(), CLIENT);
+   callbacks().tls_examine_extensions(pending_state.client_hello()->extensions(), CLIENT, Handshake_Type::CLIENT_HELLO);
 
    Session session_info;
    const bool resuming =
diff --git a/src/lib/tls/tls13/msg_certificate_13.cpp b/src/lib/tls/tls13/msg_certificate_13.cpp
index 35b822af2..b2e6e3060 100644
--- a/src/lib/tls/tls13/msg_certificate_13.cpp
+++ b/src/lib/tls/tls13/msg_certificate_13.cpp
@@ -35,7 +35,7 @@ bool certificate_allows_signing(const X509_Certificate& cert)
 
 }
 
-void Certificate_13::validate_extensions(const std::set<Handshake_Extension_Type>& requested_extensions) const
+void Certificate_13::validate_extensions(const std::set<Handshake_Extension_Type>& requested_extensions, Callbacks& cb) const
    {
    // RFC 8446 4.4.2
    //    Extensions in the Certificate message from the server MUST
@@ -46,6 +46,8 @@ void Certificate_13::validate_extensions(const std::set<Handshake_Extension_Type
       {
       if(entry.extensions.contains_other_than(requested_extensions))
          { throw TLS_Exception(Alert::ILLEGAL_PARAMETER, "Certificate Entry contained an extension that was not offered"); }
+
+      cb.tls_examine_extensions(entry.extensions, m_side, type());
       }
    }
 
@@ -104,13 +106,17 @@ void Certificate_13::verify(Callbacks& callbacks,
 
 Certificate_13::Certificate_13(const std::vector<X509_Certificate>& certs,
                                const Connection_Side side,
-                               const std::vector<uint8_t> &request_context)
+                               const std::vector<uint8_t> &request_context,
+                               Callbacks& callbacks)
    : m_request_context(request_context)
    , m_side(side)
    {
-   // TODO: proper extensions
    for (const auto& c : certs)
-      { m_entries.emplace_back(Certificate_Entry{c, Extensions()}); }
+      {
+      auto exts = Extensions();
+      callbacks.tls_modify_extensions(exts, side, type());
+      m_entries.emplace_back(Certificate_Entry{c, std::move(exts)});
+      }
    }
 
 /**
diff --git a/src/lib/tls/tls13/tls_channel_impl_13.cpp b/src/lib/tls/tls13/tls_channel_impl_13.cpp
index 06a36cd01..f04df02b4 100644
--- a/src/lib/tls/tls13/tls_channel_impl_13.cpp
+++ b/src/lib/tls/tls13/tls_channel_impl_13.cpp
@@ -398,6 +398,10 @@ void Channel_Impl_13::process_alert(const secure_vector<uint8_t>& record)
       shutdown();
 
    callbacks().tls_alert(alert);
+
+   // Respond with our "close_notify" if the application requests us to.
+   if(is_close_notify_alert(alert) && callbacks().tls_peer_closed_connection())
+      close();
    }
 
 void Channel_Impl_13::shutdown()
diff --git a/src/lib/tls/tls13/tls_client_impl_13.cpp b/src/lib/tls/tls13/tls_client_impl_13.cpp
index 5774b4a64..aceb89c4a 100644
--- a/src/lib/tls/tls13/tls_client_impl_13.cpp
+++ b/src/lib/tls/tls13/tls_client_impl_13.cpp
@@ -331,7 +331,7 @@ void Client_Impl_13::handle(const Server_Hello_13& sh)
                                                             m_transcript_hash.current());
       }
 
-   callbacks().tls_examine_extensions(sh.extensions(), SERVER);
+   callbacks().tls_examine_extensions(sh.extensions(), SERVER, Handshake_Type::SERVER_HELLO);
 
    m_transitions.set_expected_next(ENCRYPTED_EXTENSIONS);
    }
@@ -365,7 +365,7 @@ void Client_Impl_13::handle(const Hello_Retry_Request& hrr)
 
    ch.retry(hrr, m_transcript_hash, callbacks(), rng());
 
-   callbacks().tls_examine_extensions(hrr.extensions(), SERVER);
+   callbacks().tls_examine_extensions(hrr.extensions(), SERVER, Handshake_Type::HELLO_RETRY_REQUEST);
 
    send_handshake_message(ch);
 
@@ -377,20 +377,22 @@ void Client_Impl_13::handle(const Hello_Retry_Request& hrr)
 
 void Client_Impl_13::handle(const Encrypted_Extensions& encrypted_extensions_msg)
    {
+   const auto& exts = encrypted_extensions_msg.extensions();
+
    // RFC 8446 4.2
    //    Implementations MUST NOT send extension responses if the remote
    //    endpoint did not send the corresponding extension requests, [...]. Upon
    //    receiving such an extension, an endpoint MUST abort the handshake
    //    with an "unsupported_extension" alert.
    const auto& requested_exts = m_handshake_state.client_hello().extensions().extension_types();
-   if(encrypted_extensions_msg.extensions().contains_other_than(requested_exts))
+   if(exts.contains_other_than(requested_exts))
       { throw TLS_Exception(Alert::UNSUPPORTED_EXTENSION,
             "Encrypted Extensions contained an extension that was not offered"); }
 
    // Note: As per RFC 6066 3. we can check for an empty SNI extensions to
    // determine if the server used the SNI we sent here.
 
-   if(encrypted_extensions_msg.extensions().has<Record_Size_Limit>() &&
+   if(exts.has<Record_Size_Limit>() &&
       m_handshake_state.client_hello().extensions().has<Record_Size_Limit>())
       {
       // RFC 8449 4.
@@ -400,12 +402,12 @@ void Client_Impl_13::handle(const Encrypted_Extensions& encrypted_extensions_msg
       //
       // Hence, the "outgoing" limit is what the server requested and the
       // "incoming" limit is what we requested in the Client Hello.
-      const auto outgoing_limit = encrypted_extensions_msg.extensions().get<Record_Size_Limit>();
+      const auto outgoing_limit = exts.get<Record_Size_Limit>();
       const auto incoming_limit = m_handshake_state.client_hello().extensions().get<Record_Size_Limit>();
       set_record_size_limits(outgoing_limit->limit(), incoming_limit->limit());
       }
 
-   callbacks().tls_examine_extensions(encrypted_extensions_msg.extensions(), SERVER);
+   callbacks().tls_examine_extensions(exts, SERVER, Handshake_Type::ENCRYPTED_EXTENSIONS);
 
    if(m_handshake_state.server_hello().extensions().has<PSK>())
       {
@@ -430,6 +432,7 @@ void Client_Impl_13::handle(const Certificate_Request_13& certificate_request_ms
       throw TLS_Exception(Alert::DECODE_ERROR, "Certificate_Request context must be empty in the main handshake");
       }
 
+   callbacks().tls_examine_extensions(certificate_request_msg.extensions(), SERVER, Handshake_Type::CERTIFICATE_REQUEST);
    m_transitions.set_expected_next(CERTIFICATE);
    }
 
@@ -443,7 +446,7 @@ void Client_Impl_13::handle(const Certificate_13& certificate_msg)
       throw TLS_Exception(Alert::DECODE_ERROR, "Received a server certificate message with non-empty request context");
       }
 
-   certificate_msg.validate_extensions(m_handshake_state.client_hello().extensions().extension_types());
+   certificate_msg.validate_extensions(m_handshake_state.client_hello().extensions().extension_types(), callbacks());
    certificate_msg.verify(callbacks(),
                           policy(),
                           credentials_manager(),
@@ -520,7 +523,7 @@ void Client_Impl_13::send_client_authentication(Channel_Impl_13::AggregatedMessa
    //       CertificateRequest, the value of certificate_request_context in
    //       that message.
    flight.add(m_handshake_state.sending(
-      Certificate_13(std::move(client_certs), CLIENT, cert_request.context())));
+      Certificate_13(std::move(client_certs), CLIENT, cert_request.context(), callbacks())));
 
    // RFC 4.4.2
    //    If the server requests client authentication but no suitable certificate
@@ -605,6 +608,7 @@ void TLS::Client_Impl_13::handle(const New_Session_Ticket_13& new_session_ticket
                    m_info,
                    callbacks().tls_current_timestamp());
 
+   callbacks().tls_examine_extensions(new_session_ticket.extensions(), SERVER, Handshake_Type::NEW_SESSION_TICKET);
    if(callbacks().tls_session_ticket_received(session))
       {
       session_manager().save(session);
diff --git a/src/lib/tls/tls_callbacks.cpp b/src/lib/tls/tls_callbacks.cpp
index b2cd49425..f6d08d1a7 100644
--- a/src/lib/tls/tls_callbacks.cpp
+++ b/src/lib/tls/tls_callbacks.cpp
@@ -2,6 +2,7 @@
 * TLS Callbacks
 * (C) 2016 Jack Lloyd
 *     2017 Harry Reimann, Rohde & Schwarz Cybersecurity
+*     2022 René Meusel, Hannes Rantzsch - neXenio GmbH
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -42,11 +43,11 @@ std::chrono::system_clock::time_point TLS::Callbacks::tls_current_timestamp()
    return std::chrono::system_clock::now();
    }
 
-void TLS::Callbacks::tls_modify_extensions(Extensions& /*unused*/, Connection_Side /*unused*/)
+void TLS::Callbacks::tls_modify_extensions(Extensions& /*unused*/, Connection_Side /*unused*/, Handshake_Type /*unused*/)
    {
    }
 
-void TLS::Callbacks::tls_examine_extensions(const Extensions& /*unused*/, Connection_Side /*unused*/)
+void TLS::Callbacks::tls_examine_extensions(const Extensions& /*unused*/, Connection_Side /*unused*/, Handshake_Type /*unused*/)
    {
    }
 
@@ -108,6 +109,19 @@ std::optional<OCSP::Response> TLS::Callbacks::tls_parse_ocsp_response(const std:
       }
    }
 
+
+std::vector<std::vector<uint8_t>> TLS::Callbacks::tls_provide_cert_chain_status(
+   const std::vector<X509_Certificate>& chain,
+   const Certificate_Status_Request& csr)
+   {
+   std::vector<std::vector<uint8_t>> result(chain.size());
+   if(!chain.empty())
+      {
+      result[0] = tls_provide_cert_status(chain, csr);
+      }
+   return result;
+   }
+
 std::vector<uint8_t> TLS::Callbacks::tls_sign_message(
    const Private_Key& key,
    RandomNumberGenerator& rng,
diff --git a/src/lib/tls/tls_callbacks.h b/src/lib/tls/tls_callbacks.h
index af32dc4fa..709d9fc54 100644
--- a/src/lib/tls/tls_callbacks.h
+++ b/src/lib/tls/tls_callbacks.h
@@ -3,6 +3,7 @@
 * (C) 2016 Matthias Gierlings
 *     2016 Jack Lloyd
 *     2017 Harry Reimann, Rohde & Schwarz Cybersecurity
+*     2022 René Meusel, Rohde & Schwarz Cybersecurity
 *
 * Botan is released under the Simplified BSD License (see license.txt)
 */
@@ -98,6 +99,31 @@ class BOTAN_PUBLIC_API(2,0) Callbacks
        */
        virtual void tls_session_activated() {}
 
+       /**
+       * Optional callback: peer closed connection (sent a "close_notify" alert)
+       *
+       * The peer signaled that it wishes to shut down the connection. The
+       * application should not expect to receive any more data from the peer
+       * and may tear down the underlying transport socket.
+       *
+       * Prior to TLS 1.3 it was required that peers discard pending writes
+       * and immediately respond with their own "close_notify". With TLS 1.3,
+       * applications can continue to send data despite the peer having already
+       * signaled their wish to shut down.
+       *
+       * Returning `true` will cause the TLS 1.3 implementation to write all
+       * pending data and then also signal a connection shut down. Otherwise
+       * the application is responsible to call the `Channel::close()` method.
+       *
+       * For TLS 1.2 the return value has no effect.
+       *
+       * @return true causes the implementation to respond with a "close_notify"
+       */
+       virtual bool tls_peer_closed_connection()
+         {
+         return true;
+         }
+
        /**
        * Optional callback: New session ticket received
        * Called when we receive a session ticket from the server at any point
@@ -181,6 +207,22 @@ class BOTAN_PUBLIC_API(2,0) Callbacks
           return std::vector<uint8_t>();
           }
 
+      /**
+       * Called by TLS 1.3 client or server whenever the peer indicated that
+       * OCSP stapling is supported. In contrast to `tls_provide_cert_status`,
+       * this allows providing OCSP responses for each certificate in the chain.
+       *
+       * The default implementation invokes `tls_provide_cert_status` assuming
+       * that no OCSP responses for intermediate certificates are available.
+       *
+       * @return a vector of OCSP response buffers. An empty buffer indicates
+       *         that no OCSP response should be provided for the respective
+       *         certificate (at the same list index). The returned vector
+       *         MUST be exactly the same length as the incoming \p chain.
+       */
+      virtual std::vector<std::vector<uint8_t>> tls_provide_cert_chain_status(const std::vector<X509_Certificate>& chain,
+                                                                              const Certificate_Status_Request& csr);
+
        /**
        * Optional callback with default impl: sign a message
        *
@@ -309,21 +351,21 @@ class BOTAN_PUBLIC_API(2,0) Callbacks
        */
        virtual std::string tls_server_choose_app_protocol(const std::vector<std::string>& client_protos);
 
-       /**
+      /**
        * Optional callback: examine/modify Extensions before sending.
        *
        * Both client and server will call this callback on the Extensions object
-       * before serializing it in the client/server hellos. This allows an
-       * application to modify which extensions are sent during the
-       * handshake.
+       * before serializing it in the specific handshake message. This allows an
+       * application to modify which extensions are sent during the handshake.
        *
        * Default implementation does nothing.
        *
        * @param extn the extensions
        * @param which_side will be CLIENT or SERVER which is the current
-       * applications role in the exchange.
+       *                   applications role in the exchange.
+       * @param which_message will state the handshake message type containing the extensions
        */
-       virtual void tls_modify_extensions(Extensions& extn, Connection_Side which_side);
+       virtual void tls_modify_extensions(Extensions& extn, Connection_Side which_side, Handshake_Type which_message);
 
        /**
        * Optional callback: examine peer extensions.
@@ -339,8 +381,9 @@ class BOTAN_PUBLIC_API(2,0) Callbacks
        * @param extn the extensions
        * @param which_side will be CLIENT if these are are the clients extensions (ie we are
        *        the server) or SERVER if these are the server extensions (we are the client).
+       * @param which_message will state the handshake message type containing the extensions
        */
-       virtual void tls_examine_extensions(const Extensions& extn, Connection_Side which_side);
+       virtual void tls_examine_extensions(const Extensions& extn, Connection_Side which_side, Handshake_Type which_message);
 
        /**
        * Optional callback: decode TLS group ID
diff --git a/src/lib/tls/tls_magic.h b/src/lib/tls/tls_magic.h
index a53c46705..fbbd1f0ad 100644
--- a/src/lib/tls/tls_magic.h
+++ b/src/lib/tls/tls_magic.h
@@ -88,7 +88,7 @@ enum Handshake_Type {
    HANDSHAKE_NONE       = 255  // Null value
 };
 
-const char* handshake_type_to_string(Handshake_Type t);
+BOTAN_TEST_API const char* handshake_type_to_string(Handshake_Type t);
 
 using Transcript_Hash = std::vector<uint8_t>;
 
diff --git a/src/lib/tls/tls_messages.h b/src/lib/tls/tls_messages.h
index 9848c758b..117def351 100644
--- a/src/lib/tls/tls_messages.h
+++ b/src/lib/tls/tls_messages.h
@@ -509,12 +509,15 @@ class BOTAN_UNSTABLE_API Certificate_13 final : public Handshake_Message
        * Create a Certificate message
        *
        * @param certs            the certificate (chain) to be used for authentication
+       * @param side             the connection side which constructs this message
        * @param request_context  the context provided in the Certificate Request
        *                         message when performing post-handshake client auth
+       * @param callbacks        the "callbacks" handle to use (for tls_modify_extensions)
        */
       Certificate_13(const std::vector<X509_Certificate>& certs,
                      const Connection_Side side,
-                     const std::vector<uint8_t> &request_context = {});
+                     const std::vector<uint8_t>& request_context,
+                     Callbacks& callbacks);
 
       /**
       * Deserialize a Certificate message
@@ -528,11 +531,12 @@ class BOTAN_UNSTABLE_API Certificate_13 final : public Handshake_Message
 
       /**
       * Validate a Certificate message regarding what extensions are expected based on
-      * previous handshake messages.
+      * previous handshake messages. Also call the tls_examine_extenions() callback
+      * for each entry.
       *
       * @param requested_extensions Extensions of Client_Hello or Certificate_Request messages
       */
-      void validate_extensions(const std::set<Handshake_Extension_Type>& requested_extensions) const;
+      void validate_extensions(const std::set<Handshake_Extension_Type>& requested_extensions, Callbacks& cb) const;
 
       /**
        * Verify the certificate chain
@@ -623,6 +627,7 @@ class BOTAN_UNSTABLE_API Certificate_Request_13 final : public Handshake_Message
 
       std::vector<X509_DN> acceptable_CAs() const;
       const std::vector<Signature_Scheme>& signature_schemes() const;
+      const Extensions& extensions() const { return m_extensions; }
 
       std::vector<uint8_t> serialize() const override;
 
@@ -865,6 +870,8 @@ class BOTAN_UNSTABLE_API New_Session_Ticket_13 final : public Handshake_Message
 
       std::vector<uint8_t> serialize() const override;
 
+      const Extensions& extensions() const { return m_extensions; }
+
       const std::vector<uint8_t>& ticket() const { return m_ticket; }
       const std::vector<uint8_t>& nonce() const { return m_ticket_nonce; }
       uint32_t ticket_age_add() const { return m_ticket_age_add; }
diff --git a/src/lib/utils/compiler.h b/src/lib/utils/compiler.h
index 275062e2d..2f24e30ad 100644
--- a/src/lib/utils/compiler.h
+++ b/src/lib/utils/compiler.h
@@ -145,18 +145,22 @@
 
 #endif
 
-#if defined(BOTAN_BUILD_COMPILER_IS_GCC) || defined(BOTAN_BUILD_COMPILER_IS_CLANG)
-
-  #if defined(BOTAN_BUILD_COMPILER_IS_GCC)
-    #define BOTAN_DIAGNOSTIC_PUSH          	_Pragma("GCC diagnostic push")
-    #define BOTAN_DIAGNOSTIC_IGNORE_DEPRECATED  _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
-    #define BOTAN_DIAGNOSTIC_POP           	_Pragma("GCC diagnostic pop")
-  #elif defined(BOTAN_BUILD_COMPILER_IS_CLANG)
-    #define BOTAN_DIAGNOSTIC_PUSH          	_Pragma("clang diagnostic push")
-    #define BOTAN_DIAGNOSTIC_IGNORE_DEPRECATED  _Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"")
-    #define BOTAN_DIAGNOSTIC_POP           	_Pragma("clang diagnostic pop")
-  #endif
-
+#if defined(BOTAN_BUILD_COMPILER_IS_GCC)
+  #define BOTAN_DIAGNOSTIC_PUSH              _Pragma("GCC diagnostic push")
+  #define BOTAN_DIAGNOSTIC_IGNORE_DEPRECATED _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
+  #define BOTAN_DIAGNOSTIC_POP               _Pragma("GCC diagnostic pop")
+#elif defined(BOTAN_BUILD_COMPILER_IS_CLANG)
+  #define BOTAN_DIAGNOSTIC_PUSH              _Pragma("clang diagnostic push")
+  #define BOTAN_DIAGNOSTIC_IGNORE_DEPRECATED _Pragma("clang diagnostic ignored \"-Wdeprecated-declarations\"")
+  #define BOTAN_DIAGNOSTIC_POP               _Pragma("clang diagnostic pop")
+#elif defined(BOTAN_BUILD_COMPILER_IS_MSVC)
+  #define BOTAN_DIAGNOSTIC_PUSH              __pragma(warning(push))
+  #define BOTAN_DIAGNOSTIC_IGNORE_DEPRECATED __pragma(warning(disable : 4996))
+  #define BOTAN_DIAGNOSTIC_POP               __pragma(warning(pop))
+#else
+  #define BOTAN_DIAGNOSTIC_PUSH
+  #define BOTAN_DIAGNOSTIC_IGNORE_DEPRECATED
+  #define BOTAN_DIAGNOSTIC_POP
 #endif
 
 #endif


Test patch:
diff --git a/src/tests/test_tls_rfc8448.cpp b/src/tests/test_tls_rfc8448.cpp
index 159f2f30d..6b806cdd8 100644
--- a/src/tests/test_tls_rfc8448.cpp
+++ b/src/tests/test_tls_rfc8448.cpp
@@ -131,7 +131,7 @@ std::chrono::system_clock::time_point from_milliseconds_since_epoch(uint64_t mse
           std::chrono::milliseconds(additional_millis);
    }
 
-using Modify_Exts_Fn = std::function<void(Botan::TLS::Extensions&, Botan::TLS::Connection_Side)>;
+using Modify_Exts_Fn = std::function<void(Botan::TLS::Extensions&, Botan::TLS::Connection_Side, Botan::TLS::Handshake_Type)>;
 using MockSignature_Fn = std::function<std::vector<uint8_t>(const std::vector<uint8_t>&,const std::string&,Signature_Format)>;
 
 /**
@@ -173,6 +173,13 @@ class Test_TLS_13_Callbacks : public Botan::TLS::Callbacks
          // handle a tls alert received from the tls server
          }
 
+      bool tls_peer_closed_connection() override
+         {
+         count_callback_invocation("tls_peer_closed_connection");
+         // we want to handle the closure ourselves
+         return false;
+         }
+
       bool tls_session_established(const Botan::TLS::Session& session) override
          {
          count_callback_invocation("tls_session_established");
@@ -280,16 +287,17 @@ class Test_TLS_13_Callbacks : public Botan::TLS::Callbacks
          return Callbacks::tls_server_choose_app_protocol(client_protos);
          }
 
-      void tls_modify_extensions(Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side) override
+      void tls_modify_extensions(Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side, Botan::TLS::Handshake_Type which_message) override
          {
-         count_callback_invocation("tls_modify_extensions");
-         m_modify_exts(exts, side);
+         count_callback_invocation(std::string("tls_modify_extensions_") + handshake_type_to_string(which_message));
+         m_modify_exts(exts, side, which_message);
+         Callbacks::tls_modify_extensions(exts, side, which_message);
          }
 
-      void tls_examine_extensions(const Botan::TLS::Extensions& extn, Connection_Side which_side) override
+      void tls_examine_extensions(const Botan::TLS::Extensions& extn, Connection_Side which_side, Botan::TLS::Handshake_Type which_message) override
          {
-         count_callback_invocation("tls_examine_extensions");
-         return Callbacks::tls_examine_extensions(extn, which_side);
+         count_callback_invocation(std::string("tls_examine_extensions_") + handshake_type_to_string(which_message));
+         return Callbacks::tls_examine_extensions(extn, which_side, which_message);
          }
 
       std::string tls_decode_group_param(Group_Params group_param) override
@@ -723,15 +731,18 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
          // 32 - for KeyShare (eph. x25519 key pair)
          add_entropy(*rng, vars.get_req_bin("RNG_Pool"));
 
-         auto add_extensions_and_sort = [](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side)
+         auto add_extensions_and_sort = [](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side, Botan::TLS::Handshake_Type which_message)
             {
-            // For some reason, presumably checking compatibility, the RFC 8448 Client
-            // Hello includes a (TLS 1.2) Session_Ticket extension. We don't normally add
-            // this obsoleted extension in a TLS 1.3 client.
-            exts.add(new Botan::TLS::Session_Ticket());
+            if(which_message == Handshake_Type::CLIENT_HELLO)
+               {
+               // For some reason, presumably checking compatibility, the RFC 8448 Client
+               // Hello includes a (TLS 1.2) Session_Ticket extension. We don't normally add
+               // this obsoleted extension in a TLS 1.3 client.
+               exts.add(new Botan::TLS::Session_Ticket());
 
-            add_renegotiation_extension(exts);
-            sort_extensions(exts, side);
+               add_renegotiation_extension(exts);
+               sort_extensions(exts, side);
+               }
             };
 
          std::unique_ptr<Client_Context> ctx;
@@ -742,7 +753,12 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx = std::make_unique<Client_Context>(std::move(rng), read_tls_policy("rfc8448_1rtt"), vars.get_req_u64("CurrentTimestamp"), add_extensions_and_sort);
 
                result.confirm("client not closed", !ctx->client.is_closed());
-               ctx->check_callback_invocations(result, "client hello prepared", { "tls_emit_data", "tls_inspect_handshake_msg_client_hello", "tls_modify_extensions" });
+               ctx->check_callback_invocations(result, "client hello prepared",
+                  {
+                  "tls_emit_data",
+                  "tls_inspect_handshake_msg_client_hello",
+                  "tls_modify_extensions_client_hello"
+                  });
 
                result.test_eq("TLS client hello", ctx->pull_send_buffer(), vars.get_req_bin("ClientHello_1"));
                }),
@@ -759,7 +775,11 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx->check_callback_invocations(result, "server hello partially received", { });
 
                ctx->client.received_data(server_hello_b);
-               ctx->check_callback_invocations(result, "server hello received", { "tls_inspect_handshake_msg_server_hello", "tls_examine_extensions" });
+               ctx->check_callback_invocations(result, "server hello received",
+                  {
+                  "tls_inspect_handshake_msg_server_hello",
+                  "tls_examine_extensions_server_hello"
+                  });
 
                result.confirm("client is not yet active", !ctx->client.is_active());
                }),
@@ -775,7 +795,8 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                   "tls_inspect_handshake_msg_certificate",
                   "tls_inspect_handshake_msg_certificate_verify",
                   "tls_inspect_handshake_msg_finished",
-                  "tls_examine_extensions",
+                  "tls_examine_extensions_encrypted_extensions",
+                  "tls_examine_extensions_certificate",
                   "tls_emit_data",
                   "tls_session_activated",
                   "tls_verify_cert_chain",
@@ -796,6 +817,7 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
 
                ctx->check_callback_invocations(result, "new session ticket received",
                   {
+                  "tls_examine_extensions_new_session_ticket",
                   "tls_session_ticket_received",
                   "tls_current_timestamp"
                   });
@@ -837,7 +859,7 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx->check_callback_invocations(result, "CLOSE_NOTIFY sent", { "tls_emit_data" });
 
                ctx->client.received_data(vars.get_req_bin("Server_CloseNotify"));
-               ctx->check_callback_invocations(result, "CLOSE_NOTIFY received", { "tls_alert" });
+               ctx->check_callback_invocations(result, "CLOSE_NOTIFY received", { "tls_alert", "tls_peer_closed_connection" });
 
                result.confirm("connection is closed", ctx->client.is_closed());
                }),
@@ -852,18 +874,21 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
          // 32 - for KeyShare (eph. x25519 key pair)
          add_entropy(*rng, vars.get_req_bin("RNG_Pool"));
 
-         auto add_extensions_and_sort = [](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side)
+         auto add_extensions_and_sort = [](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side, Botan::TLS::Handshake_Type which_message)
             {
-            exts.add(new Padding(87));
+            if(which_message == Handshake_Type::CLIENT_HELLO)
+               {
+               exts.add(new Padding(87));
 
-            add_renegotiation_extension(exts);
+               add_renegotiation_extension(exts);
 
-            // TODO: Implement early data support and remove this 'hack'.
-            //
-            // Currently, the production implementation will never add this
-            // extension even if the resumed session would allow early data.
-            add_early_data_indication(exts);
-            sort_extensions(exts, side);
+               // TODO: Implement early data support and remove this 'hack'.
+               //
+               // Currently, the production implementation will never add this
+               // extension even if the resumed session would allow early data.
+               add_early_data_indication(exts);
+               sort_extensions(exts, side);
+               }
             };
 
          std::unique_ptr<Client_Context> ctx;
@@ -878,7 +903,13 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                                                       vars.get_req_bin("SessionTicket"));
 
                result.confirm("client not closed", !ctx->client.is_closed());
-               ctx->check_callback_invocations(result, "client hello prepared", { "tls_emit_data", "tls_inspect_handshake_msg_client_hello", "tls_modify_extensions", "tls_current_timestamp" });
+               ctx->check_callback_invocations(result, "client hello prepared",
+                  {
+                  "tls_emit_data",
+                  "tls_inspect_handshake_msg_client_hello",
+                  "tls_modify_extensions_client_hello",
+                  "tls_current_timestamp"
+                  });
 
                result.test_eq("TLS client hello", ctx->pull_send_buffer(), vars.get_req_bin("ClientHello_1"));
                })
@@ -891,23 +922,26 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
 
       static std::vector<Test::Result> hello_retry_request(const VarMap& vars)
          {
-         auto add_extensions_and_sort = [flights = 0](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side) mutable
+         auto add_extensions_and_sort = [flights = 0](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side, Botan::TLS::Handshake_Type which_message) mutable
             {
-            ++flights;
-
-            if(flights == 1)
+            if(which_message == Handshake_Type::CLIENT_HELLO)
                {
-               add_renegotiation_extension(exts);
-               }
+               ++flights;
 
-            // For some reason RFC8448 decided to require this (fairly obscure) extension
-            // in the second flight of the Client_Hello.
-            if(flights == 2)
-               {
-               exts.add(new Padding(175));
-               }
+               if(flights == 1)
+                  {
+                  add_renegotiation_extension(exts);
+                  }
+
+               // For some reason RFC8448 decided to require this (fairly obscure) extension
+               // in the second flight of the Client_Hello.
+               if(flights == 2)
+                  {
+                  exts.add(new Padding(175));
+                  }
 
-            sort_extensions(exts, side);
+               sort_extensions(exts, side);
+               }
             };
 
          // Fallback RNG is required to for blinding in ECDH with P-256
@@ -927,7 +961,12 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx = std::make_unique<Client_Context>(std::move(rng), read_tls_policy("rfc8448_hrr"), vars.get_req_u64("CurrentTimestamp"), add_extensions_and_sort);
                result.confirm("client not closed", !ctx->client.is_closed());
 
-               ctx->check_callback_invocations(result, "client hello prepared", { "tls_emit_data", "tls_inspect_handshake_msg_client_hello", "tls_modify_extensions" });
+               ctx->check_callback_invocations(result, "client hello prepared",
+                  {
+                  "tls_emit_data",
+                  "tls_inspect_handshake_msg_client_hello",
+                  "tls_modify_extensions_client_hello",
+                  });
 
                result.test_eq("TLS client hello (1)", ctx->pull_send_buffer(), vars.get_req_bin("ClientHello_1"));
                }),
@@ -941,9 +980,9 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                   {
                   "tls_emit_data",
                   "tls_inspect_handshake_msg_hello_retry_request",
-                  "tls_examine_extensions",
+                  "tls_examine_extensions_hello_retry_request",
                   "tls_inspect_handshake_msg_client_hello",
-                  "tls_modify_extensions",
+                  "tls_modify_extensions_client_hello",
                   "tls_decode_group_param"
                   });
 
@@ -955,7 +994,12 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                result.require("ctx is available", ctx != nullptr);
                ctx->client.received_data(vars.get_req_bin("ServerHello"));
 
-               ctx->check_callback_invocations(result, "server hello received", { "tls_inspect_handshake_msg_server_hello", "tls_examine_extensions", "tls_decode_group_param" });
+               ctx->check_callback_invocations(result, "server hello received",
+                  {
+                  "tls_inspect_handshake_msg_server_hello",
+                  "tls_examine_extensions_server_hello",
+                  "tls_decode_group_param"
+                  });
                }),
 
             Botan_Tests::CHECK("Server HS Messages .. Client Finished", [&](Test::Result& result)
@@ -969,7 +1013,8 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                   "tls_inspect_handshake_msg_certificate",
                   "tls_inspect_handshake_msg_certificate_verify",
                   "tls_inspect_handshake_msg_finished",
-                  "tls_examine_extensions",
+                  "tls_examine_extensions_encrypted_extensions",
+                  "tls_examine_extensions_certificate",
                   "tls_emit_data",
                   "tls_session_activated",
                   "tls_verify_cert_chain",
@@ -987,7 +1032,7 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                result.test_eq("client close notify", ctx->pull_send_buffer(), vars.get_req_bin("Client_CloseNotify"));
 
                ctx->client.received_data(vars.get_req_bin("Server_CloseNotify"));
-               ctx->check_callback_invocations(result, "encrypted handshake messages received", { "tls_alert" });
+               ctx->check_callback_invocations(result, "encrypted handshake messages received", { "tls_alert", "tls_peer_closed_connection" });
 
                result.confirm("connection is closed", ctx->client.is_closed());
                }),
@@ -1002,10 +1047,13 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
          // 32 - for eph. x25519 key pair
          add_entropy(*rng, vars.get_req_bin("RNG_Pool"));
 
-         auto add_extensions_and_sort = [&](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side)
+         auto add_extensions_and_sort = [&](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side, Botan::TLS::Handshake_Type which_message)
             {
-            add_renegotiation_extension(exts);
-            sort_extensions(exts, side);
+            if(which_message == Handshake_Type::CLIENT_HELLO)
+               {
+               add_renegotiation_extension(exts);
+               sort_extensions(exts, side);
+               }
             };
 
          auto sign_certificate_verify = [&](const std::vector<uint8_t>& msg,
@@ -1042,7 +1090,7 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx->check_callback_invocations(result, "initial callbacks", {
                   "tls_emit_data",
                   "tls_inspect_handshake_msg_client_hello",
-                  "tls_modify_extensions",
+                  "tls_modify_extensions_client_hello",
                   });
 
                result.test_eq("Client Hello", ctx->pull_send_buffer(), vars.get_req_bin("ClientHello_1"));
@@ -1053,7 +1101,7 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx->client.received_data(vars.get_req_bin("ServerHello"));
 
                ctx->check_callback_invocations(result, "callbacks after server hello", {
-                  "tls_examine_extensions",
+                  "tls_examine_extensions_server_hello",
                   "tls_inspect_handshake_msg_server_hello",
                   });
                }),
@@ -1065,7 +1113,10 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx->check_callback_invocations(result, "signing callbacks invoked", {
                   "tls_sign_message",
                   "tls_emit_data",
-                  "tls_examine_extensions",
+                  "tls_examine_extensions_encrypted_extensions",
+                  "tls_examine_extensions_certificate",
+                  "tls_examine_extensions_certificate_request",
+                  "tls_modify_extensions_certificate",
                   "tls_inspect_handshake_msg_certificate",
                   "tls_inspect_handshake_msg_certificate_request",
                   "tls_inspect_handshake_msg_certificate_verify",
@@ -1095,6 +1146,7 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
 
                ctx->check_callback_invocations(result, "after receiving close notify", {
                   "tls_alert",
+                  "tls_peer_closed_connection"
                   });
                }),
          };
@@ -1109,10 +1161,13 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
          // 32 - eph. x25519 key pair
          add_entropy(*rng, vars.get_req_bin("RNG_Pool"));
 
-         auto add_extensions_and_sort = [&](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side)
+         auto add_extensions_and_sort = [&](Botan::TLS::Extensions& exts, Botan::TLS::Connection_Side side, Botan::TLS::Handshake_Type which_message)
             {
-            add_renegotiation_extension(exts);
-            sort_extensions(exts, side);
+            if(which_message == Handshake_Type::CLIENT_HELLO)
+               {
+               add_renegotiation_extension(exts);
+               sort_extensions(exts, side);
+               }
             };
 
          std::unique_ptr<Client_Context> ctx;
@@ -1123,6 +1178,13 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                ctx = std::make_unique<Client_Context>(std::move(rng), read_tls_policy("rfc8448_compat"), vars.get_req_u64("CurrentTimestamp"), add_extensions_and_sort);
 
                result.test_eq("Client Hello", ctx->pull_send_buffer(), vars.get_req_bin("ClientHello_1"));
+
+               ctx->check_callback_invocations(result, "client hello prepared",
+                  {
+                  "tls_emit_data",
+                  "tls_inspect_handshake_msg_client_hello",
+                  "tls_modify_extensions_client_hello",
+                  });
                }),
 
             Botan_Tests::CHECK("Server Hello + other handshake messages", [&](Test::Result& result)
@@ -1132,6 +1194,21 @@ class Test_TLS_RFC8448 final : public Text_Based_Test
                                                       // ServerHandshakeMessages contains the expected ChangeCipherSpec record
                                                       vars.get_req_bin("ServerHandshakeMessages")));
 
+               ctx->check_callback_invocations(result, "callbacks after server's first flight", {
+                  "tls_inspect_handshake_msg_server_hello",
+                  "tls_inspect_handshake_msg_encrypted_extensions",
+                  "tls_inspect_handshake_msg_certificate",
+                  "tls_inspect_handshake_msg_certificate_verify",
+                  "tls_inspect_handshake_msg_finished",
+                  "tls_examine_extensions_server_hello",
+                  "tls_examine_extensions_encrypted_extensions",
+                  "tls_examine_extensions_certificate",
+                  "tls_emit_data",
+                  "tls_session_activated",
+                  "tls_verify_cert_chain",
+                  "tls_verify_message"
+                  });
+
                result.test_eq("CCS + Client Finished", ctx->pull_send_buffer(),
                               // ClientFinished contains the expected ChangeCipherSpec record
                               vars.get_req_bin("ClientFinished"));
diff --git a/src/tests/unit_tls.cpp b/src/tests/unit_tls.cpp
index 87b61d47a..d5cf700f4 100644
--- a/src/tests/unit_tls.cpp
+++ b/src/tests/unit_tls.cpp
@@ -324,7 +324,7 @@ class TLS_Handshake_Test final
                // ignore
                }
 
-            void tls_modify_extensions(Botan::TLS::Extensions& extn, Botan::TLS::Connection_Side which_side) override
+            void tls_modify_extensions(Botan::TLS::Extensions& extn, Botan::TLS::Connection_Side which_side, Botan::TLS::Handshake_Type /* unused */) override
                {
                extn.add(new Test_Extension(which_side));
 
@@ -339,7 +339,7 @@ class TLS_Handshake_Test final
                   }
                }
 
-            void tls_examine_extensions(const Botan::TLS::Extensions& extn, Botan::TLS::Connection_Side which_side) override
+            void tls_examine_extensions(const Botan::TLS::Extensions& extn, Botan::TLS::Connection_Side which_side, Botan::TLS::Handshake_Type /*unused*/) override
                {
                Botan::TLS::Extension* test_extn = extn.get(static_cast<Botan::TLS::Handshake_Extension_Type>(666));