Thanks for the review Jim,

See inline comments

On Sat, Apr 1, 2017 at 5:03 AM, Jim Schaad <[email protected]> wrote:

> Given that it was stated that the authors believe that the document was
> ready for publication, I decided to do another review pass.
>
> 1.  Following the discussion in the SET WG meeting, I believe that it would
> be reasonable to define some inputs for the external data fields to allow
> for distinguishing between the different uses of JWT structures.  Language
> about different applications extending this structure would also be
> reasonable.
>

I was not part of that discussion, could you please link to some resource
or notes from that meeting.


>

2.  I do not know if the authors looked at changing the Type3StringOrURI so
> that it would explicitly tag URIs or not.  I do no remember seeing any
> discussions on the list but have not gone back to search
>

We have no looked at changing this. Is there any good motivation for
actually doing this change?


>
> 3.  I find the description of Type6NumericDate to be slightly confusing as
> it appears to imply that this is not using a numeric value when it does.
>

I think the idea is to say that it is not a JSON number but a CBOR number.
I have added a ticket to look at the wording.
https://github.com/erwah/ietf/issues/28



>
> 4.  The authors need to look at their use of Type6NumericDate and determine
> if this is what they really want to do.  All of the examples are incorrect
> because of this tag usage.
>

Examples should be updated, see below


>
> 5.  After the discussions in the SET group, do the authors which to
> re-consider the MUST ignore statement in the first paragraph of section 3?
>

I have not seen the SET group discussion could you please link to it.


>
> 6.  The string "6 tag value 1" is normally written as "6.1" when looking at
> pretty-printed CBOR diagnostics.   This would be clearer than what is
> written.
>

Good input, I have create an issue to update this,
https://github.com/erwah/ietf/issues/26


>
> 7.  The text should be altered to use a TBD for the CWT tag rather than
> using a constant so this is highlighted.
>

Good input, I have create an issue to update this,
https://github.com/erwah/ietf/issues/25


>
> 8.  The note for step 5 in section 6.1 is problematic from a number of
> things.  A) AEAD algorithms are required, so it is not clear that the
> recommendation makes sense.  B) there is a big difference between signing
> and MACing in terms of the amount and type of integrity provided.
> Replacing
> signing w/ AEAD loses a lot.
>

I think you are correct and I have considered removing it, I added in in an
early attempt to be smart.
I have added a issue to evaluate the value of this statement and remove if
considered useless.
https://github.com/erwah/ietf/issues/24



>
> 9.  Step 6 in section 6.1 does not agree w/ the language in section 5.
> MUST
> vs maybe.
>

I see your point. I have added a ticket to look over the create and verify
steps to make sure they are consistent.
https://github.com/erwah/ietf/issues/27


>
> 10.  In starting to verify the examples I ran across the following two
> issues:
>
> a) The hex string and the diagnostic notation are equivalent, but they are
> not the same.  Specifically, the order of claims is not the same.  CBOR.ME
> gives
>
> {2: "erikw", 3: "coap://light.example.com", 4: 1444064944, 5: 1443944944,
> 6:
> 1443944944, 1: "coap://as.example.com", 7: h'0b71'}
>

I have create a issue to make them the same to make reading and testing
easier, https://github.com/erwah/ietf/issues/23


>
> b) The encoding of some of the claims is incorrect according to the
> document.  It should be
>

You are correct, I have added an issue to update,
https://github.com/erwah/ietf/issues/22


>
> { 1: "coap://as.example.com", 2: "erikw", 3: "coap://light.example.com",
> 4:
> 1(1444064944), 5: 1(1443944944), 6: 1(1443944944),7: h'0b71'}
>
> Or
>
> a7                                      # map(7)
>    01                                   # unsigned(1)
>    75                                   # text(21)
>       636f61703a2f2f61732e6578616d706c652e636f6d # "coap://as.example.com"
>    02                                   # unsigned(2)
>    65                                   # text(5)
>       6572696b77                        # "erikw"
>    03                                   # unsigned(3)
>    78 18                                # text(24)
>       636f61703a2f2f6c696768742e6578616d706c652e636f6d #
> "coap://light.example.com"
>    04                                   # unsigned(4)
>    c1                                   # tag(1)
>       1a 5612aeb0                       # unsigned(1444064944)
>    05                                   # unsigned(5)
>    c1                                   # tag(1)
>       1a 5610d9f0                       # unsigned(1443944944)
>    06                                   # unsigned(6)
>    c1                                   # tag(1)
>       1a 5610d9f0                       # unsigned(1443944944)
>    07                                   # unsigned(7)
>    42                                   # bytes(2)
>       0b71                              # "\vq"
>
> Note the additional tagging which is required.
>
>
> _______________________________________________
> Ace mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/ace
>
_______________________________________________
Ace mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/ace

Reply via email to