Now getting to Eric's COMMENTs:

On 25/05/2017 14:32, Eric Rescorla wrote:
...
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> 
> S 3.5.4.3.
>    After a GRASP device successfully discovers a locator for a
> Discovery
>    Responder supporting a specific objective, it MUST cache this
>    information, including the interface index via which it was
>    discovered.  This cache record MAY be used for future negotiation or
>    synchronization, and the locator SHOULD be passed on when
> appropriate
>    as a Divert option to another Discovery Initiator.
> 
> What's an "interface index"

It's a term of art in the socket API:
https://tools.ietf.org/html/rfc3493#section-4
Does it need a reference?

> S 3.5.4.4.
>    Since the relay device is unaware of the timeout set by the original
>    initiator it SHOULD set a timeout at least equal to
> GRASP_DEF_TIMEOUT
>    milliseconds.
> 
> I'm not sure I'm following here. Does the relay instance retransmit
> with its own timeout?

Yes, but this text was broken anyway (as detected by Bill Atwood testing
my code, embarassingly recently). The next version should be clearer,
as well as being corrected.

>    It MUST cache the Session ID
>    value and initiator address of each relayed Discovery message until
>    any Discovery Responses have arrived or the discovery process has
>    timed out.
> 
> How does this behave if the original initiator's timeout is
> longer than GRASP_DEF_TIMEOUT?

That case works, except that the original initiator waits unnecessarily.
But consider what happens if the original initiator's timeout is *shorter*
than the timeout on the relayed discovery. It can then happen that the
relayed discovery response is returned to the originator after the
originator has timed out. That's what Bill discovered :-(. 

Will fix.

> S 3.5.5.
>    A negotiation procedure concerns one objective and one counterpart.
>    Both the initiator and the counterpart may take part in simultaneous
>    negotiations with various other ASAs, or in simultaneous
> negotiations
>    about different objectives.  Thus, GRASP is expected to be used in a
>    multi-threaded mode.  Certain negotiation objectives may have
>    restrictions on multi-threading, for example to avoid
> over-allocating
>    resources.
> 
> "multi-threaded" is an odd word here. I assume you mean that you
> are doing multiple stuff at once, but you might actually write
> the system using non-multi-threaded techniques.

You could, and we need to make sure that the API allows for that.
Logically it's always multi-threaded though, even if you use
something like an event-loop technique. We can tweak the wording
accordingly.

> S 3.7.
> You seem to be going to a lot of trouble to deal wit session
> ID collisions. Why don't you just make session IDs 128-bit
> random values and then you won't have to worry about
> collisions.

Well, yes, the odds would be better with 128 than 32 but
that's extra bits on the wire and there'd still be the
birthday paradox... 

> 
>   The Session ID SHOULD have a very low collision rate locally.  It
>    MUST be generated by a pseudo-random algorithm using a locally
>    generated seed which is unlikely to be used by any other device in
>    the same network [RFC4086].
> 
> Why don't you just require a cryptographically secure PRNG?
> That will be required to implement the rest of this protocol

Well, certainly there will be for the security bootstrap
and the ACP. Is there a different reference for that?
 
> S 3.8.2.
> You seem to introduce a normative dependency on CDDL here.
> I see that it's in your changelog here, but what are
> your intentions about this document, given that CDDL seems
> to not even be a WG document

Resolve at AUTH48. If CDDL advances as planned in the CBOR WG,
we're fine. If not, we have a backup plan to add a short appendix
defining the subset of CDDL that we use. The text is already
drafted in case we need it.
 
> S 3.8.5.
>       It MUST contain a time-to-live (ttl) for the validity of the
>       response, given as a positive integer value in milliseconds. 
> Zero
>       is treated as the default value GRASP_DEF_TIMEOUT (Section 3.6).
> 
> Why do this, rather than just forbidding 0.

This feeds back to the API, so that a lazy ASA programmer doesn't
have to provide a value. (Also, I realised when fixing the discovery
timeout issue above that this default is low. Will fix.)

> S 3.8.6.
>    If a node receives a Request message for an objective for which no
>    ASA is currently listening, it MUST immediately close the relevant
>    socket to indicate this to the initiator.  This is to avoid
>    unnecessary timeouts if, for example, an ASA exits prematurely but
>    the GRASP core is listening on its behalf.
> 
> This is not secure. You need a secure indication of non-knowledge,
> not a transport-level close.

I don't see this as a security issue. There would be an alternative,
which is to send back an immediate M_DECLINE, but that is quite
a bit harder to implement and I don't really see an advantage:
the requester will just get back an error code in either case.
An ASA must always be ready for an error return.
 
> 
> S 3.9.5.4.
> What are the semantics of a Divert URI? What do I dow ith the
> path part?

Two things on this:
Following Alexy's comment, we are likely to add protocol/port
to this type of locator option. Given that, is there anything special
about the Divert case? A URI should be valid however you receive it.
 
> S 3.10.4.
> The semantics of "dry run" seem pretty unclear. Is it just
> "tell me if you would be sad about doing this"?

This is explained better in an earlier section:
https://tools.ietf.org/html/draft-ietf-anima-grasp-12#section-3.5.5

We will try to clarify this, at least with a cross-reference.

Thanks,
     Brian

_______________________________________________
Anima mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/anima

Reply via email to