> Yoav Nir writes:>> Following is our suggested new text. Please let us know what you>> think. Also, please take a look at the description of>> "AUTHENTICATION_FAILED" in section 3.10.1. "response to an IKE_AUTH>> message" means either an IKE_AUTH response to an IKE_AUTH request, or>> an INFORMATIONAL request that describes an error in the IKE_AUTH>> response. Do you think this phrasing is clear enough?>> Yes, altought I think most of the implementations do not bother> sending INFORMATIONAL requests when IKE_AUTH response has errors. I> think most implementations will then simply remove the IKE SA as> failed without any further communications to the other end

But wouldn't this cause a state de-synchronization? If the responder
sends a revoked certificate, the initiator will have no IKE SA, but
the responder would have an IKE SA, which it thinks is valid. Wouldn't
this necessarily later lead to requests that time out?

> (I do not> know any implementation sending that kind of INFORMATIONAL requests,> but I expect that almost all implementations will act correctly if> someone happen to send them (i.e. they will delete the IKE SA as> failed and send empty reply back)).>>> All errors that occur in an IKE_AUTH exchange, causing the>> authentication to fail for whatever reason (invalid shared secret,>> unrecognized ID, untrusted certificate issuer, revoked or expired>> certificate, etc.) MUST result in an AUTHENTICATION_FAILED>> notification.>> This MUST there is too strong, especially for the "unrecognized ID"> part. For example our implementation send AUTHENTICATION_FAILED return> only if processing of AUTH Payload faiiled (i.e. signature check> failed (for RSA/DSA), mac failed for shared keys authentication, or no> valid public key / shared key found for the ID).>> In cases where the other end is unknown (i.e. ID is not configured to> the policy) it will return NO_PROPOSAL_CHOSEN as it does not find> valid policy to accept the other ends proposal when looking it based> on the ID.

That allows for ID enumeration. It's similar to a password entry form,
that says "wrong user" or "wrong password". An attacker would be able
to verify whether a particular ID (say, user name) exists on a system.

> So at least remove the "unrecognized ID" from the list, as it does not> belong there, and change the "MUST" to "SHOULD" as specifying exactly> one error code in such situations will make lots of implementations> non-conforming. I know that people writing conformance tests are going> to test this kind of things, and I have already several times> explained them that the exact error codes returned by different> implementations can differ depending what checks they do and in which> order.>>> If the error occurred on the responder, the>> notification MUST be returned in the protected response, and >> MUST be>> the only payload in that response.>> Again the two MUSTs there will make some implementations> non-conforming. We had discussion about this earlier, and in general> it is good idea to send them encrypted and MACed, but as that was not> required before this is real protocol change. For such I think it> requires more explantion. Also I want to have warning here saying that> even if it is encrypted and MACed, that does not mean it is authentic> from the real recipient. It is coming from the recipient you talked> to, but if there is man-in-the-middle he can also generate such> messages, meaning the initiator should still continue trying with this> peer (he can immediately delete the current IKE SA exchange, as it> cannot succeed after this, but next try might get to the real end node> instead of man-in-the-middle and succeed).

Agree. Will fix tomorrow or Thursday.

>> If the error occurs on the>> initiator, the notification MUST be returned in a separate>> INFORMATIONAL exchange, with no other payloads.>> This MUST there is way too strong, as there is no implementations I> know that implements that. For example our implementation simply> considers the IKE SA failed in such case and removes the IKE SA, and> then when next triggering packet comes it will try again.>> I would say "MAY" would be correct in this case.

OK.

>> Note, however, that>> messages that contain an unsupported critical payload, or that are>> otherwise malformed, MUST be rejected in their entirety, and only>> lead to an UNSUPPORTED_CRITICAL_PAYLOAD or INVALID_SYNTAX>> Notification. The receiver MUST NOT verify the payloads related >> to>> authentication in this case.>> This "MUST NOT" is again too strong, as in some implementations they> might process payloads in order and depending how deep in which place,> they might for example process AUTH payload before noticing that the> SA payload of the IPsec SA is malformed in its transform substructure.>> This "MOST NOT" would make such implementations non-conforming,> meaning every single implementation must do full parsing of the> payloads first and only after that do second phase when it processes> the payloads.

How about if we limit this to packets that are malformed in their
entirety, rather than some particular payload (and packets that have
unrecognized critical payloads) ?

>> If authentication has succeeded in the IKE_AUTH exchange, the >> IKE SA>> is established, provided that there are no unsupported critical>> payloads.>> The "provide that there are no unsupported critical payloads" is> not really needed, as the first part asked to check those first, thus> authentication cannot succeed (it is not even tried) if there is> unsupported critical payload. Even if we allow parsing AUTH payloads> before checking all critical bits, then I do not think that text is> needed, it just confuses people.

OK

> This next text should be in its own paragraph as it is not related to> the authentication failures:>>> Establishing the child SA, or requesting configuration>> information may still fail, but they do not automatically cause >> the>> IKE SA to be deleted. Specifically, a responder may include all >> the>> payloads associated with authentication (IDr, Cert and AUTH) while>> sending error notifications for the piggybacked exchanges>> (FAILED_CP_REQUIRED, INVALID_SELECTORS, NO_PROPOSAL_CHOSEN, etc.),>> and the initiator MUST NOT fail the authentication because of >> this.>> The initiator MAY, of course, for reasons of policy later delete>> such>> an IKE SA.>>>> Only authentication failures and malformed packets lead to a>> deletion>> of the IKE SA without requiring an explicit DELETE payload. Other>> error conditions require such a payload.>> It would be better to talk about "INFORMATIONAL exchange having DELETE> payload for IKE SA" instead of "DELETE payload", as that is the common> way of doing things. Otherwise some implementation might start adding> DELETE payloads to other requests / replies too, and I don't expect> all imlementations to act on them in all cases.>> Currently we do not have any limitiations where DELETE payload can be,> but I would expect most implemenations would ignore it if it is sent> inside the response packet of the CREATE_CHILD_SA exchange.>> Even if we do not forbid DELETE payloads in other places, we can use> examples and language that indicate the "normal" place for it.

OK

> This following covering IKE_SA_INIT should again be in separate> paragraph:>>> In an IKE_SA_INIT>> exchange,>> any error notification causes the exchange to fail.>> Not completely true. Even when the exchange fails the creation of the> IKE SA might still continue, for example in cases where> INVALID_MAJOR_VERSION (the initiator can fall back to IKEv1),> INVALID_KE_PAYLOAD (the initiator can change to correct group), or> COOKIE (the initiator tries again with COOKIE payload included (COOKIE> is not really error notification)).>> Currently exchange is defined so that this statement is true as in> those cases we start new exchange with same peer.>>> In an IKE_AUTH>> exchange, or in the subsequent INFORMATIONAL exchnage, only the>> following notifications cause the IKE SA to be deleted or not>> created, without a DELETE payload:>> o UNSUPPORTED_CRITICAL_PAYLOAD>> o INVALID_SYNTAX>> o AUTHENTICATION_FAILED>>>> Extension documents may define new error notifications with these>> semantics, but MUST NOT use them unless the peer is known to>> understand them.>> In subsequent INFORMATIONAL exchanges the UNSUPPORTED_CRITICAL_PAYLOAD> should not be fatal. It only means that the responder ignored the> whole message and replied with UNSUPPORTED_CRITICAL_PAYLOAD. That does> not delete IKE SA.>> For the IKE_AUTH the UNSUPPORTED_CRITICAL_PAYLOAD can delete the IKE> SA as IKE SA is not yet ready.