Re: [IPsec] Review of draft-ietf-ipsecme-ddos-protection-06

["Valery Smyslov" <[e-mail]>]

Hi Paul,

thank you for part two of your review.

> This is part two of my review. I do think the document needs some work
> moving text to better locations and I have some questions I would like
> to see resolved. I wrote down some nits but stopped doing that in the
> end because I think chunks of text shoud be moved. By biggest issue
> is that section 6 and section 7 are not clearly separated, and I see
> various chunks of text I think is in the wrong section (or is section 6
> text repeated in section 7)
> 
> I think this document should update 7296 due to adding non-encrypted
> payloads to IKE_AUTH - even though the core IKEv2 RFC does not say that
> is not allowed. Someone implementing 7296 should be aware of it to allow
> it in their implementation.

Hmmm... Not sure it is needed. As you said RFC 7296 doesn't explicitely
prohibit that (so we don't violate its requirements) and there is a clear negotiation 
mechanism with puzzles, so that old implementations won't be affected.

I think we should be consitent in our policy whether to update core RFC or not.
For example, RFC 5723 (Resumption) defines new exchange IKE_SA_RESUME
that can be used in conjunction with IKE_AUTH instead of IKE_SA_INIT.
RFC 7296 tells only about IKE_SA_INIT + IKE_AUTH, however 
RFC 5723 doesn't update IKEv2 RFC. Is our situation much
different? I don't think so.

To be clear - I'm not against updating RFC 7296, I just think it is not needed.

> I will redo a nits/grammer check on the next iteration of the document.

OK.

>  Note, that it is not possible with clients using NULL Authentication,
>  since their identity cannot be verified.
> 
> It feels that this sentence should be followed by some specific advise for
> this category of clients?

What's your suggestion? We've mentioned several times that NULL Authentiacted 
peers are special case for DoS defense, here we just remind about this fact
to readers once more. I don't think any special advice could be gived here.
If you have one, please share it with us.

> Section 6 descibes in item 1: "A general DDoS attack" some numbers that I
> find dangerous to follow. It descibes a scenario of 20 tunnels per second
> as expected that when increased to 100 tunnels per second is considerared
> a DDOS attack. But that does not take into account network failures
> that would cause a large chunk of clients to reconnect at once. While
> the draft says this "can be interpreted as an attack", implementors
> might just put in these hardcoded numbers from the document. I'd rather
> describe it a bit different so we don't give them these absolute numbers.

Well, there is a text in the beginning of this section:

        the numbers given here are not normative, and their purpose is to
        illustrate the configurable parameters needed for defeating a
        DDoS attack.

Isn't this text enough for any sane implementer not to hardcode those numbers?

> for example:
> 
>  Typical measures might be 5 concurrent half-open SAs, 1 decrypt
>  failure, or 10 EAP failures within a minute.
> 
> Why is this "typical"? And again, these numbers provide too tempting easy
> numbers for implementors to hardcode in their implementation. And I
> think these are speculation at best.

How about:

            For example, measures might be 5 concurrent half-open
            SAs, 1 decrypt failure, or 10 EAP failures within a
            minute. 

>    puzzle difficulty should be set to such a level (number of zero-bits)
>    that all legitimate clients can handle it without degraded user
>    experience.
> 
> This is of course the big issue of this draft. Is this possible at all?
> Note te _lack_ of specific numbers here :)

Yes, no specific figures can be given here. This text just says
that unless the responder finds itself under severe DoS attack,
puzzles, if used, must not be hard.

> I don't understand this paragraph:
> 
>    it is best to begin by requiring stateless cookies from all
>    Initiators.  This will force the attacker to use real source
>    addresses, and help avoid the need to impose a greater burden in the
>    form of cookies on the general population of Initiators.
> 
> Perhaps the "form of cookies" was meant to say "puzzles" ?

Agreed.

> And this one confuses me too:
> 
>    When cookies are activated for all requests and the attacker is still
>    managing to consume too many resources, the Responder MAY increase
>    the difficulty of puzzles imposed on IKE_SA_INIT
> 
> But up to now, we haven't been given advise to enable puzzles, and now we
> are recommended to increase the difficult of puzzles?

This text has already been corrected (thanks to Dave's review).

>    If the load on the Responder is still too great, and there are many
>    nodes causing multiple half-open SAs or IKE_AUTH failures, the
>    Responder MAY impose hard limits on those nodes.
> 
> Unlike elsewhere, it does not describe what failure to send back, if
> anything, to the responder. Sending back a NOTIFY might actually not
> be desirable, as it would tell the attacker their attack has reached
> a good enough volume to lock out real clients. Some advise on how
> to handle this scenario is needed.

Section 4.2 describes a "Hard Limit" as situation when any 
further IKE_SA_INIT requests are rejected. 
In any case, I think section 7 is explicit enough that no NOTIFY
is sent in this case.

And I think that sending back a NOTIFY is a really bad idea.
First, as you said it'll be an extra information for attacker 
and second - it consumes additional resources from responer
that is already under DoS attack, for no good reason.

> And confusingly, only NOW are puzzles suggested as a next "last step",
> even though before this it already told us to incrase puzzle strength.

The text earlier is about puzzles for selected IP addresses, while
here it advises to impose puzzles on all initiators.

> Why is there not an option to add puzzles to the CREATE_CHILD_SA to
> punish just those clients requesting too many of those? (I'm not sure
> if it is a good idea, I'm asking because I'm not sure it is a bad idea)

Once IKE SA is created you don't need to use puzzles.
Section 5 lists less controversal and more effective countermeasures.

> Section 7 has:
> 
>  According to the plan, described in Section 6, [.....]
> 
> We are not Cylons :)
> 
> Seriously though, section 6 describes _when_ to activate puzzles, and section 7
> should describe how to activate puzzles without any contetx of when to enable it or not.
> Currently, section 7 repeats some of the "plan" of section 6, which should not be
> needed and makes the implementation section longer/harder to read. Some of the text
> in section 7, like the new "processing some fraction of requests" should be in section 6,
> not section 7.

Well, my understanding is that section 6 gives a higher level picture (roadmap, strategy) 
of how responder is recommended to defend itself, while section 7 is very specific about 
how puzzles should be used in IKEv2 to get interoperability. I agree that they overlap,
but I don't think they must be merged. When you start implementing puzzles
you may read the draft starting from Section 7. 

So, what's wrong with the fact that SEction 7 refers to SEction 6?
Do you want the reference to be removed?

> 7.1.1 states: "then it MUST include two notifications in its response message"
> So earlier text said "may" also use cookies, and this text assumes there puzzles
> can only happen with cookies. That is contradicting. I would say remove the requirement

Sorry, where is contradiction? You may always use cookie as described in RFC 7296.

> in section 7 and change the text in section 6 to make it obvious that cookies should be
> the first line of defense and should still be used when handing out puzzles on top of
> cookies.

Section 6 already recommends to first require cookies from all initiators before 
making next step and require puzzles.

> If you mean to talk about the interaction or combination of puzzles and cookies, perhaps
> a separate section on that would be most clear.

Yes. The text above is only meant to say that puzzle notification
must always go together with cookie notification. I don't think 
we need a separate section for this requirement. How about:

                If the Responder makes a decision to use puzzles, then it 
                includes two notifications in its response message - the COOKIE notification and
                the PUZZLE notification. Note that the PUZZLE notification MUST always 
                be accompanied with the COOKIE notification, since the content of the COOKIE 
                notification is used as an input data when solving puzzle.

> 7.1.1.1 introduces a term ZBC which I have no idea what it means yet. It then talks

Already introduced in 4.4.

> about difficulty level 0 which I don't know what that is. Does it translate to number

The format of the PUZZLE notification is described in 8.1

> of zero's in solution? If so I would expect level 1 to be the lowest? Maybe this
> discussion should go into the section 7 introduction. What is the general idea of the
> puzzle, what are difficuly levels, etc.

I think 4.4 serves this.

>    The
>    Responder MAY set different difficulty levels to different requests
>    depending on the IP address the request has come from.
> 
> I would think that MAY should be stronger, a SHOULD ? If you can detect a few problem
> causing IPs or IP ranges, you made good points saying to only punish those with puzzles.

I don't think so. It is responder's local matter how to defend itself.

>    The Responder parses received SA payload and
>    finds mutually supported set of transforms of type PRF.  It selects
>    most preferred transform from this set and includes it into the
>    PUZZLE notification.
> 
> I find the use of transform a bit confusing. I would say PRF. (and "most preferred" -> preferred)

OK. How about:

              The Responder parses the received SA payload and finds a
              mutually supported PRFs. The
              Responder selects the preferred PRF from the list of 
              mutually supported ones and includes it into the PUZZLE notification.

>    If there are no mutually
>    supported PRFs, then negotiation will fail anyway and there is no
>    reason to return a puzzle.
> 
> I first thought of the AES_GCM not needing PRF, then realised I confused IKE and IPsec SA.
> Perhaps add change "negotiation will fail" to "IKE SA negotiation will fail".

OK.

> 7.1.1.3.  Generating Cookie
> 
>    If Responder supports puzzles then cookie should be computed in such
>    a manner, that the Responder is able to learn some important
>    information from the sole cookie,
> 
> We are in the middle of puzzles and not cookies. Why suddenly cookies?

Because puzzles closely linked with cookies :-)

> Again, I think the document can use a better introduction in section 7
> that explains the interaction between the two, laying out the principes.


Again, section 4.4 introduces puzzles on a higher level.

> Only later on do I read responder puzzle state is encoded in the cookie.
> The point of encoding puzzle information in the cookie is presumbly so
> that this state does not need to be remembered by the responder. So how
> does it know "The number of consecutive puzzles given to the Initiator."?
> Is this a counter in the <AdditionalInfo> ?

Yes. However, it is a local matter how responder generated cookie.
He could use alternative methods, the method described in this section 
is only an example.

> I would like to put a note here as well about the maximum cookie size of 64
> bytes that implementors might not realise. to avoid naive implementations
> building a very big "string cookie" with these bullet points of information.

OK.

> Cookie = <VersionIDofSecret> | <AdditionalInfo> |
>                      Hash(Ni | IPi | SPIi | <AdditionalInfo> | <secret>)
> 
> This would give the responder the AdditionalInfo information which it might

Attacker?

> use as feedback on how successful the attack is. Why not use an encryption
> of <AdditionalInfo> using the <secret> ? Would that be too expensive for
> the Responder? I'd think not as it is not more expensive than decrypting
> IKE_AUTH.

Sure, it is possible. The above is just an example.
However, I don't think an attacker learns much from
AdditionalInfo. All the information from there he must already know
(of course unless the responder puts there something really sensitive).

> Or stick with the second approach listed, where the responder keeps this
> state locally, which probably is better anyway because it needs to know
> the scale of the entire attack that cannot be learned from individual
> negotiation states.

Both approaches (and also some combinations of them) are possible.

> 7.1.2 states if the inititor does not want to solve a puzzle of difficulty X,
> it will pretend not to support the NOTIFY. This causes the responder to not
> learn that the initiator rejected the difficulty versus that it just does not
> support puzzles. It would be useful for the responder to know how many iniators
> support puzzles, so I would recommend a different NOTIFY for the "puzzle too
> difficult" error path (Maybe a return notify of PUZZLED :)

Hmmm... What the responder shoud do with this notify?
Just perform some completely unreliable statistical measurments (for what purpose)?

I think that better approach would be for such initiators that want to 
participate in such polls just solve puzzle with say 1 bit difficulty.
It'll cost them always nothing and will read to the same results for responder: 
serve such initiators with the lowest priority (as if they don't support
puzzles). But again, I don't understand what is the value of such polls.

>    If the received message contains a PUZZLE notification and doesn't
>    contain a COOKIE notification, then this message is malformed because
>    it requests to solve the puzzle, but doesn't provide enough
>    information to do it.
> 
> Again, conflicting with earlier text saying cookies are not mandated for puzzles,
> which now it seems they are.

Sorry, I don't see where the conflict is. Cookie mechanism is unchanged from RFC 7296.
Puzzles mechanism is used only with cookie mechanism because cookie content
is used while solving puzzle. 

> It seems 7.1.4 paragraph 2 and 3 are better moved to the introduction of that
> section.

I think it will make reading the section more difficult.

Why do you think is is needed?

>    If a PS payload is found in the message, then the Responder MUST
>    verify the puzzle solution that it contains.
> 
> Doesn't that open up the responder to a DDOS attack. Initiators will just
> submit fake puzzle solutions to drive up the initiator CPU.

Every defense mechanism has its limits. If attacker can exhaust your CPU
by sending fake puzzle solutions, then you loose. 

> Also, if the responder is no longer under attack, why can't it just ignore
> the puzzle solution and continue with regular IKE?

I see your point. However the puzzles must be easy to check, especially if the responder 
is not under attack.

Are you suggesting to change this MUST to SHOULD?
Then what are situations when responder can skip puzzles validation?

>    if the Responder didn't indicate any
>    particular difficulty level (by setting ZBC to zero) and the
>    Initiator was free to select any difficulty level it can afford,
> 
> Woah, these options were not discussed before at all. So that's what level 0 means!
> I would really move this text to the start of section 7 in the introduction of how
> puzzles work in general.

These options are described in 7.1.1.1

>    o  Demand more work from Initiator by giving it a new puzzle.
> 
> This seems a waste of a round trip. Why can the responder demand a variable puzzle
> without telling what would make it happy, only to have the initiator misguess and
> cause another roundtrip, or to avoid a potential roundtrip, waste too much resources
> and cause visible delays to endusers by overestimating puzzle difficulty? I think
> this is not a good feature of the protocol.

It is the responder's local policy whether to demand more work (by requesting
new puzzle) or not. 

>    The more puzzles the Initiator solves the higher its chances are to be served
> 
> That seems bad. Each puzzle is a delaying round-trip!

But each new puzzle proves that the initiator spends more effort,
so its chances to be served raise with each round trip.
I don't think where will be more than 2-3 of them.
And in many cases solving more difficult puzzle would 
delay connection more than solving a few of less
difficult ones, even with additional round trips.

>    includes a puzzle
>    solution in the first IKE_AUTH request message outside the Encrypted
>    payload
> 
> Note this is very exceptional and should probably be written out in our syntax, eg:
> 
> HDR, SK {IDi, [CERT,] [CERTREQ,]
>        [IDr,] AUTH, SAi2,
>        TSi, TSr} [PUZZLE] -->

Your picture is wrong. SK payload is always the last payload in the message
(according to RFC 7296), so PS payload can only be placed before it.
And it is not optional in case the puzzle mechanism is used.

What's wrong for you with the picture from the draft?

   HDR, PS, SK {IDi, [CERT,] [CERTREQ,]
               [IDr,] AUTH, SA, TSi, TSr}   -->

> While I checked RFC 7296 it does not state some exchanges only have encrypted payloads,
> and so technically this document does not update the core document, but I think many
> implementations might not expect unencrypted payloads in IKE_AUTH and CREATE_CHILD_SA
> so perhaps that is important enough to mark this document as "updating 7296".

Please, see above (the beginning of this message).

>    in the IKE_AUTH exchange S is a concatenation of Nr and SPIr.
> 
> Why Nr and SPIr? I think because of re-using DH vales on the responder? If so, it would
> be good to explain that.

No, it has nothing common with reusing DH secrets.
We just need fresh some unpredictable for an Initiator data for solving
IKE_AUTH puzzles, so Nr and SPIr are natural choise.

> Should it state somewhere that IKE_AUTH puzzles are not allowed unless the clinet
> confirmed support for puzzles in IKE_INIT. Because then the responder does not actually
> know whetrher the initiator supports puzzles at all. And since it is stated those IKE_AUTH
> responses without puzzle should be silently dropped, that becomes very important.

Section 7.2.1 says:

   The Responder MUST NOT use puzzles in the IKE_AUTH exchange unless a
   puzzle has been previously presented and solved in the preceding
   IKE_SA_INIT exchange.

Isn't it not enough?

> I think the IKE fragmentation paragraph deserves to be in its own sub-section. It's pretty
> important to get it right and not start attempts to decrypt potentially bogus fragments.

I'm very reluctant to do so, since IKE fragmentation nuances are present
in severalsections. I think it's better to have them "in place" rather than
keep together out of context. Another option is to make IKE Fragmentation 
dedicated subsection in every relevant section, but it's usually only a para
and somebody already complained on the large number of subsections in Section 7.
So, unless you think it is absolutely necessary, I'll refrain from doing that.

> Regarding the puzzle format, this is now 11 octects. we've aligned things in the past, so
> should we do that hear and add another 1 octet of reserved bits? The puzzle notification

I'd rather make it compact. We already have payloads and notifications with odd length 
even in the core spec (like CERREQ or IPCOMP_SUPPORTED) and there is no
any requirement in IKEv2 that payloads are aligned, so I don't see any reason for wasting
an extra byte in the precious IKE_SA_INIT spae (remember IP fragmentation!).

> also misses an IANA line:    The payload type for the Puzzle payload is <TBA by IANA>.

It is in the description of the corresponding field:

   o  Notify Message Type (2 octets) -- MUST be <TBA by IANA>, the value
      assigned for the PUZZLE notification.

> Section 9 states: A good rule of thumb is for taking about 1 second to solve the puzzle.
> 
> Why is this a good rule of thumb? Again this comes down to me having no idea whether or

There are some explanations below.

> not puzzles is a good idea to begin with. I'm very skeptical of this claim. A botnet
> will be able to waste 1s of 99% CPU on this attack per node.

Then the difficulty level will increase.

>    Initiators should set a maximum difficulty level beyond which they
>    won't try to solve the puzzle and log or display a failure message to
>    the administrator or user.
> 
> and again note that this information will never be available to the Responder, so it
> can never figure out what solving levels is causing a sharp drop in legitimate clients.
> (other than seeing an attack that is more successful, but it won't know that this is
> caused by the puzzle difficulty level)

Do you have better solution?

>    that the Responder's load remain close to the maximum it can tolerate.
> 
> Which ignores the pain on initiators. it should probably be pointed out somewhere
> that confirmation a puzzle solution is a lot cheaper than solving the puzzle.

The text means that the difficulty level must not be too high even under attack.
So, I don't understand how it ignores the pain on initiators. Instead, the responder
is advised to keep difficulty level as low as possible, only to keep itself
operational even under attack, so that it can still serve legitimate initiators.

> nits

Accepted most of them, exceptions described below.

> Note this draft also mentions IKEv2 with version instead of
> refering to just IKE, which makes more sense if we end up
> with IKEv3.
> 
> buggy implementation -> broken implementation
> 
> escape any responsibility for its actions ->
> escape any responsibility for its bad behaviour
> 
> Since currently there is no way -> Since there is currently no way
> 
> In IKEv2 client can request various configuration attributes ->
> In IKE, a client can request various configuration attributes
> 
> Most often those attributes  -> Most often these attributes
> 
> for defeating the DDoS attack -> for surviving DDoS attacks.
> 
> Implementations may be deployed in different environments ->
>  Implementations are deployed in different environments
> 
> As an example -> For example
> 
> , searching for two things: -> for two scenarios:
> 
> Supposing the the tunnels  -> Supposing that the tunnels
> 
> If they are mitigated well enough -> If these are mitigated successfully
> 
> 
>    This is a good
>    thing as it prevents Initiators that are not close to the attackers
>    from being affected.
> 
> I think this sentence adds nothing and can be removed.
> 
> This will force the attacker to use real source addresses,  ->
> This will mitigate attacks based on IP address spoofing
> 
> I would probably shorten the introduction of section 7 to something like:
> 
>  Puzzles can be added in the IKE_INIT and IKE_AUTH ecchanges.
> 
> and leave out the text describing the document flow, like "Both sections are divided into ..."

Well, that part was added by Dave's request as a short descrition
of Section 7 subsections.

>  The message may optionally contain a COOKIE notification
> 
> If implementations base themselves on this draft, it is actually basically
> guaranteed to have a cookie, say "may optionally" seems a bit weak?
> "most likely" or "usually" seems better.
>
> I mean, this part of the document should assume previous parts of the document
> are implemented.

This section describes how puzzles are handled by the responder,
including situations with non-supporting initiators, in which cases
COOKIE is really optional/ So, left as is.

> To support this feature -> To support crypto agility
> 
> also MAY ignore -> MAY ignore
> 
> ready to deal with them, -> ready to solve them"

Thank you,
Valery.