Hi Paul, thank you for this review.
> Sorry for the (very) late review. I support the document but have a few > comments and questions. > > The SUPPORTED_AUTH_METHODS NOTIFY is used for multiple purposes. One > of these methods (with no payload data) is used for two different things. > Would it be better to split this in two? eg > SUPPORTED_AUTH_METHODS_SUPPORTED and > SUPPORTED_AUTH_METHODS ? Actually, there is no ambiguity here. The first appearance of SUPPORTED_AUTH_METHODS anywhere means that this extension is supported by the party sent it. The content provides supported methods and if the content in responder's message is empty, then it means that it will be provided in subsequent exchange. It is possible to use two notifications types, but is there a real need for that? > The recommendations on in which IKE_INTERMEDIATE to send the payload is > a lot of handwaving. Maybe just say "in any IKE_INTERMEDIATE, but later > might give you more protection" Yes, I think this can be changed along these lines. > There is text about IDi/IDr payloads being used in IKE_INTERMEDIATE and > then talk about SHOULD be identical to the ones in IKE_AUTH. I would prefer a > different notify for this (eg SAM_IDi/SAM_IDr) to avoid implementers > confusing/erroring on confusing these with the real IDi/IDr payloads. Hm, I'm not sure this would help implementers. Currently they have some code that prepares IDi/IDr and they can re-use it for IKE_INTERMEDIATE. If these identities are to be put in notifies, then probably the amount of code re-use will be smaller, so more new code needs to be wtitte/tested etc. These payloads are optional in IKE_INTERMEDIATE, their presence may give the responder more information which authentication methods are applicable for this particular initiator. On the other hand, they may receive less protection, compared to IKE_AUTH, so the initiator may provide some other identities, than its real ones are, some kind of pseudonyms, which give enough information for selecting the proper authentication method, but still don't reveal real initiator's identity in case attacker can break IKE_NTERMEDIATE. > There are two methods described, one for old style RSA/ECDSA and one for > Digital Signatures (RFC 7247, auth method 14). I would prefer to not support > the old style, as we are trying to obsolete those. These use undesirable > features anyway (RSA v1.5, sha-1, etc) That's a fair point. But what if some new authentication method appears in the future, that will unambiguously identify the authentication algorithm (so no additional information like AlgorithmIdentifier is needed? I prefer to keep the format (anyway, it is mostly the same, only length matters), but state, that currently this format (Len = 3) is not used. > There are oddities with the CERTREQ payload. Some implementations using a > list of hashes in 1 CERTREQ, others using 1 hash per CERTREQ. This makes using > a numbered cert link number a bit tricky. (eg number 2 of the 3rd CERTREQ). The draft assumes that a single list of CAs from all CERTREQ payloads is formed, so, the link contains the position of the CA from this list. > I'd much rather select based on hash, not number. It is possible, but will consume 19 bytes more for each announcement. I also don't like that the information is duplicated in this case - both CERTREQ and SUPPORTED_AUTH_METHODS will contain the same hashes. It's better get rid of CERTREQ in this case, but this will break interoperability with unsupported implementations. > Additionally, implementations might build the CERTREQ payload and then > throw it away. I wouldn't want to need to keep those around for this > extension. My understanding, that when you receive a message, you start parsing it. Until you fully parse it, you unlikely throw it away. The protocol currently is constructed in such a way, that SUPPORTED_AUTH_METHOFS and CERTREQ are in the same message, except for one case - when the responder sends this information in IKE_INTERMEDIATE. It is possible to modify the protocol to duplicate CERTREQ in this message. Would it eliminate your concern? > With PAKE, can we say MUST NOT instead of SHOULD NOT, and throw a fatal > error? I tend to agree in general. But for some reason RFC 6467 exchange diagrams contain CERT and CERTREQ payloads, so, it makes me think that they can be used somehow and thus this extension can be useful in this case too 😊 Perhaps some combined authentication? But probably this is a bug in RFC 6467... Regards, Valery. > Paul > > _______________________________________________ > IPsec mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/ipsec _______________________________________________ IPsec mailing list [email protected] https://www.ietf.org/mailman/listinfo/ipsec
