[P2PSIP] AD evaluation: draft-ietf-p2psip-share-07

[Alissa Cooper]

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.


== 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)?

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. 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?

= 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.

= 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

(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)?

(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?

(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.”

= 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.

= Section 6.3 =

How strings are to be compared (e.g., as binary objects or whatever it is) needs to be normatively specified.

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.

= 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? 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?).

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?

= 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.

= 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?

= Section 9.2 =

What is the significance of 17, other than that it is in the unassigned range?


== 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.

s/from one authorized to another (previously unauthorized) user/from one authorized user to another (previously unauthorized) user/

= 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>]/

= Section 3.1 =

s/Append an 8 bit long short individual index value/Append an 8-bit individual index value/

= Section 4.1 =

s/an Access Control including/an Access Control List including/

= 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.

[Thomas C. Schmidt]

Dear Alissa,

many thanks for your detailed feedback. We'll address the comments
shortly and be back.

Best,
Thomas

Post by Alissa Cooper
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.
== 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)?
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. 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?
= 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.
= 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
(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)?
(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?
(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
""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.”
= 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.
= Section 6.3 =
How strings are to be compared (e.g., as binary objects or whatever it
is) needs to be normatively specified.
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.
= 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? 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?).
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?
= 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.
= 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?
= Section 9.2 =
What is the significance of 17, other than that it is in the unassigned
range?
== 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.
s/from one authorized to another (previously unauthorized) user/from one
authorized user to another (previously unauthorized) user/
= 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>]/
= Section 3.1 =
s/Append an 8 bit long short individual index value/Append an 8-bit
individual index value/
= Section 4.1 =
s/an Access Control including/an Access Control List including/
= 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.

[Alissa Cooper]

Hi Thomas,

What is the status of this?

Thanks,
Alissa

Post by Thomas C. Schmidt
Dear Alissa,
many thanks for your detailed feedback. We'll address the comments shortly and be back.
Best,
Thomas

Post by Alissa Cooper
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.
== 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)?
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. 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?
= 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.
= 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
(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)?
(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?
(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
""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.”
= 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.
= Section 6.3 =
How strings are to be compared (e.g., as binary objects or whatever it
is) needs to be normatively specified.
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.
= 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? 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?).
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?
= 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.
= 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?
= Section 9.2 =
What is the significance of 17, other than that it is in the unassigned
range?
== 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.
s/from one authorized to another (previously unauthorized) user/from one
authorized user to another (previously unauthorized) user/
= 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>]/
= Section 3.1 =
s/Append an 8 bit long short individual index value/Append an 8-bit
individual index value/
= Section 4.1 =
s/an Access Control including/an Access Control List including/
= 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.

--
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: [phone] °
° http://www.informatik.haw-hamburg.de/~schmidt Fax: [phone] °

[Thomas C. Schmidt]

Hi Alissa,

I'll be back with ShaRe in a few days.

Best,
Thomas

Post by Alissa Cooper
Hi Thomas,
What is the status of this?
Thanks,
Alissa

Post by Thomas C. Schmidt
Dear Alissa,
many thanks for your detailed feedback. We'll address the comments shortly and be back.
Best,
Thomas

Post by Alissa Cooper
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.
== 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)?
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. 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?
= 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.
= 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
(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)?
(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?
(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
""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.”
= 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.
= Section 6.3 =
How strings are to be compared (e.g., as binary objects or whatever it
is) needs to be normatively specified.
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.
= 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? 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?).
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?
= 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.
= 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?
= Section 9.2 =
What is the significance of 17, other than that it is in the unassigned
range?
== 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.
s/from one authorized to another (previously unauthorized) user/from one
authorized user to another (previously unauthorized) user/
= 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>]/
= Section 3.1 =
s/Append an 8 bit long short individual index value/Append an 8-bit
individual index value/
= Section 4.1 =
s/an Access Control including/an Access Control List including/
= 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.

--
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: [phone] °
° http://www.informatik.haw-hamburg.de/~schmidt Fax: [phone] °

[Thomas C. Schmidt]

Hi Alissa,

many thanks for the careful review. We have revised and resubmitted the
document accordingly.

For the details, please see our comments inline.

Post by Alissa Cooper
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?

Post by Alissa Cooper
== 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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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."

Post by Alissa Cooper
(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."

Post by Alissa Cooper
(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.

Post by Alissa Cooper
(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
""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.

Post by Alissa Cooper
= 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."

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
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."

Post by Alissa Cooper
= 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"

Post by Alissa Cooper
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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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'.

Post by Alissa Cooper
== 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.

Post by Alissa Cooper
s/from one authorized to another (previously unauthorized) user/from one
authorized user to another (previously unauthorized) user/

Thanks, fixed.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= Section 3.1 =
s/Append an 8 bit long short individual index value/Append an 8-bit
individual index value/

O.K., done.

Post by Alissa Cooper
= Section 4.1 =
s/an Access Control including/an Access Control List including/

Thanks, done.

Post by Alissa Cooper
= 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

[Alissa Cooper]

Thanks, the changes look good and the explanations make sense. Marc should go ahead and request the early secdir review by sending mail to ***@ietf.org.

Alissa

Post by Thomas C. Schmidt
Hi Alissa,
many thanks for the careful review. We have revised and resubmitted the document accordingly.
For the details, please see our comments inline.

Post by Alissa Cooper
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?

Post by Alissa Cooper
== 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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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."

Post by Alissa Cooper
(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."

Post by Alissa Cooper
(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.

Post by Alissa Cooper
(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
""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.

Post by Alissa Cooper
= 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).
"To protect the privacy of the users, the Resource Owner SHOULD overwrite all subtrees that have been invalidated."

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
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.

"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."

Post by Alissa Cooper
= 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?

"contains a username that matches to the user and domain portion in one of the variable resource name patterns"

Post by Alissa Cooper
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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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'.

Post by Alissa Cooper
== 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.

Post by Alissa Cooper
s/from one authorized to another (previously unauthorized) user/from one
authorized user to another (previously unauthorized) user/

Thanks, fixed.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= Section 3.1 =
s/Append an 8 bit long short individual index value/Append an 8-bit
individual index value/

O.K., done.

Post by Alissa Cooper
= Section 4.1 =
s/an Access Control including/an Access Control List including/

Thanks, done.

Post by Alissa Cooper
= 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: [phone] °
° http://www.informatik.haw-hamburg.de/~schmidt Fax: [phone] °

[Alissa Cooper]

Post by Thomas C. Schmidt
Alissa

Post by Thomas C. Schmidt
Hi Alissa,
many thanks for the careful review. We have revised and resubmitted the document accordingly.
For the details, please see our comments inline.

Post by Alissa Cooper
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?

Post by Alissa Cooper
== 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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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."

Post by Alissa Cooper
(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."

Post by Alissa Cooper
(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.

Post by Alissa Cooper
(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
""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.

Post by Alissa Cooper
= 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).
"To protect the privacy of the users, the Resource Owner SHOULD overwrite all subtrees that have been invalidated."

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
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.

"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."

Post by Alissa Cooper
= 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?

"contains a username that matches to the user and domain portion in one of the variable resource name patterns"

Post by Alissa Cooper
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.

Post by Alissa Cooper
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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= 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'.

Post by Alissa Cooper
== 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.

Post by Alissa Cooper
s/from one authorized to another (previously unauthorized) user/from one
authorized user to another (previously unauthorized) user/

Thanks, fixed.

Post by Alissa Cooper
= 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.

Post by Alissa Cooper
= Section 3.1 =
s/Append an 8 bit long short individual index value/Append an 8-bit
individual index value/

O.K., done.

Post by Alissa Cooper
= Section 4.1 =
s/an Access Control including/an Access Control List including/

Thanks, done.

Post by Alissa Cooper
= 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: [phone] °
° http://www.informatik.haw-hamburg.de/~schmidt Fax: [phone] °

_______________________________________________
P2PSIP mailing list
https://www.ietf.org/mailman/listinfo/p2psip