That should say [email protected]. > On Mar 23, 2016, at 1:21 PM, Alissa Cooper <[email protected]> wrote: > > 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
_______________________________________________ P2PSIP mailing list [email protected] https://www.ietf.org/mailman/listinfo/p2psip
