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

driver-only ECC: summary issue (Q2) #7241

Closed
mpg opened this issue Mar 9, 2023 · 3 comments
Closed

driver-only ECC: summary issue (Q2) #7241

mpg opened this issue Mar 9, 2023 · 3 comments
Labels
component-psa PSA keystore/dispatch layer (storage, drivers, …)

Comments

@mpg
Copy link
Contributor

mpg commented Mar 9, 2023

Requirement checklist

  • Timescale
    • no external deadline, let's try to define an intermediate goal for Q2
  • Description:
    • if I have a driver for some ECC, I don't want to have to compile in the built-in implementation
    • for Q2 (basic): for each one of ECDSA, ECDH, EC J-PAKE, if it's accelerated, then we can remove its built-in implementation (but ECP stays for now)
    • for Q2 (stretch-goal): in addition, it's OK for some curves to only be available via PSA the curve arithmetic par of ecp.c to be removed
  • Deliverables:
    • The library now supports builds as above, and everything work except as noted below
  • Exclusions / out-of-scope:
    • Support for compressed points
    • Interruptible
    • We can't remove ecp.c fully, or bignum.c at all, for now (to be lifted in Q3)
    • [edit] You need to enable MBEDTLS_USE_PSA_CRYPTO if you want everything to work in PK, X.509 and TLS.
  • Dependencies
    • None
  • Relevant contacts
    • Manuel, Valerio / Nordic

Epic checklist

  • Is epic fully broken down?
    • No, need to first split into two EPICs
    • Then task breakdown on-demand (or slightly ahead of demand)
  • Epic estimate
    • TBD
    • Roughly: MPG to create tasks; Valerio to implement; MPG + one other to review. MPG to prepare Q3 breakdown. So aprox. MPG + 1 engineer from Mbed TLS team.
  • Parallelism?
    • Not so much for the actual coding, but investigation for Q3 can be done in parallel
  • Potential for conflicts?
    • Nothing special.
  • Description/Use Case
    • See requirements.
  • DoD
    • See requirements.
    • Must have test parity (i.e., accelerated build with sw impl removed must exercise and pass roughly same set of tests as a non-accelerated builds)
  • Experts list
  • Design documentation
  • Dependencies
    • None
  • Resources needed
    • None
  • Infra/Test automation/CI needs
    • Nothing special, just adding new jobs to all.sh - some increase in CI load
  • Requirement link
  • Review use of a separator to split must-have from nice-to-have
    • Nice-to-have part is the second epic - some curves to only be available via PSA
  • Risks
    • Usual risk of unforeseen issues - no real mitigation - these may arise as the work progresses
    • Reliance on Valerio (external, Nordic)
  • Testing notes
    • Mostly about making existing tests pass in new configs, not so much new tests
  • Holiday
    • MPG not planning any large blocks of holiday in this timeframe
@mpg
Copy link
Contributor Author

mpg commented Mar 10, 2023

@daverodgman @gilles-peskine-arm @yanesca I've edited the "exclusions / out-of-scope" par of the requirements to record the fact that PK, X.509 and TLS will work fully only when MBEDTLS_USE_PSA_CRYPTO is defined - which was already documented in #6839 but we forgot to formally record it here.

I've also split the previous all-in-one ECC EPIC into three: two for Q2 as above: 2a top-level and 2b curves, and then one more for the rest: 2c low-level which we may end up splitting further into ECP and Bignum, which will be determined during the DI to happen in Q2.

I've dispatch issues in the new EPICs, including closed issues, and it was instructive: I noticed at lot of issues that belong to the Q3 EPIC have already been addressed. I think it's because even with one contributor, it was good to have some level of parallelism, which the "top-level" EPIC did not provide on its own, so I used easily-discovered low-handing fruits from the "low-level" EPIC as a filler. This doesn't fundamentally change what I said yesterday however: if we want to move beyond these and fully study the "low-level" EPIC, we need the "top-level" EPIC to be (mostly) completed first. That's because I expect a number of hidden hurdles will surface at that point.

@mpg
Copy link
Contributor Author

mpg commented Apr 3, 2023

Note: as discussed with @daverodgman and in the weekly team meeting, it turns out "some curves can be available only via PSA" was not a great intermediate goal for the 2nd EPIC, as we found out it actually depends on the hard parts of the next EPIC, which is quite contrary to its intended status as a stepping stone towards removing ECP_C.

Instead, a new, more realistic intermediate goal has been defined: remove the "curve arithmetic" part of ECP_C (that is, everything related to ecp_mul()). It is a good intermediate goal because:

  • it can be done independently, as shown by the prototype PR Investigate ECP light #7357.
  • it is quite evidently a step towards removing ECP_C entirely;
  • it already brings a significant code size improvement for users with ECC accelerators (around 5kB on M0).

I've update the goal in the "requirements checklist" above.

@davidhorstmann-arm davidhorstmann-arm added the component-psa PSA keystore/dispatch layer (storage, drivers, …) label May 18, 2023
@mpg
Copy link
Contributor Author

mpg commented Sep 29, 2023

Closing as the EPICs have been complete (last quarter).

@mpg mpg closed this as completed Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component-psa PSA keystore/dispatch layer (storage, drivers, …)
Projects
None yet
Development

No branches or pull requests

2 participants