Merge pull request #4369 from hanno-arm/relax_psk_config

Implement relaxed semantics for static PSK configuration in Mbed TLS 3.0

ChangeLog.d/relaxed-psk-semantics.txt

@@ -0,0 +1,7 @@
+API changes
+    * Modify semantics of `mbedtls_ssl_conf_[opaque_]psk()`:
+      In Mbed TLS 2.X, the API prescribes that later calls overwrite
+      the effect of earlier calls. In Mbed TLS 3.0, calling
+      `mbedtls_ssl_conf_[opaque_]psk()` more than once will fail,
+      leaving the PSK that was configured first intact.
+      Support for more than one PSK may be added in 3.X.

docs/3.0-migration-guide.d/relaxed-psk-semantics.md

@@ -0,0 +1,18 @@
+Relaxed semantics for PSK configuration
+-----------------------------------------------------------------
+
+This affects users which call the PSK configuration APIs
+`mbedtlsl_ssl_conf_psk()` and `mbedtls_ssl_conf_psk_opaque()`
+multiple times on the same SSL configuration.
+
+In Mbed TLS 2.x, users would observe later calls overwriting
+the effect of earlier calls, with the prevailing PSK being
+the one that has been configured last. In Mbed TLS 3.0,
+calling `mbedtls_ssl_conf_[opaque_]psk()` multiple times
+will return an error, leaving the first PSK intact.
+
+To achieve equivalent functionality when migrating to Mbed TLS 3.0,
+users calling `mbedtls_ssl_conf_[opaque_]psk()` multiple times should
+remove all but the last call, so that only one call to _either_
+`mbedtls_ssl_conf_psk()` _or_ `mbedtls_ssl_conf_psk_opaque()`
+remains.

include/mbedtls/ssl.h

@@ -2712,8 +2712,14 @@ int mbedtls_ssl_conf_own_cert( mbedtls_ssl_config *conf,
 
 #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED)
 /**
- * \brief          Configure a pre-shared key (PSK) and identity
+ * \brief          Configure pre-shared keys (PSKs) and their
- *                 to be used in PSK-based ciphersuites.
+ *                 identities to be used in PSK-based ciphersuites.
+ *
+ *                 Only one PSK can be registered, through either
+ *                 mbedtls_ssl_conf_psk() or mbedtls_ssl_conf_psk_opaque().
+ *                 If you attempt to register more than one PSK, this function
+ *                 fails, though this may change in future versions, which
+ *                 may add support for multiple PSKs.
  *
  * \note           This is mainly useful for clients. Servers will usually
  *                 want to use \c mbedtls_ssl_conf_psk_cb() instead.
@@ -2721,13 +2727,6 @@ int mbedtls_ssl_conf_own_cert( mbedtls_ssl_config *conf,
  * \note           A PSK set by \c mbedtls_ssl_set_hs_psk() in the PSK callback
  *                 takes precedence over a PSK configured by this function.
  *
- * \warning        Currently, clients can only register a single pre-shared key.
- *                 Calling this function or mbedtls_ssl_conf_psk_opaque() more
- *                 than once will overwrite values configured in previous calls.
- *                 Support for setting multiple PSKs on clients and selecting
- *                 one based on the identity hint is not a planned feature,
- *                 but feedback is welcomed.
- *
  * \param conf     The SSL configuration to register the PSK with.
  * \param psk      The pointer to the pre-shared key to use.
  * \param psk_len  The length of the pre-shared key in bytes.
@@ -2740,7 +2739,9 @@ int mbedtls_ssl_conf_own_cert( mbedtls_ssl_config *conf,
  *                 of the SSL configuration.
  *
  * \return         \c 0 if successful.
- * \return         An \c MBEDTLS_ERR_SSL_XXX error code on failure.
+ * \return         #MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE if no more PSKs
+ *                 can be configured. In this case, the old PSK(s) remain intact.
+ * \return         Another negative error code on other kinds of failure.
  */
 int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf,
                 const unsigned char *psk, size_t psk_len,
@@ -2748,8 +2749,14 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf,
 
 #if defined(MBEDTLS_USE_PSA_CRYPTO)
 /**
- * \brief          Configure an opaque pre-shared key (PSK) and identity
+ * \brief          Configure one or more opaque pre-shared keys (PSKs) and
- *                 to be used in PSK-based ciphersuites.
+ *                 their identities to be used in PSK-based ciphersuites.
+ *
+ *                 Only one PSK can be registered, through either
+ *                 mbedtls_ssl_conf_psk() or mbedtls_ssl_conf_psk_opaque().
+ *                 If you attempt to register more than one PSK, this function
+ *                 fails, though this may change in future versions, which
+ *                 may add support for multiple PSKs.
  *
  * \note           This is mainly useful for clients. Servers will usually
  *                 want to use \c mbedtls_ssl_conf_psk_cb() instead.
@@ -2758,13 +2765,6 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf,
  *                 the PSK callback takes precedence over an opaque PSK
  *                 configured by this function.
  *
- * \warning        Currently, clients can only register a single pre-shared key.
- *                 Calling this function or mbedtls_ssl_conf_psk() more than
- *                 once will overwrite values configured in previous calls.
- *                 Support for setting multiple PSKs on clients and selecting
- *                 one based on the identity hint is not a planned feature,
- *                 but feedback is welcomed.
- *
  * \param conf     The SSL configuration to register the PSK with.
  * \param psk      The identifier of the key slot holding the PSK.
  *                 Until \p conf is destroyed or this function is successfully
@@ -2781,7 +2781,9 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf,
  *                 SSL configuration.
  *
  * \return         \c 0 if successful.
- * \return         An \c MBEDTLS_ERR_SSL_XXX error code on failure.
+ * \return         #MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE if no more PSKs
+ *                 can be configured. In this case, the old PSK(s) remain intact.
+ * \return         Another negative error code on other kinds of failure.
  */
 int mbedtls_ssl_conf_psk_opaque( mbedtls_ssl_config *conf,
                                  psa_key_id_t psk,

library/ssl_tls.c

@@ -3721,6 +3721,19 @@ int mbedtls_ssl_set_hs_ecjpake_password( mbedtls_ssl_context *ssl,
 
 #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED)
 
+static int ssl_conf_psk_is_configured( mbedtls_ssl_config const *conf )
+{
+#if defined(MBEDTLS_USE_PSA_CRYPTO)
+    if( !mbedtls_svc_key_id_is_null( conf->psk_opaque ) )
+        return( 1 );
+#endif /* MBEDTLS_USE_PSA_CRYPTO */
+
+    if( conf->psk != NULL )
+        return( 1 );
+
+    return( 0 );
+}
+
 static void ssl_conf_remove_psk( mbedtls_ssl_config *conf )
 {
     /* Remove reference to existing PSK, if any. */
@@ -3786,8 +3799,10 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf,
                 const unsigned char *psk_identity, size_t psk_identity_len )
 {
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
-    /* Remove opaque/raw PSK + PSK Identity */
+
-    ssl_conf_remove_psk( conf );
+    /* We currently only support one PSK, raw or opaque. */
+    if( ssl_conf_psk_is_configured( conf ) )
+        return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
 
     /* Check and set raw PSK */
     if( psk == NULL )
@@ -3855,8 +3870,10 @@ int mbedtls_ssl_conf_psk_opaque( mbedtls_ssl_config *conf,
                                  size_t psk_identity_len )
 {
     int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
-    /* Clear opaque/raw PSK + PSK Identity, if present. */
+
-    ssl_conf_remove_psk( conf );
+    /* We currently only support one PSK, raw or opaque. */
+    if( ssl_conf_psk_is_configured( conf ) )
+        return( MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
 
     /* Check and set opaque PSK */
     if( mbedtls_svc_key_id_is_null( psk ) )

tests/suites/test_suite_ssl.data

@@ -1,3 +1,15 @@
+Attempt to register multiple PSKs
+test_multiple_psks:
+
+Attempt to register multiple PSKS, incl. opaque PSK, #0
+test_multiple_psks_opaque:0
+
+Attempt to register multiple PSKs, incl. opaque PSK, #1
+test_multiple_psks_opaque:1
+
+Attempt to register multiple PSKs, incl. opaque PSK, #2
+test_multiple_psks_opaque:2
+
 Test calback buffer sanity
 test_callback_buffer_sanity:
 

tests/suites/test_suite_ssl.function

@@ -8,6 +8,8 @@
 #include <ssl_tls13_keys.h>
 #include "test/certs.h"
 
+#include <psa/crypto.h>
+
 #include <ssl_invasive.h>
 
 #include <test/constant_flow.h>
@@ -4535,3 +4537,109 @@ exit:
     mbedtls_free( src );
 }
 /* END_CASE */
+
+/* BEGIN_CASE depends_on:MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED */
+void test_multiple_psks()
+{
+    unsigned char psk0[10] = { 0 };
+    unsigned char psk0_identity[] = { 'f', 'o', 'o' };
+
+    unsigned char psk1[10] = { 0 };
+    unsigned char psk1_identity[] = { 'b', 'a', 'r' };
+
+    mbedtls_ssl_config conf;
+
+    mbedtls_ssl_config_init( &conf );
+
+    TEST_ASSERT( mbedtls_ssl_conf_psk( &conf,
+                     psk0, sizeof( psk0 ),
+                     psk0_identity, sizeof( psk0_identity ) ) == 0 );
+    TEST_ASSERT( mbedtls_ssl_conf_psk( &conf,
+                     psk1, sizeof( psk1 ),
+                     psk1_identity, sizeof( psk1_identity ) ) ==
+                 MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
+
+exit:
+
+    mbedtls_ssl_config_free( &conf );
+}
+/* END_CASE */
+
+/* BEGIN_CASE depends_on:MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED:MBEDTLS_USE_PSA_CRYPTO */
+void test_multiple_psks_opaque( int mode )
+{
+    /*
+     * Mode 0: Raw PSK, then opaque PSK
+     * Mode 1: Opaque PSK, then raw PSK
+     * Mode 2: 2x opaque PSK
+     */
+
+    unsigned char psk0_raw[10] = { 0 };
+    unsigned char psk0_raw_identity[] = { 'f', 'o', 'o' };
+
+    psa_key_id_t psk0_opaque = (psa_key_id_t) 1;
+    unsigned char psk0_opaque_identity[] = { 'f', 'o', 'o' };
+
+    unsigned char psk1_raw[10] = { 0 };
+    unsigned char psk1_raw_identity[] = { 'b', 'a', 'r' };
+
+    psa_key_id_t psk1_opaque = (psa_key_id_t) 2;
+    unsigned char psk1_opaque_identity[] = { 'b', 'a', 'r' };
+
+    mbedtls_ssl_config conf;
+
+    USE_PSA_INIT( );
+    mbedtls_ssl_config_init( &conf );
+
+    switch( mode )
+    {
+        case 0:
+
+            TEST_ASSERT( mbedtls_ssl_conf_psk( &conf,
+                         psk0_raw, sizeof( psk0_raw ),
+                         psk0_raw_identity, sizeof( psk0_raw_identity ) )
+                   == 0 );
+            TEST_ASSERT( mbedtls_ssl_conf_psk_opaque( &conf,
+                         psk1_opaque,
+                         psk1_opaque_identity, sizeof( psk1_opaque_identity ) )
+                   == MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
+            break;
+
+        case 1:
+
+            TEST_ASSERT( mbedtls_ssl_conf_psk_opaque( &conf,
+                         psk0_opaque,
+                         psk0_opaque_identity, sizeof( psk0_opaque_identity ) )
+                   == 0 );
+            TEST_ASSERT( mbedtls_ssl_conf_psk( &conf,
+                         psk1_raw, sizeof( psk1_raw ),
+                         psk1_raw_identity, sizeof( psk1_raw_identity ) )
+                   == MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
+
+            break;
+
+        case 2:
+
+            TEST_ASSERT( mbedtls_ssl_conf_psk_opaque( &conf,
+                         psk0_opaque,
+                         psk0_opaque_identity, sizeof( psk0_opaque_identity ) )
+                   == 0 );
+            TEST_ASSERT( mbedtls_ssl_conf_psk_opaque( &conf,
+                         psk1_opaque,
+                         psk1_opaque_identity, sizeof( psk1_opaque_identity ) )
+                   == MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE );
+
+            break;
+
+        default:
+            TEST_ASSERT( 0 );
+            break;
+    }
+
+exit:
+
+    mbedtls_ssl_config_free( &conf );
+    USE_PSA_DONE( );
+
+}
+/* END_CASE */