openwsn-berkeley / py-edhoc

Python implementation of EDHOC
BSD 3-Clause "New" or "Revised" License
5 stars 6 forks source link

Scripts outdated / _parse_credentials doesn't support callbacks any more #16

Open chrysn opened 3 years ago

chrysn commented 3 years ago

During 2625755282c7f5b75a443b98149bf6b07b0010ef, the scripts were not updated, making them unrunnable (they import OKP and that sort).

I only noticed because I was looking into _parse_credentials which now is a lot type stricter (appreciated), but in that course lost the ability to implicitly process callables. (That functionality was broken beforehand too, which didn't matter for the examples as they only ran in SIGN_SIGN without verification so the cred was never used).

Sure this could be fixed easily, but making _peer_cred work with callbacks required some ugly tricks anyway, so my suggestion is to rather not fix it that way but make EdhocRole's parse_cred be unconditionally callable in the first place (fewer distinct code paths, and for the rare cases where the peer is known in a hardcoded fashion a lambda id: foo will do).

Then the peer_cred passed in would be stored once, _peer_cred and _remote_authkey remain unavailable until a peer cred ID came in, and when that comes in the peer_cred function is once called, its output fed through _parse_credentials and it's all good again.

Can provide a PR if you agree with the roadmap.

chrysn commented 3 years ago

As for the scripts: The server side is just growing into a configurable module in aiocoap (with all the "concurrent requests", "telling messages apart" and "picking a suitable C_R"), and the client side is to follow, so if you want to abandon them there, I probably already got all I need from them.

TimothyClaeys commented 3 years ago

I agree with what you proposed. I've created a PR #17 that updates the EdhocRole classes to take a callable for the remote_cred_cb. The remote_cred_cb takes the cred_id object as a parameter and returns the remote credentials. It is up to the application to implement this callback. In simple cases, as you suggested, this can be a lambda (see the setup of the unit tests)

https://github.com/openwsn-berkeley/py-edhoc/blob/2bcca9a4eee671ba0bf5c9492fe526d74f3d3836/tests/conftest.py#L141

Output of remote_creb_cb is then passed to _parse_credentials.

The PR also fixes the scripts and now uses real (self-signed certificates).

Let me know what you think.

chrysn commented 3 years ago

PR #17 would close this. (Mentioned this already somewhere else I don't find yet: I didn't test all the changes in the scripts, but the cb changes work fine and the script changes helped me update the aiocoap side).