-
Notifications
You must be signed in to change notification settings - Fork 0
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
Initial stab at the schema #3
Conversation
Partially addresses #2
Success and failure seem odd terms to me, because failing to validate an invalid document is a success. Unless you're saying that tests need to know what the result is, rather than returning a yes this seems ok or no this doesn't pass our library checks and mapping to success or failure. If it's this then it's easy to just return success all the time 😀 |
Also does skipped imply "library does not implement" or is that separate (probably based on operation)? |
If I look at the spec, https://cose-wg.github.io/cose-spec/ and then at the sign1-0000.json file I can't easily map the test case terms to the spec. From a .NET perspective our validation process is two steps, CoseMessage.DecodeSign1() and then Verify() passing content in if it's detached. It's hard for me to look at the json and see how I would construct the encoded message would be to pass into Decode from the myriad of parameters. Does it all need to be that detailed, rather than "Here, message document, go", or "Here, key, separate detached content, go" |
also add a spell check action in the Makefile
README.md
Outdated
|
||
It is expected that the output of the sign API and the value contained in the | ||
`cborHex` field of the `expectedOutput` are compared up the the 3rd entry of the | ||
COSE_Sign1 array, i.e., excluding the 4th (signature) field. If the two values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth providing something like a signatureLength
hint in the test description so that a non-deterministic test driver can just do their equivalent of return output[0..^test.signatureLength].SequenceEquals(test.expectedOutput[0..^test.SignatureLength]);
? Or, if the concern is that one day COSE might support a signature scheme with a dynamic length, the opposite (fixedOutputLength
?)? The fixedOutputLength would still be valid for a dynamic-length signature since CBOR arrays identify the number of elements in the array, not the number of bytes.
(using something like fixedOutputLength actually seems generally nicer, despite signatureLength being the easier concept).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Done in f6bd705
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am okay with the CDDL. We could do it more sophisticated. E.g.:
add optional payload for the detached signature case.
This would be present ONLY when the payload is not in the taggedCOSESign1
That mutual exclusiveness is now in English text, we could add a generic doing that. That might be over-engineering. For now this is fine, it would not change the message/document layout, just the data definition for it.
How much pain would it be to change structure itself later on? What is the expectation on stability?
I am not sure how that would be done in CDDL because one needs to inspect the value of the taggedCOSESign1 key in a non-trivial way.
It depends on the kind of change. I guess adding (co-)constraints that are currently expressed only in natural language (but not in CDDL) would not have any impact over the stability. On the contrary it'd make the whole thing better. Adding new optional fields should be OK as well. Adding new mandatory fields or enforcing (co-)constraints that were not anticipated / implicit is going to be impacting.
I have high hopes here :-) |
Barry do you have any pending comment that needs addressing or can we merge this? |
I'm happy enough with the wording :) |
Partially addresses #2