Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mixed PSK 2c: support RSA-PSK #5665

Closed
mpg opened this issue Mar 28, 2022 · 2 comments · Fixed by #5704
Closed

Mixed PSK 2c: support RSA-PSK #5665

mpg opened this issue Mar 28, 2022 · 2 comments · Fixed by #5704
Assignees
Labels
enhancement size-s Estimated task size: small (~2d)

Comments

@mpg
Copy link
Contributor

mpg commented Mar 28, 2022

This task is to add support for RSA-PSK using PSA-held (aka opaque) keys, that is, keys provisioned with mbedtls_ssl_conf_psk_opaque() or mbedtls_ssl_set_hs_psk_opaque() (used in the PSK callback).

The task is done when there are passing tests in ssl-opt.sh using RSA-PSK with psk_opaque=1 with sufficient coverage (client and/or server, static conf and/or callback, see existing tests for pure PSK).

Depends on: #5662 (also provides context and references).
Related: #5663, #5664

@mprse
Copy link
Contributor

mprse commented Apr 5, 2022

I understand that first the missing psa implementation for opaque rsa-psk needs to be added here:
https://github.com/Mbed-TLS/mbedtls/blob/development/library/ssl_tls12_client.c#L3040-L3054 ?

@mpg
Copy link
Contributor Author

mpg commented Apr 5, 2022

I think that's one place that will need changing, yes, but other than removing the check and error I don't think there's much to implement here - we're just generating a random secret and encrypting it with RSA, not accessing the PSK in any way. I think the actual use of the PSK happens later in the same function when calling mbedtls_ssl_psk_derive_premaster() except we skip this with opaque keys:

#if defined(MBEDTLS_USE_PSA_CRYPTO) && \
defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED)
if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK &&
ssl_conf_has_static_raw_psk( ssl->conf ) == 0 )
{
MBEDTLS_SSL_DEBUG_MSG( 1,
( "skip PMS generation for opaque PSK" ) );
}
else
#endif /* MBEDTLS_USE_PSA_CRYPTO &&
MBEDTLS_KEY_EXCHANGE_PSK_ENABLED */
if( ( ret = mbedtls_ssl_psk_derive_premaster( ssl,
ciphersuite_info->key_exchange ) ) != 0 )
{
MBEDTLS_SSL_DEBUG_RET( 1,
"mbedtls_ssl_psk_derive_premaster", ret );
return( ret );
}
(so again, this is a place where the existing check need adapting, but not where the actual computation occurs).

So, the PSK is actually used elsewhere... let me check. Must be when deriving the MS, since with opaque PSKs we go directly from PSK to MS (the TLS side of the library does not get to "see" the PMS). Ok, it's in ssl_compute_master() indeed:

mbedtls/library/ssl_tls.c

Lines 5136 to 5183 in ea75049

#if defined(MBEDTLS_USE_PSA_CRYPTO) && \
defined(MBEDTLS_KEY_EXCHANGE_PSK_ENABLED)
if( handshake->ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_PSK &&
ssl_use_opaque_psk( ssl ) == 1 )
{
/* Perform PSK-to-MS expansion in a single step. */
psa_status_t status;
psa_algorithm_t alg;
mbedtls_svc_key_id_t psk;
psa_key_derivation_operation_t derivation =
PSA_KEY_DERIVATION_OPERATION_INIT;
mbedtls_md_type_t hash_alg = handshake->ciphersuite_info->mac;
MBEDTLS_SSL_DEBUG_MSG( 2, ( "perform PSA-based PSK-to-MS expansion" ) );
psk = mbedtls_ssl_get_opaque_psk( ssl );
if( hash_alg == MBEDTLS_MD_SHA384 )
alg = PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_384);
else
alg = PSA_ALG_TLS12_PSK_TO_MS(PSA_ALG_SHA_256);
status = setup_psa_key_derivation( &derivation, psk, alg,
salt, salt_len,
(unsigned char const *) lbl,
(size_t) strlen( lbl ),
master_secret_len );
if( status != PSA_SUCCESS )
{
psa_key_derivation_abort( &derivation );
return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
}
status = psa_key_derivation_output_bytes( &derivation,
master,
master_secret_len );
if( status != PSA_SUCCESS )
{
psa_key_derivation_abort( &derivation );
return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
}
status = psa_key_derivation_abort( &derivation );
if( status != PSA_SUCCESS )
return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
}
else
#endif
- I think this is the place where we'll actually do the computation and use the extension that you've coded in the first PR. For mixed PSK, some previous function (I think it's always mbedtls_ssl_write_client_key_exchange() or one of its callees) will have left the other_secret part of the PMS in the handshake->pms buffer so you can just pass it to PSA from there.

Also, you'll probably need to adjust checks used in ciphersuite selection: hopefully in ClientHello/ServerHello parsing/writing we're currently refusing to use an RSA-PSK ciphersuite if we only have an opaque PSK; this will have to change. Well looking at the code I'm not seeing that anywhere, so perhaps there's nothing to change to ciphersuite selection, I guess we'll see when running tests.

And of course that was for the client, we need to identify the relevant places in the server too. Though the place where the actual computation is done should be common to both sides, as it's ssl_compute_master() in ssl_tls.c. So, hopefully the only other changes require on server will also be to adapt the conditions in some places as on the client, and ensure the other_secret part of the PMS lies in the handshake->pms buffer by the time we reach ssl_computer_master().

I don't remember exactly where everything happens, but the best way to find out is probably to write a test in ssl-opt.sh (or run the equivalent manually) that uses an opaque PSK on one side and forces an RSA-PSK ciphersuite and see where the first error message is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants