search

openwsn-berkeley / lakers

EDHOC implemented in Rust, optimized for microcontrollers, with bindings for C and Python.
https://crates.io/crates/lakers
BSD 3-Clause "New" or "Revised" License
15 stars 10 forks source link

Python regression around crypto-method-agility #303

Closed chrysn closed 3 days ago

chrysn commented 3 months ago

The lakers-python package suffered some regressions (or unnoticed API changes) between 8e926b0fe331575f86087219eabac180460bc62b (good) and aa6eca5ababcc7ac64d67874c08f905e63953b29 (bad). Commits inbetween could not be tested. The changes were part of #284 AFAICT.

To reproduce, check out aiocoap (eg. 0.4.11), and run:

$ tox -e py312-allextras exec --skip-pkg-install -- pip uninstall -y lakers-python
    # (just in case you had it installed before)
$ tox -e py312-allextras exec --skip-pkg-install -- pip install 'git+https://github.com/openwsn-berkeley/lakers/@4e4aac33c00538d5904b11716e94a417cd514ed5#egg=lakers-python&subdirectory=lakers-python'
$ tox -e py312-allextras -- -k 'TestServerEdhocKidKid and test_whoami_is_client'

The name of the failing test indicated that things go bad when both parties present their keys by Key ID.

I'll continue from there to dig further into what is going wrong.

geonnave commented 3 months ago

That's weird, I just tested the kid<->kid version and the unit tests pass (see #304).

chrysn commented 3 weeks ago

I've dug down into this and found that when sending credentials by value, the 4th argument to compute_mac_2 (id_cred_r) used to be just the KID (cred_r.get_id_cred()), while now it is the full sent value (id_cred_r.as_full_value()). As that argument corresponds to the ID_CRED_R from context_2, the new version is most likely correct, but represents a breaking change in lakers' behavior when sending keys by value.

I think that #284 was therefore not just refactoring but fixed an actual bug.

I'll give all this some further testing, but I think that the right thing to resolve this here is just to say that yes 0.3.3 was buggy w/rt KID-by-value, call it a day, and I'll update aiocoap to use the latest version instead.

geonnave commented 3 weeks ago

Sounds good, thanks!

chrysn commented 3 days ago

The regression was actually a bugfix; closing this.