Subject: [Stox] Very belated review of draft-ietf-stox-groupchat-07

From: Ben Campbell <[email-masked]>
Date: Tue, 4 Nov 2014 17:53:34 -0600

Hi,

First, let me say that groupchat-07 is a far better document than when I last reviewed it (I think that was 07.) This version cleans up most of the architectural confusion from the older version. It's so much more understandable, that I found some new (to me) issues ;-)  I apologize in advance if any of these have already been discussed and I missed it:

*** Major Issues:

-- Section 6: 

When Romeo (a SIP user) joins the capulet chatroom (an XMPP group chat), what prevents Romeo's client from bypassing his own SIP proxy and trying to directly find a SIP service at example.com? This is to some degree an RFC7247 issue, where we assume that a SIP server knows to send stuff through a SIP-XMPP gateway. The problem is, SIP doesn't actually require a UAC to route requests through it's own proxy.

Now, I recognize that in _practice_, the UAC will almost only route outbound requests through its own server for NAT traversal and/or authentication reasons(perhaps using the SIP outbound mechanism). But that's not a 3261 requirement. If this draft wants to assume that this will always happen, that's probably ok--but it should mention that explicitly, and perhaps mention that if SIP clients do not use outbound SIP services, this approach may not work. (Or alternately, the client itself needs to know to route things through gateways...)

Related Question: Is there a use case for when a SIP service does not have a gateway, and just sends SIP to a gateway at the XMPP domain? If so, that would not seem to need the assumption that a SIP client always uses an outbound proxy.

-- Also Section 6:

If I read things correctly, the actual joining of the MUC is a consequence of the SIP user subscribing to the conference event package. But that subscription is not normatively required for simple-chat implementations. The simple-chat draft only says that clients "typically" will do so. So unless you want to make a normative requirement for simple-chat clients to do so in order to work with stox-groupchat, you cannot depend on that subscription to trigger the presence interchange.

-- Section 9:

The draft says that it that it introduces no new security considerations. But there must be some assumptions about user identities, authentication and trust. These probably should have been in RFC7247, but I don't find them there on a quick scan.

*** Minor Issues:

-- general:

There's a fair amount of 2119 normative language that really repeats statements from the other protocols (SIP, MSRP, XMPP, etc.). Please consider avoiding 2119 language when talking about how one of those protocols works, unless you are creating some new requirement that doesn't already exist for that protocol.

-- section 2:

The draft lists MSPR (RFC 4975) as a groupchat related spec. It's really not; that should probably reference simple-chat. (It would make sense to add RFC4975 to the "We assume readers are familiar..." list.)

-- section 3 (and the flow diagrams)

You indicate that you will use the same diagram convention for SIP and MSRP traffic. I suggest you distinguish between the two; they are completely separate (if loosely related) protocols.

-- section 4, "... MSRP groupchat sessions are considered to be a type of media ..."

That's true for MSRP in _general_, not just for group chat.

-- Figure 1: I would not assume a direct app-specific connection or flow between the SIP proxy and the MSRP switch. It may exist, but it's equally valid that all such interactions would be intermediated by the focus.  (And it doesn't seem to be needed by anything else in this draft.)

-- Section 5:

It's worth mentioning that any SIP provisional responses have been left off for the sake of simplicity.

In F6, there should really be an MSRP SEND request that happens before the nickname request. RFC4975 requires an immediate SEND request by the active party. It can be empty if you aren't ready to send a real message yet.  (There might should be some guidance about not accidentally creating a privacy leak by sending a real message prior to setting a nickname.)

F28: There's work going on in SIPCORE to change the way REFER requests handle notifies (See draft-sparks-sipcore-refere-clarifications and draft-sparks-sipcore-refer-explicit-subscription). There probably don't need to block anything here, but it might be worth a footnote to say that, at the time of this writing, work is happening that might change this part of the flow.)

-- section 5.1, example 2:

It's unlikely that a SIP UAC is going to generate a gruu tag with a value as neat as "balcony". Unlike XMPP, SIP doesn't typically use human readable identifiers to distinguish devices (analogous to XMPP resources.) More likely that will be random string assigned by the registrar.  (It's not _wrong_ as stated, but it might mislead implementors.)

-- section 5.1, table 1:

Do I understand correctly that you could have more than one <thread/> element over the course of a MUC? For the SIP/MSRP user, the Call-ID will stay the same for the life of the session. Also, the call-id will be unique for each SIP participant; I assume that's not true for <thread/>.

-- Example 3:

The 200 has the From and To wrong. Responses do not swap the from and to from the original request. They should be identical to the original, with the possible exception of the "to" tag.  (I think this repeats throughout the draft.)

-- Example 8:

Inconsistent use of Allow header fields. I would probably leave them out except where required, but others may have other opinions.

-- section 5.4: first paragraph

First part of the paragraph seems to belong in 5.3. It might make sense to combined 5.3 and 5.4 into one section.

-- 5.5.2: Second Paragraph:

Maybe this paragraph should belong in the section about the original INVITE?

-- section 6, F42-44?

If the initial SEND in F42 was empty, would F44 and 45 still happen? Can F44 and F45 fail?

-- 6.1, paragraph before example 28:

What if the MSRP user never sets a nick at all?

6.3.1, last paragraph:

How do you determine "sameness" between stanzas in this context?

-- 7, 3rd paragraph: "In SIP, the nickname..."

s/SIP/" the SIP conference event package"

-- 7, 2nd to last paragraph:

I'm confused by this--I can see saying to display the contents of <display-text/> or the JID resource part if the respective xcon element or <nick/> element are not present, but this seems to suggest one should ignore them if they are present. Is that the intent?

---

From: Peter Saint-Andre - &yet <[email-masked]>
Date: Wed, 05 Nov 2014 20:38:52 -0700

Hi Ben,

Thank you very much for the in-depth feedback. I'll carve out some time 
soon to review and reply (probably next week).

Peter

---

From: Peter Saint-Andre - &yet <[email-masked]>
Date: Fri, 14 Nov 2014 19:20:28 -0700

Hi Ben, thanks for the review. Comments inline.

On 11/4/14, 4:53 PM, Ben Campbell wrote:
> Hi,
>
> First, let me say that groupchat-07 is a far better document than
> when I last reviewed it

I'm happy to hear it.

> (I think that was 07.) This version cleans up most of the
> architectural confusion from the older version. It's so much more
> understandable, that I found some new (to me) issues ;-)

It happens. :-)

> I apologize in advance if any of these have already been discussed
> and I missed it:
>
>
> *** Major Issues:
>
> -- Section 6:
>
> When Romeo (a SIP user) joins the capulet chatroom (an XMPP group
> chat), what prevents Romeo's client from bypassing his own SIP proxy
> and trying to directly find a SIP service at example.com? This is to
> some degree an RFC7247 issue, where we assume that a SIP server knows
> to send stuff through a SIP-XMPP gateway. The problem is, SIP doesn't
> actually require a UAC to route requests through it's own proxy.

Yes, I can see that. (This document clearly reflects the fact that I 
don't know SIP as well as I know XMPP.)

> Now, I recognize that in _practice_, the UAC will almost only route
> outbound requests through its own server for NAT traversal and/or
> authentication reasons(perhaps using the SIP outbound mechanism). But
> that's not a 3261 requirement. If this draft wants to assume that
> this will always happen, that's probably ok--but it should mention
> that explicitly,

I think it's really just a simplifying assumption for this document 
(e.g., it's not easy to illustrate every possible deployment 
architecture or communication path). But I agree with you that we need 
to make this explicit.

> and perhaps mention that if SIP clients do not use
> outbound SIP services, this approach may not work.

Good point.

> (Or alternately,
> the client itself needs to know to route things through gateways...)

That seems unlikely in practice, especially if an end user would need to 
configure the SIP UA with a gateway address.

> Related Question: Is there a use case for when a SIP service does not
> have a gateway, and just sends SIP to a gateway at the XMPP domain?

Again as a simplifying assumption, we've described the gateways as 
components that are associated with the sending domain (thus a SIP 
service would have a SIP-to-XMPP gateway for outbound traffic, and an 
XMPP service would have an XMPP-to-SIP gateway for outbound traffic). 
But nothing prevents services from having inbound gateways and in 
practice some services do host such gateways. As you say, this is really 
an RFC 7247 issue. (Sometimes I want to go back and revise both RFC 7247 
and RFC 7248!)

However, it does seem that we might want to make this crystal clear in 
this document.

> If so, that would not seem to need the assumption that a SIP client
> always uses an outbound proxy.

Right.

> -- Also Section 6:
>
> If I read things correctly, the actual joining of the MUC is a
> consequence of the SIP user subscribing to the conference event
> package. But that subscription is not normatively required for
> simple-chat implementations. The simple-chat draft only says that
> clients "typically" will do so. So unless you want to make a
> normative requirement for simple-chat clients to do so in order to
> work with stox-groupchat, you cannot depend on that subscription to
> trigger the presence interchange.

Hmm, this is a bit of a conundrum.

I had indeed noticed the "typically" language in simple-chat. And I 
certainly don't want to place a normative requirement on simple-chat 
implementations. On the other hand, in XMPP multi-user chat what the 
client does is best mapped to a subscription construct. The mapping 
document under consideration could be worded better, but the way I see 
it the XMPP-to-SIP gateway needs to send a conference event package 
subscription only if it wants to receive presence notifications back 
from the MSRP switch (Sections 5.3 & 5.4). And similarly in the other 
direction (Sections 6.1 & 6.2).

It's not clear to me how either side would be able to receive presence 
notifications without such a subscription, and those notifications are 
quite helpful in the context of a groupchat channel. But I don't think 
they are absolutely required.

> -- Section 9:
>
> The draft says that it that it introduces no new security
> considerations. But there must be some assumptions about user
> identities, authentication and trust. These probably should have been
> in RFC7247, but I don't find them there on a quick scan.

Would you mind expanding on those points a bit more deeply?

> *** Minor Issues:
>
> -- general:
>
> There's a fair amount of 2119 normative language that really repeats
> statements from the other protocols (SIP, MSRP, XMPP, etc.). Please
> consider avoiding 2119 language when talking about how one of those
> protocols works, unless you are creating some new requirement that
> doesn't already exist for that protocol.

Good point, we'll scrub the text in that regard.

> -- section 2:
>
> The draft lists MSPR (RFC 4975) as a groupchat related spec. It's
> really not; that should probably reference simple-chat. (It would
> make sense to add RFC4975 to the "We assume readers are familiar..."
> list.)

Agreed.

> You indicate that you will use the same diagram convention for SIP
> and MSRP traffic. I suggest you distinguish between the two; they are
> completely separate (if loosely related) protocols.

In the diagrams we use separate conventions:

       Legend:
           . = XMPP
           % = MSRP
           * = SIP
           & = app-specific

So we'll fix that text.

> That's true for MSRP in _general_, not just for group chat.

True. Will adjust.

> -- Figure 1: I would not assume a direct app-specific connection or
> flow between the SIP proxy and the MSRP switch. It may exist, but
> it's equally valid that all such interactions would be intermediated
> by the focus.  (And it doesn't seem to be needed by anything else in
> this draft.)

I suppose that by "app-specific" we meant that it's not standardized 
(any particular implementation could use whatever method it deems best).

> left off for the sake of simplicity.

Ah, I thought we'd added that. Will fix.

> In F6, there should really be an MSRP SEND request that happens
> before the nickname request. RFC4975 requires an immediate SEND
> request by the active party. It can be empty if you aren't ready to
> send a real message yet.  (There might should be some guidance about
> not accidentally creating a privacy leak by sending a real message
> prior to setting a nickname.)

Will fix.

> F28: There's work going on in SIPCORE to change the way REFER
> requests handle notifies (See
> draft-sparks-sipcore-refere-clarifications and
> draft-sparks-sipcore-refer-explicit-subscription). There probably
> don't need to block anything here, but it might be worth a footnote
> to say that, at the time of this writing, work is happening that
> might change this part of the flow.)

Noted.

> It's unlikely that a SIP UAC is going to generate a gruu tag with a
> value as neat as "balcony". Unlike XMPP, SIP doesn't typically use
> human readable identifiers to distinguish devices (analogous to XMPP
> resources.) More likely that will be random string assigned by the
> registrar.  (It's not _wrong_ as stated, but it might mislead
> implementors.)

In XMPP we've moved away from these human-readable identiers, too. Will 
adjust or at least add a note.

> -- section 5.1, table 1:
>
> Do I understand correctly that you could have more than one <thread/>
> element over the course of a MUC? For the SIP/MSRP user, the Call-ID
> will stay the same for the life of the session. Also, the call-id
> will be unique for each SIP participant; I assume that's not true for
> <thread/>.

In practice, the <thread/> element is not used heavily in XMPP 
groupchat, mostly because no one has ever figured out a good user 
experience for multiple threads in a conference. However, that doesn't 
change the fact that the current text is misleading. Will fix.

> -- Example 3:
>
> The 200 has the From and To wrong. Responses do not swap the from and
> to from the original request. They should be identical to the
> original, with the possible exception of the "to" tag.  (I think this
> repeats throughout the draft.)

Probably a generalized misunderstanding or copy-and-paste error. Will fix.

> -- Example 8:
>
> Inconsistent use of Allow header fields. I would probably leave them
> out except where required, but others may have other opinions.

It seems OK to remove them.

> First part of the paragraph seems to belong in 5.3. It might make
> sense to combined 5.3 and 5.4 into one section.

Probably, yes.

> Maybe this paragraph should belong in the section about the original

This seems like a fine place for it, but I'll take a second look.

> If the initial SEND in F42 was empty, would F44 and 45 still happen?

What do you mean by "empty"? Is this a bodiless request, or a request 
with an empty body?

> What if the MSRP user never sets a nick at all?

Hmm, good point. Instituting a timeout doesn't seem optimal. Will 
reconsider.

> How do you determine "sameness" between stanzas in this context?

Good question! I would say that, in this context, the stanza is "the 
same" if the XML elements in the payload match those from the outbound 
stanza that the MSRP-to-XMPP gateway generated from the MSRP SEND 
request. (There might be more elements, such as delayed delivery 
elements added by the MUC service, but matching the elements from the 
outbound stanza might work well, as long as the gateway matches only on 
stanzas received from the MSRP user's occupant JID.) I think we can 
describe this in a helpful manner (alternatively we could drop this 
behavior).

> s/SIP/" the SIP conference event package"

Correct.

> I'm confused by this--I can see saying to display the contents of
> <display-text/> or the JID resource part if the respective xcon
> element or <nick/> element are not present, but this seems to suggest
> one should ignore them if they are present. Is that the intent?

Right, the text should say "if present" in both cases.

Thanks again for the helpful and thorough review!

Peter

-- 
Peter Saint-Andre
https://andyet.com/