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

"Implicit" ASN fields #66

Open
ssadler opened this issue Jan 17, 2017 · 0 comments
Open

"Implicit" ASN fields #66

ssadler opened this issue Jan 17, 2017 · 0 comments

Comments

@ssadler
Copy link

ssadler commented Jan 17, 2017

I did the best part of an implementation of Crypto-Conditions in Haskell (link) (I work for BigchainDB), and when I came to test my code against the provided test suite (very useful btw), there were a few things that were not as I expected, so I will outline them here as feedback:

  1. ASN

Throughout the spec, the ASN encoding is expressed in terms of ASN field types, however, the encoded binary does not contain the type headers, instead the field headers are "implicit" and manually numbered.

I am using the hs-asn1 libraries which have a data type to describe ASN1 encodable data, rather than using models, but the calls to implicit(0) are apparent in the Javascript code.

Below is an example of how that looks, encoding a payload that contains "hello", then a Sequence of an integer and then "world".

I expected:

[ OctetString "hello"
, Start Sequence
, IntVal 1
, OctetString "world"
, End Sequence
]

"\EOT\ENQhello0\n\STX\SOH\SOH\EOT\ENQworld"

But instead it's:

[ Other Context 0 "hello"
, Start (Container Context 1)
, Other Context 0 "\SOH"
, Other Context 1 "world"
, End (Container Context 1)
]

"\128\ENQhello\161\n\128\SOH\SOH\129\ENQworld"

So you can see that the field headers are different but the data is the same. Although the spec specifies SEQUENCE OF, usually I had to enter Start (Container Context n), where n is the index of the sequence in the parent data structure.

The exception to this is in the fingerprints, where the payload that gets hashed is in fact wrapped in a Sequence, at least according to my code. Looking at the Javascript code, this.seq() is always called, it could be that a call to implicit(n) is what ends up setting the header of the container, I don't know.

I'm not sure if the use of numbered "implicit" (I suppose that means the type is implicit) fields is better or worse than the other way, but, it wasn't obvious or explained that way in the spec, as far as I saw.

  1. Test Suite

Overall really good, very reassuring to have actual encoded values to test against 👍.

I made a pr with one small issue I found. I would also like to have seen a message or a prefix payload in some places where they were empty, just to be thorough. I didn't implement all the tests yet, and I see there have been some changes to the latest HEAD so I can't say for sure that's the only one, but still, the vast marjority of it worked, really good!

  1. The Spec

I didn't read it in its entirity from start to finish, so I may have missed details, but I did refer to it extensively.

At a high level, it feels like it's been re-written a bit, half old half new, I tend to see this effect when documentation has patches applied to it, it no longer feels natural, I could be off the mark though.

There are some sections like the following:

A PREFIX-SHA-256 fulfillment is valid iff:

* The size of M, in bytes, is less than or equal to F.maxMessageLength AND
* F.subfulfillment is valid, where the message used for validation of f is M prefixed by F.prefix AND
* D is equal to C

Which left me wondering what D and C were, and I was just now still unable to locate "D".

Hope this is useful!

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

No branches or pull requests

1 participant