EC2: Allow creation with x only (for compact applications)

[chrysn]

Something like this is essential for getting cipher suites 2-3 running in pyedhoc.

The FIXMEs in the code primarily represent questions I have about the style of pycose:

• Can we have the curve classes as used in EC just as properties of the curve classes as used in cose? (I'm duplicating a bit from the generate_key switch, but I doubt it should be there in the first place).
• The Union[Type['CoseCurve'] type input is a bit hard to use (as seen by the if crv in part), and it doesn't seem like it is resolved even on the path to the final object. Is there any harm in giving CoseCurve some copy-constructor like behavior that'd allow us to just pass that union type through crv = CoseCurve(crv) once and be done with it?

(This has been tested as a part of a py-cose branch, but not interop'd; only the fact that if I swap around any key endianness the later curve point import to openssl fails makes me relatively confident that the reconstruction does work out).

[TimothyClaeys]

Yes, this is something we definitely need to include in the pycose library. I'll prepare a PR (including this commit) that updates the test vectors so they pass and answers some of your FIXME comments in the code.

[chrysn]

To elaborate on the second point: The EC2Key init is accepting the curve as Union[Type['CoseCurve'], str, int].

Consequently, when I later look up the curve, I have to do if crv in (P256, P256.identifier):.

There may be merit to accepting all these types, but I don't think there is merit to allowing self.store[EC2KpCurve] to be any of those -- that should really just be EC2KpCurve. In the crv.setter we already do a conversion, so maybe it's a bug anyway that the init function lets any crv thing pass directly into the dict.

The right thing to do IMO would be to pass crv through CoseCurve.from_id whenever we get our hands on it in EC2 and similar classes (eg. right in the first lines of the constructor), possibly eased by a case in from_id that says if isinstance(attribute, cls): return cls to not need that check everywhere else around. That makes from_id a very practical constructor. (I've originally described that wrongly as copy-constructor-like, but didn't find a good term for no-op-if-already-constructed constructors either).

But I can open a PR for something along these lines to actually change this once this is merged (so we don't open up conflicts).

[TimothyClaeys]

Hi @chrysn, your explanation makes a lot of sense. You are right, it is a bug and I definitely intended to store the crv attribute in the internal dictionary store as a EC2KpCurve. The current behavior is a bug.

[TimothyClaeys]

On second thoughts, I think this bug was already fixed in master but it was not released on PyPi. Thus py-edhoc is using an outdated pycose.

In the EC2Key constructor, I'm using the crv property of EC2Key to check the provided crv parameter and I do the conversion (in case of an integer or string) there.

pycose/cose/keys/ec2.py

Line 88 in a4da657

self.crv = crv

pycose/cose/keys/ec2.py

Line 116 in a4da657

self.store[EC2KpCurve] = CoseCurve.from_id(crv)

[sanghvibk]

On second thoughts, I think this bug was already fixed in master but it was not released on PyPi. Thus py-edhoc is using an outdated pycose.

@TimothyClaeys , It seems that code in Git is not in sync with PyPi for the same version number 0.9.dev5. May be version number needs to be incremented in Git and latest package needs to be pushed to PyPi. Thank you for developing such a nice library.

[TimothyClaeys]

Thanks @sanghvibk, I bumped the version and uploaded everything to PyPi.

[TimothyClaeys]

@chrysn I cherry-picked this commit and put it on top of the ongoing work on the develop branch. I will close this PR since it was included in the latest develop branch which I just merged into master. Code was also packaged and uploaded to PyPi.