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

Extend cipher suite list #21

Merged
merged 13 commits into from
May 18, 2021

Conversation

chrysn
Copy link
Collaborator

@chrysn chrysn commented Apr 26, 2021

This depends on TimothyClaeys/pycose#57, and builds on #19 (this actual PR is just a single commit, the rest is noise GitHub fails to assign to the other PR).

I think there's a bit of refactoring possibilities in here, see the "is this a good criterion" comment -- but that also interacts with pycose in that curves could gain a public_key_from_x constructor or something like that that'd spare us the if tree here. (We have a curve and some expression of a point, let's let the object do its magic).

Timothy Claeys and others added 12 commits April 21, 2021 18:05
+ Fix the broken example scripts
+ Use real X.509 certificates in the examples (authentication keys are Ed25519) --> remove pickled dictionary with legacy credential store
+ remote_cred_cb takes a Callable that returns the remote credentials
+ the return value of remote_cred_cb should go through _parse_credentials to parse/verify the RPK/certificate and extract the remote public authentication key
+ Add a custom CBOR encoder to properly encode COSE header maps with COSE header attributes
+ remove dependency on asn1crypto (everything can be done with the X509 module of the cryptography package)
This will allow the verifier to rebuild a MAC in the next step.
The remote callback used to be evaluated lazily whenever one of the
properties is first accessed; now it is evaluated as soon as all its
input data are set (ie. when the ID_CRED_remote is set). That laziness
currently *would* saved some calculation, but these savings go away as
things are verified completely.

The way this is done may later need to be changed again to accommodate
asynchronous operation, either by making some parts async, or by
altering how the library is used.

Not evaluating the callback result also led to some usage patterns being
untested.  Resulting from these tests, *all* callback outputs are fed
through _parse_credentials; this was previously done in only one code
path.
The lambda wrapper represents a change already done earlier in
"remote_cred_cb takes a Callable that returns the remote credentials"
(just never hit the test cases).

The cred_id[ri] setting is not so much done to have that available (it's
not even stored currently), but to ensure that this step (that usually
happens when parsing an incoming message) takes its side effects (of
evaluating the credentials callback and storing remote cred and authkey)
before these are used in the test evaluations.
Arguments may be had about whether this should be expanded even earlier,
but at least now it's consistently accessible.
@chrysn chrysn changed the title P256 Add support for EC2 keys (P256; others should be trivial to add) Apr 26, 2021
@chrysn chrysn changed the title Add support for EC2 keys (P256; others should be trivial to add) Extend cipher suite list Apr 29, 2021
@chrysn
Copy link
Collaborator Author

chrysn commented Apr 29, 2021

Suite 2 has been interoperated successfully today with Marco's implementation. Feeling bold and adding 4-5; these are untested so far (not even locally), but then again don't introduce anything conceptually new.

A function has been added that, when not running with -O, goes through the classes at import time and checks their algorithms against the numbers shown in the EDHOC spec.

@TimothyClaeys TimothyClaeys merged commit eba4f93 into openwsn-berkeley:master May 18, 2021
@TimothyClaeys
Copy link
Member

I've opened an issue in TimothyClaeys/pycose#62 to remind me to add your suggested functionality so we can refactor the code here and remove the big if-else tree.

@chrysn chrysn deleted the p256 branch May 18, 2021 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants