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

Clean up CDDL, work on this in a separate file for now #27

Closed
wants to merge 3 commits into from

Conversation

cabo
Copy link
Contributor

@cabo cabo commented Apr 9, 2020

Henk and I created a cleaned up version of the CBOR encoding and accompanying CDDL.
We should check whether this encoding helps us and then integrate it into the document.

@@ -36,33 +36,35 @@ TEEP-TYPE-teep-error = 5
TEEP-TYPE-teep-success = 6

version = uint .size 4
ext-info = uint
ext-info = uint

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Martin's I-D template toolchain might complain about trailing spaces at some point. But there is make fix-lint

$data-item-requested /= suit-commands

query-request = [
type: TEEP-TYPE-query-request,
token: uint,
options: {
? cipher-suites => [ + suite ],
? supported-cipher-suites => suite,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If suite becomes a bitmap, I'd rename it to suite-selection.

? nonce => bytes,
? version => [ + version ],
? oscp-data => bytes,
* $$query-request-extensions
* $$teep-option-extensions
},
* data-item-requested, ; why is this not a bitmap?
data-item-requested

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the intended semantics here? "Requesting one thing at a time"?

]

suite = $TEEP-suite .within uint
; ciphersuites as bitmaps
suite = $TEEP-suite .within uint .size 8

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe suite-selection. Also, I am not sure, if a bitmap is the best way to do this. How many options do we expect in the foreseeable future? And how many selected suites can be expected, typically?

In general, this should work. Again, just checking.

token: uint,
options: {
? supported-cipher-suites => suite,
? nonce => bytes,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the same size as EAT.
? nonce => bytes .size (8..64),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Thanks. I knew that, but forgot it :-)


teep-message-framework = [
type: 0..23 / $teep-type-extension,
token: uint,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer token: uint .size 4, to have constant size rather than variable size to make it easier to parse.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any CBOR parser should take care of that, I think. There might be other benefits provided by fixed length to specific post-processing, but parsing the CBOR data item will not become more difficult with variable length, in practice

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the ".size" declaration in the CDDL does not have an influence on the representation size that will be chosen by the encoder. It just limits the range of the number. The encoder most likely will choose "preferred serialization" https://cbor-wg.github.io/CBORbis/draft-ietf-cbor-7049bis.html#rfc.section.4.1, i.e. shortest form.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and if you really want to limit the token to 0..4294967295, you do need the .size 4 or need to use the range 0..4294967295 — is that the intention?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cabo Yes, my intention was to limit the token to 0..4294967295, so the binary size for the token will not change other than 32bits/4 bytes.
As @henkbirkholz has mentioned having variable size should not be a problem if everybody use the reasonable CBOR parser.
The one of cbor package of python looks really good.
https://bitbucket.org/bodhisnarkva/cbor/src/default/py/cbor/cbor.py

I have seen so many implementation something similar that just reading the first 4 bytes of the member even the size was 8bytes, ignoring the half of the value, ignoring the network byte order, and so on. The issue is that those implementation will work for most of the time, and some few years later, suddenly stops working when the TAM starts to handle more Devices, and then they have untrusted feeling for using the protocol.
In other protocol I have seen ignoring all the valuable information in the header of the packets and skipping of reading the header by just read 20 bytes offset to get the content.

I may be worrying too much, but since the type and token are the mandatory members of array of the teep protocol, I felt limiting the both members to be constant binary size would make things easier in the world.

However, this is just my preference. I am fine with both ways.

This was referenced Apr 13, 2020
@hannestschofenig
Copy link
Collaborator

Incorporated the CDDL into the main document and therefore closed this PR.

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.

None yet

4 participants