search

openwsn-berkeley / py-edhoc

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

Develop: merge in update to draft -05 + new test vectors #14

Closed TimothyClaeys closed 3 years ago

TimothyClaeys commented 3 years ago

@chrysn Will merge once CI completes successfully.

chrysn commented 3 years ago

Just looking through the changes: Does it make sense to have an undefined CORR that does not err out but just behaves like one of the existing ones in encoding? The encode functions could just as well have no default value to encode -- it's not like there's any API promises there yet, and taking an argument that seems required and silently behaves in one of two ways (CORR_UNKNOWN is neither == CORR_0 nor == CORR_2, after all) sounds dangerous to me.

TimothyClaeys commented 3 years ago

I agree that it's kind of weird to have it. I wanted to properly override the encode method from the base message class https://github.com/openwsn-berkeley/py-edhoc/blob/bbe062f0d22d4acbcb9a586d97ae10b3f5cc3821/edhoc/messages/base.py#L28 in message1, but it makes no sense to have a CORR_UNKNOWN.

chrysn commented 3 years ago

Ah, I see now -- because it doesn't matter there so could be left out. I suggest to still require the caller to pass it in: since https://github.com/lake-wg/edhoc/commit/e3f542d148aee0196c2ca73fe5c530f1292b6fac there is a difference even in message 1.

TimothyClaeys commented 3 years ago

Ok, sounds good, I'll prepare a fix.