Subject: [secdir] secdir review of draft-ietf-forces-protoextension-04

From: Tobias Gondrom <[email-masked]>
Date: Mon, 04 Aug 2014 23:17:42 +0100

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 draft is standards track and describes a few extensions to ease
programmability and to improve wire efficiency of some transactions. It
adds extensions to the ForCES Protocol RFC5810 and RFC7121.

The document appears mostly ready for publication.

The security considerations section (section 6) only refers to RFC 5810.

1 Nits and 3 Questions regarding the security section below:

Nits:
- section 3.2.3.1. last paragraph:
s/writting/writing

Questions:
1. section 3.2.3.1. last paragraph: 
"On an FE which supports both RESULT-TLVs and EXTENDEDRESULT-TLVs, a
   master CE can turn on support for extended results by setting the
   value to 2 in which case the FE MUST switch over to sending only
   EXTENDEDRESULT-TLVs."

Not sure about the full network topology, but could that enable a master
CE to effectively make the replies of a FE unreadable to other
old-version CEs (which can only understand RESULT-TLVs) by switching on
the "only" new EXTENDEDRESULT-TLVs?

2. with the New Codes in section 3.2.1. :
do any of the new more detailed error codes (e.g. "E_EMPTY |  Table is
empty") pose a risk of data leakage of information detail that was not
intended to be disclosed in that detail in the original RFC 5810?

3. section 3.1.:
Could the new "TABLERANGE-TLV", which requires significantly more
computing on the receiver per request from the sender, enable an
attacker for a type of Denial of Service attack scenario?

NOTE: though I reviewed the ID, I did not validate the XML structure in
appendix A.

Thank you and best regards.

Tobias

---

From: Tobias Gondrom <[email-masked]>
Date: Tue, 05 Aug 2014 12:13:56 +0100

Hi Jamal,

thanks for the fast reply.
And thank you for taking the time to answer my questions.

Please see my answers only as my personal comments and I am no expert in
the forces spec. So I might very well be wrong.

Answers inline.

On 05/08/14 11:49, Jamal Hadi Salim wrote:
> Hi Tobias,
>
> Thank you for the review. Comments in line.
>
> On Mon, Aug 4, 2014 at 6:17 PM, Tobias Gondrom
> <[email-masked]> wrote:
>> 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.
>>
>> programmability and to improve wire efficiency of some transactions. It adds
>> extensions to the ForCES Protocol RFC5810 and RFC7121.
>>
> Fixed.
>> Not sure about the full network topology, but could that enable a master CE
>> to effectively make the replies of a FE unreadable to other old-version CEs
>> (which can only understand RESULT-TLVs) by switching on the "only" new
>> EXTENDEDRESULT-TLVs?
>>
> Hrm. In a cluster of CEs where you have new + older CEs, yes - that is possible.
> Caveat: The CEs are spec-ed to talk to each other on CE-CE plane. I would expect
> what you described to be a "buggy" decision by the master CE.
> Would text that describes this as a faulty setup be reasonable to add
> to cover this?

Yes. That might be a useful point.
But please see my review questions only as personal comment. I would not
want to make suggestions that go against the WG views.

Small add-on question:
Not sure whether there are potentially two scenarios here:
1. the first as you described might be a "buggy" decision by a master CE
to switching on the "only" new
EXTENDEDRESULT-TLVs and by that switching off the old codes for the
other old CEs.
2. wondering whether there might be a second scenario where a malicious
person would intentionally try to insert one new CE and switch the FEs
to EXTENDEDRESULT-TLVs to "blind" an existing "old" CE network to some
of the error codes?

> There is no new information per-se being sent. Implementations sent back
> different things like E_NOT_FOUND or E_INVALID_PATH etc before this
> and it was left up to the controller to interpret i.e  no explicit coherency
> for what ended up being a common use case. Therefore E_EMPTY is
> providing a little more clarity.

Ok.

>
>> 3. section 3.1.:
>> Could the new "TABLERANGE-TLV", which requires significantly more computing
>> on the receiver per request from the sender, enable an attacker for a type
>> of Denial of Service attack scenario?
>>
> Note: it is the controller that would be doing this. If your brain wants
> to attack your legs by punching them endlessly, not sure what those
> legs could do;->
> (cant run unless brain tells you to). i.e there is explicit, unquestionable
> trust of the controller. If the controller is compromised, then we are
> talking about
> a different ballgame altogether (and nothing we can do here resolves that).
>
> I am not sure i agree on the "significant compute" statement[1].
> You are right however that a not so good implementation doing the filtering
> at the source would suffer because it has to check for index validity.
> An implementation can reject table range requests by issuing a
> response of E_INVALID_TFLAGS (section 3.1)
> Probably we need to add a small discussion to describe this scenario as another
> one that would neccessitate the issuing of E_INVALID_TFLAGS?

Yes. I understand.
My concern is the following: AFAIK (and I might very well be wrong) so
far the protocol mostly required many requests to receive the data from
the FEs. Which is inefficient, but equally put symmetric compute
resource requirements on both points (CE and FE). Aka, you would see a
many requests to put a FE to work. With the new "TABLERANGE-TLV", it
seems I can send one request which will trigger more computing activity
on the FE. So yes, you are right with your analogy between brain and
legs and the trust relationship to the controller. However, before, your
"brain" could send one command at a time to the legs, now it could send
a whole marching program to the legs with one single request. And send
multiple such requests in succession. Could that pose risks for denial
of service (accidental or malicious) that might not have been
anticipated in RFC5810?

> At least one other person (the shepherd) has taken of this, so no worries.

Thanks.

Cheers, Tobias

>
> Thanks again for taking the time to review.
>
> cheers,
> jamal
>
> [1] The filter essentially does a table dump,
> checks what the index is and adds it to the response if it matches.
>
>

---

From: Tobias Gondrom <[email-masked]>
Date: Tue, 05 Aug 2014 14:11:23 +0100

Thanks for your response.
Thank you. I think my concerns have been heard and leave it to you
whether you want to amend text in some places or not.
Best wishes, Tobias

On 05/08/14 13:55, Jamal Hadi Salim wrote:
> Hi Tobias,
>
> On Tue, Aug 5, 2014 at 7:13 AM, Tobias Gondrom
> No worries, an extra pair of eyes with a different view always provides a
> perspective we may have overlooked.
>
> Ok, will find a spot to discuss this.
>
>> Small add-on question:
> An arbitrary CE should not be able to join a cluster or win an election for
> mastership without authentication and authorization.
> i.e this is all covered by the trust issue i described earlier.
> The FE has explicit trust with the CE. If that trust is compromised, security
> breach is not limited to just this one issue.
> The reason i described the other scenario as buggy is the CEs should be
> able to coordinate amongst themselves if they have discrepancies
> (although it is outside scope of ForCES to describe how that is achieved)
>
>> Yes. I understand.
>> on the FE.
> Note that a single request to get many (possible) responses back already
> exists *today*. Example, I can send one message to dump 10M table rows.
> Nothing new there. ForCES says to restrict the transaction pipeline window size
> (default of 1) so you cant send 10000 of those - and if you could,
> then such requests
> which will cause problems of starving other possible transactions because you
> are hogging both bandwidth and cpu at the FE . It is likely a buggy
> control application to do this. But these are *trusted* apps - they are just
> bad code as opposed to malice and the transaction windowing should minimize
> damage.
> In our implementation (and specified in RFC 5811) we would lower the priority
> of these dump responses in a work conserving way
> if there are other high prio requests (like a SET for example).
>
> A simplistic approach to retrieve a table range would be to get all the 10M
> table rows and then do the filtering to select 2000 rows in the controller.
> This uses more bandwidth and possibly more CPU on the FE (slave) since it
> has to dump *all* rows.
> What you describe as "more computing activity" boils down  to running the filter
> i described above at the FE device instead of the controller.
> In my experience this hasnt been an issue - but i am willing to be cautious.
>
>> So yes, you are right with your analogy between brain and
> Not any more than they do today. i.e Malice is solved by the general security
> infrastructure; if a CE can get into the cluster, be authenticated and vetted as
> legit, wins an election and starts telling the FE what to do, then we have
> a bigger problem.
> OTOH, bugs may exist and we gotta protect against those.
>
> Thanks again for taking the time.
>
> cheers,
> jamal

---

From: Jamal Hadi Salim <[email-masked]>
Date: Tue, 5 Aug 2014 06:49:20 -0400

Hi Tobias,

Fixed.

cheers,
jamal

---

From: Jamal Hadi Salim <[email-masked]>
Date: Tue, 5 Aug 2014 08:55:44 -0400

Hi Tobias,

cheers,
jamal

---

From: Jamal Hadi Salim <[email-masked]>
Date: Tue, 5 Aug 2014 09:21:47 -0400

Thanks Tobias.
I will make the changes we discussed - wait for other reviews and amalgamate.
The AD should be able to decide if we need a revised publication.

cheers,
jamal