Use PSA EC-JPAKE in TLS (1.2) - Part 2

[valeriosetti]

Description

Following the fixes of PR #6390 by @mpg and the issues found in PR #5886, since @superna9999 could not address those issues, this PR aims at continuing the work of PR #5886.

As a consequence it resolves #5847

The workflow was as follows:

• checked out at PR Fix ecjpake PSA format #6390
• rebased PR Use PSA EC-JPAKE in TLS (1.2) #5886 on top of PR Fix ecjpake PSA format #6390
• applied fixes
  • basically the fix was to align the output/parsing of the ECJPAKE implementation that uses PSA crypto to the one that doesn't (which itself agrees to the format defined in draft-cragie-tls-ecjpake-01

As a consequence this PR depends on PR #6390 to be merged.

Gatekeeper checklist

• changelog not required well after more careful thinking, it was required - provided by Add a changelog entry explaining usage of PSA in TLS 1.2 EC J-PAKE #6674
• backport not required - new feature
• tests provided

[valeriosetti]

It has been tested following the procedure suggested by @mpg in PR #5886

git checkout -- include/mbedtls/mbedtls_config.h
scripts/config.py set MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED
make clean

make -C programs ssl/ssl_server2 ssl/ssl_client2
cp programs/ssl/ssl_server2 s2_no_use_psa
cp programs/ssl/ssl_client2 c2_no_use_psa


scripts/config.py set MBEDTLS_USE_PSA_CRYPTO
make clean

make -C programs ssl/ssl_server2 ssl/ssl_client2
make -C programs test/udp_proxy test/query_compile_time_config

P_SRV=../s2_no_use_psa tests/ssl-opt.sh -f ECJPAKE
P_CLI=../c2_no_use_psa tests/ssl-opt.sh -f ECJPAKE

and now it seems to work properly:

ECJPAKE: client not configured ......................................... PASS
ECJPAKE: server not configured ......................................... PASS
ECJPAKE: working, TLS .................................................. PASS
ECJPAKE: password mismatch, TLS ........................................ PASS
ECJPAKE: working, DTLS ................................................. PASS
ECJPAKE: working, DTLS, no cookie ...................................... PASS
ECJPAKE: password mismatch, DTLS ....................................... PASS
ECJPAKE: working, DTLS, nolog .......................................... PASS
------------------------------------------------------------------------
PASSED (8 / 8 tests (0 skipped))
ECJPAKE: client not configured ......................................... PASS
ECJPAKE: server not configured ......................................... PASS
ECJPAKE: working, TLS .................................................. PASS
ECJPAKE: password mismatch, TLS ........................................ PASS
ECJPAKE: working, DTLS ................................................. PASS
ECJPAKE: working, DTLS, no cookie ...................................... PASS
ECJPAKE: password mismatch, DTLS ....................................... PASS
ECJPAKE: working, DTLS, nolog .......................................... PASS
------------------------------------------------------------------------
PASSED (8 / 8 tests (0 skipped))

[AndrzejKurek]

It seems that for now, unfortunately, some CI tests fail on EC-JPAKE. Let me know @valeriosetti once the CI passes and please also add a needs:review label then!

[valeriosetti]

It seems that for now, unfortunately, some CI tests fail on EC-JPAKE. Let me know @valeriosetti once the CI passes and please also add a needs:review label then!

@AndrzejKurek yes, there's a dependency between this PR and #6390: I expect tests to work properly once #6390 is merged into development branch and this PR is rebased on top of it.

PS = as for the label can I add it? Or do I need some extra privilege?

[AndrzejKurek]

PS = as for the label can I add it? Or do I need some extra privilege?

Can you check if you have an option to add it? There should be a small "cog" next to the labels.

[valeriosetti]

Can you check if you have an option to add it? There should be a small "cog" next to the labels.

Nope, I don't have it :/
Indeed I would also like to set myself as the assignee for #5847, but I don't think I have the permission to do that

[mpg]

It has been tested following the procedure suggested by @mpg in PR #5886
and now it seems to work properly:

That's great news! Can you also add an all.sh component that does this testing?

Regarding labels / assigning, I'll try to sort out the permissions, in the meantime can you just post a comment when you think this is ready for review?

[gilles-peskine-arm]

On the internal CI, test_depends_py_curves_psa timed out but Windows has a genuine failure from MSVC:

   213>C:\builds\workspace\mbed-tls-pr-merge_PR-6533-merge\worktrees\tmpqmhr7jmt\library\ssl_tls.c(8519): error C2220: warning treated as error - no 'object' file generated [C:\builds\workspace\mbed-tls-pr-merge_PR-6533-merge\worktrees\tmpqmhr7jmt\cmake_solution\library\mbedtls.vcxproj]
   213>C:\builds\workspace\mbed-tls-pr-merge_PR-6533-merge\worktrees\tmpqmhr7jmt\library\ssl_tls.c(8519): warning C4267: '=' : conversion from 'size_t' to 'unsigned char', possible loss of data 

That's line 8519 in the merge with 339406d.

[valeriosetti]

That's line 8519 in the merge with 339406d

@gilles-peskine-arm is this the right commit? Because to me it seems that this one is related to mpi :/

[gilles-peskine-arm]

@valeriosetti The CI runs on a merge of the branch of the pull request with the current state of the target branch. So the line number is for a merge of your branch at d4a9b1a with development at 339406d.

[mpg]

LGTM

[AndrzejKurek]

Thanks for addressing all the comments!