Thanks, the changes look good and the explanations make sense. Marc should go ahead and request the early secdir review by sending mail to [email protected].
Alissa > On Mar 20, 2016, at 4:38 PM, Thomas C. Schmidt <[email protected]> > wrote: > > Hi Alissa, > > many thanks for the careful review. We have revised and resubmitted the > document accordingly. > > For the details, please see our comments inline. > > On 15.01.2016 00:03, Alissa Cooper wrote: >> I have reviewed this document in preparation for IETF last call. I have >> a number of comments and questions that need to be resolved before last >> call can be initiated. I’ve also included some nits below that should be >> resolved together with last call comments. >> >> Given the nature of this document, I’d like for the shepherd to request >> an early SECDIR review after the comments below have been resolved so >> that the authors and WG can receive security feedback before the >> document progresses to IESG evaluation. >> > > I assume this needs to be initiated by Marc (Petit-Huguenin) right away? > >> >> == Substantive comments and questions == >> >> = Section 3.1 = >> >> I think this section requires clarification. >> >> How is the index value supposed to be initialized? Is it supposed to be >> chosen at random or set to 0 (or 1, as in the figure)? >> > > It is now clarified that the (8 bit individual) index is under sole control > of the writing peer. The peer is free to use these bits according to > application needs. No interoperability issue, as the key only requires > uniqueness, no further semantic attached to it. > >> I don’t understand how this mechanism relates to how SSRCs are chosen. >> In fact RFC 3550 doesn’t specify a particular algorithm to use, but >> merely provides one example. > > The mechanism to build the keys is now more explicitly stated. The pointer to > RFC 3550 refers only to a method for calculating the collision probability, > no reference to assignment algorithms. > >> Furthermore, I don’t see how the collision >> probably for the array index value, which selects the least significant >> three bytes from a cryptographically random Node-Id that must be 16 >> bytes or longer, would be the same as for a randomly chosen 32-bit >> integer. Could you explain? >> > > The formula presented in RFC 3550 has a length parameter (L) and we added > that L=24 must be chosen in the present case. Otherwise, the argument is that > the selected 24 bits from a (cryptographically random) Node-Id also form > uniform pseudo-random numbers, since the selection mechanism does not produce > a bias. > >> = Section 5 = >> >> Are variable resource names expected to be UTF-8 strings? I think >> somewhere in this section the internationalization expectations for >> these strings need to be specified. >> > > Resource names correspond to regular Reload resource names, thus opaque > (ASCII-encoded) strings of variable length up to 254 bytes. We are not > touching this here. > >> = Section 5.3 = >> >> (1) >> I think this section needs to specify normative requirements on the >> pattern construction to avoid duplicative or substring names as >> described in 5.1 >> > > Yes, we've included > > "variable parts in <pattern> elements defined in the overlay configuration > document MUST remain syntactically separated from the user name part (e.g., > by a dedicated delimiter) to prevent collisions with other names of other > users." > >> (2) >> "Configurations in this overlay document MUST adhere in syntax and >> semantic of names as defined by the context of use. For example, syntax >> restrictions apply when using P2PSIP[I-D.ietf-p2psip-sip], while a more >> general naming is feasible in plain RELOAD." >> >> I don’t understand what the normative requirement is here or why it is >> needed. How is “the context of use” defined? Shouldn’t it be up to the >> specific protocol documents to define the required syntax and semantics >> for specific usages (e.g., the way draft-ietf-p2psip-sip does)? >> > > Right, this was misleading. We changed to > > "It is noteworthy that additional constraints on the syntax and semantic of > names can apply according to specific Usages." > > >> (3) >> "In the absence of a <variable-resource-names> element for a Kind using >> the USER-CHAIN-ACL access policy (see Section 6.6 >> <https://tools.ietf.org/html/draft-ietf-p2psip-share-07#section-6.6>), >> implementors SHOULD assume this default value." >> >> Why is this SHOULD and not MUST? Shouldn’t implementations >> conservatively assume that variable names are not allowed unless >> explicitly specified? >> > > We agree - changed to MUST. > >> (4) >> "If no pattern is defined for a Kind or the "enable" attribute is false, >> allowable Resource Names are restricted to the username of the signer >> for Shared Resource.” >> >> I think this needs to account for an error condition where the pattern >> does not meet the pattern construction requirements, e.g.: >> >> ""If no pattern is defined for a Kind, if the "enable" attribute is >> false, or if the regular expression does not meet the requirements >> specified in this section, the allowable Resource Names are restricted >> to the username of the signer for Shared Resource.” >> > > Thanks, we forgot this - changed now. > >> = Section 6.2 = >> >> For privacy reasons, wouldn’t it be better to overwrite every entry in a >> subtree when the root of the subtree gets overwritten? Otherwise the >> list of users who were given write access may remain long after their >> access has been revoked. >> > > Yes in general, but there are two constraints. > > (a) Only the Resource Owner can overwrite all entries of a subtree (entries > of which may have different owners). Periodic checks of the Owner may produce > an unwanted extra load. > (b) There may be use cases, where a subtree shall be temporarily invalidated, > but reactivated later (e.g., some behavioral monitoring puts some peer under > suspect, but later releases this). > > For these reasons, we argue for a "SHOULD" here: > > "To protect the privacy of the users, the Resource Owner SHOULD overwrite > all subtrees that have been invalidated." > >> = Section 6.3 = >> >> How strings are to be compared (e.g., as binary objects or whatever it >> is) needs to be normatively specified. >> > > Yes, comparing binary objects is added, now. > >> It is confusing to use normative language only in step 5 here. I would >> suggest either normatively defining each action or not using SHALL here. >> > > Yes, agreed and changed to: > > "This final ACL item is expected to be the root item of this ACL which MUST > be further validated by verifying that the root item was signed by the owner > of the ACL Resource." > >> = Section 6.6 = >> >> "Otherwise, the value MUST be written if the certificate of the signer >> contains a username that matches to one of the variable resource name >> pattern (c.f. Section 5 >> <https://tools.ietf.org/html/draft-ietf-p2psip-share-07#section-5>) >> specified in the configuration document" >> >> It seems to me that matching the pattern is not sufficient — isn’t it >> the case that both the user and domain portions of the user name in the >> certificate need to match the user and domain name portions present in >> the resource name? > > Yes, this is made clearer now: > > "contains a username that matches to the user and domain portion in one of > the variable resource name patterns" > >> In general, the document seems to be missing >> discussion of the implications of having the user name and the resource >> name diverge. I think this affects every operation that involves >> comparing the two (or the Resource-Id, right?). >> > > The logic is as follows: Store if (a) a regular USER-MATCH or USER-NODE-MATCH > works (we are regular owner), or if (b) a variable resource name is > successfully authenticated (we are owner using variable naming - this > includes matching of username *and* resource name), or if (c) ACL > authorization is in effect (we harvest trust delegation) for this resource. > > Otherwise, the store request must be denied, which we added explicitly, now. > >> I’m also unclear about why policy for allowing access to shared >> resources is being strictly coupled with policy for allowing variable >> resource names. Might there be cases where it makes sense to authorize >> one but not the other? >> > > I don't see the strict coupling, here: First the check is done to store based > on USER-MATCH or USER-NODE-MATCH, which do not work with variable resource > names, then ... It is rather that USER-CHAIN-ACL grants storing rights in > both cases, variable naming and trust delegation. This makes IMO sense, as it > abstracts from the strict naming authorities of USER-MATCH or USER-NODE-MATCH. > >> = Section 8.2 = >> >> This section misses the threat of a misbehaving peer who is delegated >> write access — that seems like an important case to cover. >> > > O.K., we added a subsection on that. > >> = Section 8.3 = >> >> By “publicly readable” do you mean “readable by any node in the >> overlay”? Admission to the overlay would still be access controlled, >> correct? >> > > Yes, of course. We clarified. > >> = Section 9.2 = >> >> What is the significance of 17, other than that it is in the unassigned >> range? >> > None, I guess it was just free. We changed to 'TBD'. > >> >> == Nits == >> >> = Section 1 = >> >> The reference to I-D.ietf-p2psip-disco should be removed given that the >> document is several years old and not expected to advance as far as I know. >> > > Yup, this document died with the working group. > >> s/from one authorized to another (previously unauthorized) user/from one >> authorized user to another (previously unauthorized) user/ >> > Thanks, fixed. > >> = Section 2 = >> >> s/the peer-to-peer SIP concepts draft [I-D.ietf-p2psip-concepts >> <https://tools.ietf.org/html/draft-ietf-p2psip-share-07#ref-I-D.ietf-p2psip-concepts>]/[I-D.ietf-p2psip-concepts >> <https://tools.ietf.org/html/draft-ietf-p2psip-share-07#ref-I-D.ietf-p2psip-concepts>]/ >> > > Thanks, fixed. > >> = Section 3.1 = >> >> s/Append an 8 bit long short individual index value/Append an 8-bit >> individual index value/ >> > > O.K., done. > >> = Section 4.1 = >> >> s/an Access Control including/an Access Control List including/ >> > Thanks, done. > >> = Section 5.1 = >> >> Same comment about I-D.ietf-p2psip-disco >> <https://tools.ietf.org/html/draft-ietf-p2psip-share-07#ref-I-D.ietf-p2psip-disco> >> as >> in Section 1. >> > Done. > > Thanks, > Thomas > > -- > > Prof. Dr. Thomas C. Schmidt > ° Hamburg University of Applied Sciences Berliner Tor 7 ° > ° Dept. Informatik, Internet Technologies Group 20099 Hamburg, Germany ° > ° http://www.haw-hamburg.de/inet Fon: +49-40-42875-8452 ° > ° http://www.informatik.haw-hamburg.de/~schmidt Fax: +49-40-42875-8409 ° _______________________________________________ P2PSIP mailing list [email protected] https://www.ietf.org/mailman/listinfo/p2psip
