Subject: [tsvwg] Some comments on draft-ietf-tsvwg-ecn-tunnel-02]]

From: Gorry Fairhurst <[email-masked]>
Date: Thu, 25 Jun 2009 20:51:14 +0100

Bob,

I've just read draft-ietf-tsvwg-ecn-tunnel-02, and have some comments.
Thanks for preparing this new version, which is certainly easier to read
(and seems a little shorter) than the previous one, I think as I said at 
the IETF, the general approach in this draft seems good to me.

I've separated my comments into two parts. The first parts are comments
which I'd hope you may be able to respond to. The second parts I think 
may be editorial comments (but do ask questions, if you do not 
agree/understand).

Best wishes,

Gorry

---
1) Updates RFC4301?

I do not understand whether the document updates RFC 4301, or is an
update that aligns with the changes made in RFC 4301. At some points,
the document says the former, whereas at other points I interpret it as
the latter. This needs to be very clear in the abstract and the document
text.
---
2) Abstract

I'd be happier if the abstract could call-out that this updates the 
behaviour of tunnel end points and middleboxes. That is say something so 
that people using and configuring these devices know to read this document.

I'd also like the abstract to be clear that there is more than one mode
and say something of this topic - I'm not sure what, because I still
have questions in this space (see below).

The abstract should say the set of RFCs that are updated by the document 
(but not cite these as references), e.g.  This specification updates 
RFC3168 {and RFC4301 --- if it does}.	
---
3) Current status of PCN work

Section 3 includes:
/  A  number of wasteful or convoluted work-rounds to this problem are
       being considered for standardisation by the PCN working group (see
       Appendix D), but by far the simplest approach is just to remove
       the covert channel blockages from tunnelling behaviour, that are
       now deemed unnecessary anyway./
- Is this still the case? If this document removes this constraint, can
we just say so?
---
4) Recommended default and tunnel negotiation method for non-IPsec case?

- The following set of points are all very much related, and I would 
like some more detail on what you envisage:
---
Section 4.1
- The current section seems to step-back from actually recommending what
the mode that SHOULD be used when there is no tunnel negotiation. This
leaves me with some questions about  what a tunnel endpoint needs to do
by default.
- What do you think the document should say?
---
Section 5:
   /Before switching to normal mode, a compliant tunnel ingress that does
    not know the egress ECN capability MUST negotiate with the tunnel
    egress./
-It is not clear to me *HOW* this tunnel negotiation would normally be
performed. Can you clarify?
I found this paragraph in Section 5.3
/   Therefore, a compliant tunnel ingress MUST at least implement `normal
    mode' and, if it might be used with arbitrary tunnel egress nodes, it
    MUST also implement `compatibility mode'./
---
Section 5.3, para 2:
Section 5.3, para 4:
- Para 2 and later talks of safeguards, and if I understand it makes a
requirement that tunnel ingress MUST start in compatibility mode if it
can not negotiate. How do tunnels find out this information? (see earlier).
- I am not sure how the tunnels we be configured outside of IPsec, so
could you help me understand how this negotiation will take place?
----------
5) Informative status of section 4.3

Section 4.3 is stated as informative
- Can you remind me why this section is informative?
- It seems that there  are strong messages here on the use of the
method. /should/ and /should  not/ are used many times in a
non-normative way. I'd like to explore whether the WG would like to make
these guidelines normative.
---
6) Relation of section 6 with Updating IPsec

Section 6:
- I do not understand this section. I'd have expected a specific section
on how this document updates IPsec - if it does, but I was not sure
whether this section was intended to be the update to IPsec, or whether
the changes relate to all tunnel end points (if so, how
- The discussion here seems to assume IPsec and IKEv2, but I do not
understand the context, either more is needed or much less is needed.
---
7) Alarm raised when ECT(1) encapsulates CE.

Section 6, bullet 3 / Figure 4:
- This says an alarm is generated, Is this really a problem?
- I understand that ECT(0) is allowed in some cases to be re-mapped to
ECT(1). This then makes me wonder why a packet that was remarked from
ECT(0) to ECT(1) then raises an alarm when the inner packet was CE
marked. Have I understood this correctly?
/outer ECT(1) with inner CE/
- To me, this also seems compatible with RFC 4301.
---
8) "Danger" of not forwarding congestion indications?

Section 8:
/  In fact, ensuring congestion markings are not
    lost is itself another aspect of security, because if we allowed
    congestion notification to be lost, any attempt to enforce a response
    to congestion would be much harder./
- I accept that loss of congestion marking can impede congestion
control. I do no agree that this is accepted as a factual security
threat to the Internet.
---
9) Judgement on ECN-N?

Section 8:
This last statement on ECN-N adoption is not required or neccessary for
this document. I would rather not see statements such as /and there has
been no evidence that anyone has implemented it beyond the author's
prototype./ introduced as a side item:

    4.  The ECN nonce [RFC3540] currently has experimental status and
        there has been no evidence that anyone has implemented it beyond
        the author's prototype.
---
10) Size of Appendices.

My review ran out of steam as I read the pages of appendices and I am
not currently persauded that most of the text in the appendices is
needed to publish the updated specification.

Section A.2:

There are many places where the English could be improved. Examples from
the final para include:
/ingress confuses a proposed.../
/It ends up removing excessive amounts of traffic uncessarilly.../

Section B seems to be related to the rationale for PCN, rather than the
use of ECN over tunnels. Is this really required in this specific document?

B.1 seems to be a background to ECN
B.2 again reads like a paper, but the language is rather imprecise

Appendix C seems OK, but is it needed?

Appendix D seems to justify the work, but since this is now adopted, is
the text neccessary?
Section D.1 could be removed to improve the document.

The appendices currently seem to read more like a technical paper than
an IETF PS. If these appendices really are required, then some editorial
work is required to make this a standards quality contribution, and
clarify what actually needs to be said. After this, someone will need to
review the statements about the discussed IETF protocols carefully -
perhaps by referral to the appropriate WGs.
---
11) Relevance to GRE, IP-in-IP, L2VPN, etc, etc.

- If this change is intended to be appropriate to a range of tunnel
behaviours, how do we make sure implementors clearly understand this?
- Perhaps it would be helpful to list a set of example encapsulations at
the front of the introduction in para 1.

----------------------------------------------------------------------
* NiTs
---
Consider changing all:
/doesn't/
- to say /does not/
---
Abstract:
/in order that these combinations may be usefully/
- could we say /to allow these combination to be usefully/
---
Introduction, para 1:
/Three main change/
                  ^
  - please insert 's'
---
para after table 1,
/it must follow rules to ensure marking is propagated.../
    ^^^^^
  - could say 'needs' to avoid conflict with RFC2119 keywords?
---
/The newly specified behaviours/
  ^^^^^^^^^^^^^^^^^^^
- Please say /The new behaviours specified in this document.../
---
/   The rules for constructing the ECN field to be forwarded after tunnel
    decapsulation ensure this happens, but they are not wholly/
- That's the rules in the documents that this document updates?
- Some references would be helpful here.
---
Para starting /as a result,/
The following text, while true, will read oddly once published. I think
this sentence could be usefully rephrased to be more objective:
/As a result, an unfortunate sequence of standards actions has left us
    with nearly the worst of all possible combinations of outcomes,
    despite the best endeavours of everyone concerned./
- later in the para /still/ will likely be changed by publishing this
document and /now is/ was true at the time it was written, but not for
evermore.
---
1.1 Scope, para 2:
- should this refer to RFC 2983 for example?
---
1.2 bullet 1:
/current tunneling behaviours/
- legacy methods? - It would be clearer to explicitly reference the
specs. here.
---
section 3, para 1:
/It merely recaps/
    ^^^^^^^ - could we delete this word?
---
Figure 1
- final line has a spurious 'e' character.
---
3 paras after figure 1:
/But.../ is used twice at the start of a sentence. Consider deleting the
firs case, and replacing the second with /However,/
---
Page 11, and following:
- The covert channel appears several times. I think it is good to
document this and explain the implications, but it seems to be repeated
several times. It also appears again at the end of section 9.
---
End of section 3, first bullet, para 1:
/Firstly/
- Consider using /First,/
---
Section 3.1, end:
/ To ensure copying rather than resetting CE on ingress will not cause
    unintended side-effects, Appendix A assesses whether either harm any
    security, control or management functions. /
/It finds that resetting
    CE makes life difficult in a number of directions/
- I think I understand this, but it was not an easy read.
---
End of section 3, first bullet, para 2:
/dangerously discarding/
- I agree that this results in discard of end-to-end congestion signals,
and that from a network stability perspective this is undesirable, but I
think the word "dangerous" needs more explanation.
---
After Fig 3:
/as the ingress end of RFC4301 IPsec/
- Could this be written better, as something like:
/as the tunnel ingress processing of IPsec in RFC4301/
---
/  Not only will this streamline PCN
       standardisation, but it could also streamline other future uses of
       these codepoints./
- What exactly does this mean?
---
End of 4.2:
/not so often that the illegal combinations would amplify into a flood
of alarm messages/
- I agree that these need to be controlled. Can you say "rate-limited" -
is there some more helpful bound we can cite here for guidance?
---
Section 4.3:
/S.5/
  ^ - Section?
--
Section 4.3, bullet 1
/should not set the ECN field/
             ^^^
- Do you mean set this to CE? or do you mean re-mark? Please be more
specific.
---
Section 5:
/Note: in/
  ^^^^^^^
- Would it be OK to say "In"?
---
Section 5, para 1:
/doesn't need/
     ^^^ - could we say /does not need/
---
Section 5, para 1:
/to know what mode the ingress is in./
     ^^^^^                      ^^^^^
- Could this be /need to determine the mode of the tunnel ingress/.
---
Section 5.1, para 2:
/whichever mode the tunnel was in/
                            ^^^^^^
- could you improve the English of this line.
---
Section 5.1, para 3:
/as its behaviour/
  ^^^^^^
/since the behaviour/
---
Section 5.1, final para:
/doesn't/ and /don't/
- could you say /does not/ and /do not/
---
Section 4.3, title:
/Upgrading/
  ^^^^^^^^
- Is this upgrading or updating, I'd prefer the latter if you think this
is applicable.
---
Section 5.3, para 1:
/dangerously discarding/
- I agree that this results in discard of end-to-end congestion signals,
and that from a network stability perspective this is undesirable, but I
think the word "dangerous" needs more explanation.
---
Section 5.3, para 1:
/ratchet up/ and /ratchet down/
- I agree, but this could be written more clearly.
---
Section 5.3, para 4:
/if the egress says/
                ^^^
- advertises? - can you select a better word?
/doesn't understand the question/
- could you rephrase this please in terms of the protocol or set of
actions that need to take place?
---
Section 6, para 1:
/is a change to the ingress/
     ^^^^^^^^
- is it better to say /updates the ingress/?
---
References:
draft-ietf-pcn-architecture has now been published as RFC 5559.
---

---

From: Bob Briscoe <[email-masked]>
Date: Fri, 26 Jun 2009 14:28:54 +0100

Gorry,

Tx for such a thoro review. It seems nearly all the comments are 
about poor description, rather than any protocol disagreements. Is this fair?

The only substantive disagreement seems to be over whether ECT(1) 
encapsulating  CE should raise an alarm at the egress.

Inline...

At 20:51 25/06/2009, Gorry Fairhurst wrote:
>(and seems a little shorter) than the previous one, I think as I 
>said at the IETF, the general approach in this draft seems good to me.
>
>which I'd hope you may be able to respond to. The second parts I 
>think may be editorial comments (but do ask questions, if you do not 
>agree/understand).
>
>Best wishes,
>
>Gorry
>
>
>---
>1) Updates RFC4301?
>

I thought I'd made this clear, but obviously not. I included the 
section below which is a hint to the RFC editor to make this clear in 
the headers and in the RFC index.
/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\
Additional changes to the RFC Index (to be removed by the RFC Editor):
In the RFC index, RFC3168 should be identified as an update to 
RFC2003. RFC4301 should be identified as an update to RFC3168.
This specification updates RFC3168 and RFC4301.
/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\/\

Updating 4301 isn't as bad as it sounds tho:
* The tunnel ingress behaviour is in line with 4301.
* The tunnel egress behaviour adds to 4301, but doesn't change any 
4301 behaviours that currently exist. So compliance with this spec 
has no backward compat issues.
Therefore compliance can be called for only as it is needed (e.g. for PCN).

>behaviour of tunnel end points and middleboxes. That is say 
>something so that people using and configuring these devices know to 
>read this document.

I can say it updates the behaviour of *some* types of tunnel 
end-point. But it would be too complicated for an abstract to say 
exactly which ones.

>have questions in this space (see below).

The mode stuff is not needed in most cases, and the actions in this 
RFC make modes less likely to be needed. It's only for some cases 
that are trying to be completely generic and dynamic. And even then, 
it's only carrying through detailed stuff that was already in 
RFC3168. As it's not the new thing, I'd say it's not appropriate for 
the abstract.

>The abstract should say the set of RFCs that are updated by the 
>document (but not cite these as references), e.g.  This 
>specification updates RFC3168 {and RFC4301 --- if it does}.

I'd rather get the RFC Editor to put "updates RFC3168 and RFC4301" in 
the status header, than crowd the abstract.  (that's why I added the 
note for the RFC Editor in S.6 "Changes from Earlier RFCs")

>- Is this still the case? If this document removes this constraint, can

It's still the case, because the PCN work can't assume this spec will 
get through yet - last calls can raise probs even tho I've been 
careful to ensure there are no back-compat issues for the security area.

>- What do you think the document should say?

OK, I thought I'd made it clear, but obviously not.
4.1 refers forward to 5. on this issue (so as not to crowd the main 
point with details that aren't usually necessary).
5.1 says "...the new normal ingress behaviour introduces no backward 
compatibility isses with IKEv2 [RFC4306] IPsec [RFC4301] tunnels, and 
no need for any new modes, options or configuration. "

I think I see the problem. I tend to start by describing the logic 
for all cases, then say what this implies for each. Instead I should 
assert what should be done in each case, then say why. I'll do the 
latter in the next rev.
It's a good point. Implementers just want to find the bits they need 
to read and ignore the rest.

>performed. Can you clarify?

Again, I've obviously made it hard-reading by giving the general 
logic for all cases, rather than listing each common case. How about 
text something like this to start off with:

"Both endpoints of an RFC4301 tunnel can be sure that the other end 
is 4301-compatible, because the tunnel is only formed after key 
management has been completed, at which point both ends will be 
RFC4301-compliant by definition. Therefore, an IPsec tunnel ingress 
compliant with RFC4301 does not need to implement compatibility mode 
for legacy ECN tunnels.

Similarly, the implementer of any (non-RFC4301) tunnel ingress that 
is designed to be deployed at the same time as its corresponding 
tunnel egress, will not need to implement compatibility mode for 
legacy ECN tunnels.

However, an implementer might want to create a general-purpose tunnel 
ingress that includes some form of tunnel endpoint discovery to 
dynamically create and destroy tunnels to different tunnel egresses, 
driven by the varying needs of arriving packets. If the tunnel 
discovery process is arranged to only ever find tunnel egresses 
compliant with normal mode of RFC3168 or a later ECN tunnelling 
specification, then the ingress would not need to implement 
compatibility mode. But if earlier tunnel egresses might be 
discovered, the ingress would need to implement compatibility mode as 
well as normal mode, and also implement appropriate negotiation as 
defined in RFC3168."

Would this text be OK?

>    MUST also implement `compatibility mode'./

Is this text a problem? Or are you saying it solves the above issues? 
Or you want it said earlier?

>can not negotiate.

It doesn't say "if it cannot negotiate". It says if it "does not know 
the egress ECN capability ".

>How do tunnels find out this information? (see earlier).

It's easy if both ends of the tunnel are statically config'd (ie the 
info is part of the config, and it's up to the sysadmin to get it right).

I'm trying to get rid of modes, but I have to allow for the same 
problem RFC3168 had to allow for. So I have to allow the same 
negotiation as RFC3168. I don't know if anyone does this, or how 
exactly they do it. I've been told negotiation is frowned on. But I'm 
not adding anything beyond what RFC3168 said.

I'll say "...negotiation as in RFC3168" if that helps.

In S.6 I have clarified that RFC4301 IPsec implies the negotiation 
machinery of RFC3168 isn't needed for an IPsec tunnel any more:

"The optional ECN Tunnel field in the IPsec security association 
database (SAD) and the optional ECN Tunnel Security Association 
Attribute defined in RFC3168 are no longer needed. The security 
association (SA) has no policy on ECN usage, because all RFC4301 
tunnels now support ECN without any policy choice. "

>could you help me understand how this negotiation will take place?

Maybe I don't understand customary procedure, but I imagine that 
guidelines for future protocol design are generally informative, and 
normative is only needed when a standards track doc is written that 
uses the guidelines. As this doc is intended for standards track, I 
wanted to make the boundary clear, where it had shifted into 
"informational land".

I don't believe guidelines for future protocol design mean anything 
more than "you'll need to give good reasons, if you break these". 
Perhaps my philosophy for encouraging evolvability doesn't match the 
mentality of the typical IETFer these days tho, so discussion is welcome.

>the changes relate to all tunnel end points (if so, how

This section is meant to summarise how the changes introduced in 
prior sections update previous RFCs.

But the section is not organised by previous RFCs. It's organised by function:
- ingress
- egress
- negotation between the two (if present).

For each function, it says which aspects of previous RFCs are updated.

This was designed to help implementers, who would be working on each 
function, having to pull together the relevant RFCs for that function.

>understand the context, either more is needed or much less is needed.

It doesn't assume IPSec. It just says what has changed if you're doing IPSec.

IKEv2 key negotiation is integral to RFC4301. But David Black told me 
there could be corner cases that use manual keying instead.

I could remove the stuff on the manual keying corner case. I'd like 
to take David Black's advice on that. He implied if I put it in it 
will get unnecessarily complicated given no-one does this in 
practice, altho RFC4301 allows them to. But if I leave it out, 
someone could point out that we haven't thought about it.

Does this help?

>- This says an alarm is generated, Is this really a problem?

In practice, implementers tend to drop unexpected combinations, on 
the spurious pretext that there might be a security risk.

The idea behind recommending alarms is to give implementers a 
legitimate way to not drop combinations just because are unexpected 
today, given they might be used in the future. While still allowing 
operators to monitor for possible attacks.

Then, if there's a sudden rise in alarms of this type, the operator 
can engage brain and say either
- "Ah, that's because RFC7024 is starting to be implemented, we can 
turn off those alarms" or
- "Hmmm, there's not even an experimental standard that would cause 
this strange combination, could this be a novel attack?"

>ECT(1).

If you know of these cases, then fine, I'll remove the need for an 
alarm. But I wasn't aware of any. Any specifics?

>This then makes me wonder why a packet that was remarked from

RFC4301 & RFC3168 define the same output (CE) as I do for this 
combination. The issue is over whether the combination of inputs 
would ever be *expected* to arise, given none of the other ECN RFCs 
could lead to this happening.

I'm not aware of a valid ECN protocol transition that would produce 
this combination, so I've suggested it should raise an alarm. RFC3168 
had some cases that raised alarms, but I just think it missed this 
one. If I'm wrong, I'll happily retract.

>control. I do no agree that this is accepted as a factual security

Flooding attacks classify as a security threat (not information 
security, but still security). Enforcing congestion control is a 
valid approach for handling flooding attacks. If we allow explicit 
congestion information to be thrown away, it makes both responding to 
it and enforcing a response to it harder. That's all I'm saying. 
Surely that's non-controversial?

I appreciate that a large part of the security community focuses on 
using information security rather than congestion control enforcement 
to solve flooding, but that doesn't mean they are right. And others 
disagree with them.

>        there has been no evidence that anyone has implemented it beyond

An RFC changes as it moves from draft to RFC. To start with it has to 
justify what it's doing. As people accept the arguments, a lot of 
justification text becomes removeable. I'd be happy to remove 
justifications eventually (as text like this can get dated), but I'd 
rather not until the arguments have got past the security area.

We have two conflicting choices to reconcile. This is one of the 
reasons for the choice made. If we don't mention it, it weakens the case.

>needed to publish the updated specification.

I'm also running out of steam here, as I'm meant to be going on 
holiday for a week now. As my wife is politely suggesting a cup of 
water might end up being spilled over my keyboard, I'll also be brief...

>perhaps by referral to the appropriate WGs.

There's a balance needed between recording why things were done and 
not making the thing flowery and unweildy. Point taken. I'll give 
this some attention in the next draft.

>the front of the introduction in para 1.

In the rev between -01 & -02 I took out all the stuff on non-IP-in-IP 
tunnels. It will probably re-appear in some future draft, but not this one.

If I've left any text in here that implies this is relevant beyond 
IP-in-IP, pls point me at it. I thought I had taken it all out.

Tx for the nits below. I'll handle them.

Bob

________________________________________________________________
Bob Briscoe,               Networks Research Centre, BT Research