Re: [Anima] Secdir last call review of draft-ietf-anima-grasp-api-07

[Brian E Carpenter <[e-mail]>]

Hi Joseph,

Thanks for the review. Comments in line...

On 28-Oct-20 06:25, Joseph Salowey via Datatracker wrote:
> Reviewer: Joseph Salowey
> Review result: Has Issues
> 
> I have reviewed this document as part of the security directorate's
> ongoing effort to review all IETF documents being processed by the
> IESG.  These comments were written primarily for the benefit of the
> security area directors.  Document editors and WG chairs should treat
> these comments just like any other last call comments.
> 
> The summary of the review is Has issues.  The document does needs a bit more
> discussion of the security implications.
> 
> 1. Authorization
> 
> In the security considerations section the document says that authorization is
> left for future work.  I would expect that the section should at least describe
> what the implications of no authorizations are. 

This isn't really an issue for the API itself, but for the underlying protocol
and for the ANIMA ecosystem. It's an issue that the WG explicitly deferred
(and it's now a chartered work item). Let me quote here what the GRASP spec
says in its security considerations:

>>    - Authorization and Roles
>> 
>>       The GRASP protocol is agnostic about the roles and capabilities of
>>       individual ASAs and about which objectives a particular ASA is
>>       authorized to support.  An implementation might support
>>       precautions such as allowing only one ASA in a given node to
>>       modify a given objective, but this may not be appropriate in all
>>       cases.  For example, it might be operationally useful to allow an
>>       old and a new version of the same ASA to run simultaneously during
>>       an overlap period.  These questions are out of scope for the
>>       present specification.

and what draft-carpenter-anima-asa-guidelines says:

>> Authorization of ASAs is a subject for future study. At present, ASAs are trusted by virtue of being installed on a node that has successfully joined the ACP. In the general case, a node may have mutltiple roles and a role may use multiple ASAs, each using multiple GRASP objectives. Additional mechanisms for the authorization of nodes and ASAs to manipulate specific GRASP objectives could be designed.

That draft is on the verge of WG adoption. The point is that the current
trust model is that we trust any node that has successfully joined the ACP,
and therefore we trust any ASA in such a node. We should state that clearly
in the text.

IMHO it would be out of scope for the API to say more but we should add a
reference to the GRASP Security Considerations.

> What risks are not being
> mitigated?   What modes of operations should not be used? 

Those are good questions for the WG to look at.

> In general leaving
> security items out suggests that the work is not ready for wide deployment. 
> Perhaps this is OK because the work is informational, but the document should
> probably say this.


Personally I don't think we have left a hole here, because there's a well-defined
trust boundary, but we should indeed state that as well as citing the GRASP spec's
security considerations.

> 
> 2. Authentication
> 
> Section 2.3.1.4 discusses the ASA_locator.  How is is the entity accessed by
> the locator authenticated?  How does the caller of the API know they are
> talking to the right entity?  I don't see any discussion of this in this
> document and there is very little in draft-ietf-anima-grasp-15 on this.    Is
> there a section in one of these documents I should be looking for?

No, you're not missing anything. The trust boundary is the ACP, and that takes
us back to the previous point. If we do decide that we need a fine-grained
trust model inside the ACP, we'll presumably need to extend GRASP itself
to add some form of authentication option beyond what GRASP over TLS can
provide. 

> 
> 3. ASA_Nonce
> 
> The ASA nonce is described as a security mechanism.  It is only 4 bytes long. 
> This seems short, making the ASA_Nonce vulnerable to collisions.  Why is this
> not a problem?

This isn't used on the wire, just locally within the GRASP instance,
so collisions can be excluded.

> How long is the ASA nonce supposed to be valid?  How often does registration
> happen?

At the moment, there is no sensible answer to those questions. When we develop
the authorization model, we'd definitely have to answer those questions and maybe
the nonce would have to be replaced by a crypto object.

> 
> 4. Session Nonce
> 
> Are there security implications of revealing the session nonce to the caller as
> suggested in section 2.3.1.7?  Could it allow for the ASA to bypass
> authorization if it knows this value?   Perhaps forming the nonce from the
> underlying session ID is not the best guidance?  Also it seems the underlying
> protocol session ID is only 4 bytes and collisions are likely and may be a
> problem for the protocol.

No, the collision risk is avoided because the session nonce includes the
ACP IPv6 address of the session originator. We should explain that
more clearly.

> 
> 5. Error Codes
> 
> In general, the list of API codes in the appendix is not described in the API. 
> It seems they should be.  Some of the error codes seem related to
> authorization, which is out of scope?

Our hope was the the WG could move faster on that topic, but the
incredible delays on BRSKI and the ACP made that impossible.
Our idea is certainly that register_asa() and register_objective()
should include authorization in the longer term.

> It seems that some of the error code could lead to disclosure of information,
> for example:
> 
> notYourASA       7 "ASA registered but not by you"  might reveal a valid nonce
> from an invalid nonce

Hmm... I don't think so. Let me glance at the code...

The only place where I found that error code useful was in deregister_asa()
and that doesn't return anything else. It could be used in an exhaustive
search attack to deregister an ASA, but in the current trust model that
doesn't seem like a significant risk. 

> 
> 6. Denial of service
> 
> are there protections in the underlying protocol for denial of service with
> respect to Flood(), Synchronize() or any other method?

There are recommendations about rate throttling for relaying GRASP Flood and
Discover multicasts, which should confine any multicast abuse to a single
link-layer segment. That's one good reason for using link-layer multicast only.
We also have recommendations for exponential backoff in the GRASP spec, but
of course a malicious sender could ignore those. In any case we can put a
specific pointer to that subsection of the GRASP Security Considerations.

DoS against the Negotiate or Synchronize parts of GRASP seems to be like
any other client/server protocol. I'm not sure what we can say in the
API spec about that. In my implementation (which is not intended to be
production quality) I've simply put finite queues for all the request
handlers, with silent discard if the queue overflows.

We'll collate updates to all reviews after the LC expires.

Thanks again
    Brian