Re: [AVT] draft-ietf-avt-ports-for-ucast-mcast-rtp-04: Review Comments

["Ali C. Begen (abegen)" <[e-mail]>]

Hi Magnus,

We are almost there. A few points are left below. I also removed a few issues (such as O/A stuff) and I will discuss them separately.

> >> I have done a review of the document and have a number of comments. The
> >> upper part will contain technical or larger editorial comments, while
> >> the lower parts will be minor editorial comments.
> >>
> >> T1. Section 1. I am missing a discussion of the signalling aspects for
> >> the applications using these tools. Declarative service signalling seems
> >> to be one of the reasons for doing dynamic session creation.
> >
> > Yes, that is the primary use case. But, I am not sure whether I understand what you mean by "discussion of the signaling
> aspects".
> 
> I think the general requirements on what needs to be signalled is not
> sufficiently well explained in the draft. Like the need for a unicast
> session with the RTP configuration for what will be transmitted. The c=
> address, etc.

I hope the new version will address this. If not, we will try again.

> >
> >> T2. Section 3. I am missing an explicit list of the assumptions on what
> >> is in place around the service when describing the port mapping.
> >
> > I am also not sure what you mean here.
> 
> To me it appears there is certain assumptions around the service when
> using port mapping. That the service are highly dynamic around the the
> unicast sessions, and also that it uses a more declarative style of
> configuration.
> 

Also, I will let you read the revision first.

> >> How does a client determine if it has a valid token or not. To my
> >> understanding the following things needs to be done.
> >>
> >> First, it needs to check if it has a token for the server + port
> >> specified by the session description. May be from a previous session.
> >> Secondly, it checks if it hasn't expired due to age.
> >
> > The client can only do one reliable check based on the relative exp time. The client's public IP may have changed and it
> cannot necessarily know this.
> 
> The first step is about the servers address and port. It is how a
> receiver determine if it can reuse a token or needs to get another one.
> That is not described so that one can safely implement this. But I
> assume you are of interest in reusing the token for example between all
> TV channels a particular RS handles.

Right.
 
> >
> >> T4. Section 4. IP address for token request.
> >>
> >> It is not clear on how the IP address for the Token Request is
> >> determined. Is this the C=? I think there is a good point in allowing
> >> optional specification of the address information in this SDP attribute,
> >> similar to the a=rtcp attribute.
> >
> > The address is the feedback target address in the primary session, which equals to the address given in the C line for the
> unicast session (as you indicated).
> >
> > Does it make any sense to have the token request message to be sent to a different address than the unicast source (which
> also means the token response could be sent by another entity)? This may break the Token actually since client's IP address
> might be different for different destinations, right?
> 
> I think we should enable it as I find it likely that someone will use
> such a setup. If the setup is a anything like homes with NATs and no
> large scale NAT then I don't think there is likely that you will get
> another IP address in the NAT. Even large scale NATs will do their best
> to use the same IP address for the same underlying host.

True, it really increases the number of failure modes. Do we really need this?

> > I don't think the server can determine which component was the reason for verification failure. I.e., the server generates a
> new token based on the new input and it can only decide whether it is a match or not. If it is not, it cannot determine
> whether the nonce or the IP was to blame.
> >
> > As for the expiration time, the client should not send such tokens anyway. If it does and it receives a failure message, the
> reason would be obvious.
> >
> > Overall, I don't think we can (or need anyway to) specify the reason for failure.
> 
> So you are basically saying, if you get Token error, simply go request a
> new one?

Right. 
 
> And you don't think there will be any difference in the client behavior
> based on the reason.

The client always needs to make sure that the token has not expired. Beyond that its behavior does not matter.

> 
> Ok, I think I can live with that in the token verification step.
> However, then I think we need something in the token request response
> message that says, sorry, I can't give you a valid token. Otherwise the
> server has no way of cleanly say no to a receiver that are not being

We already have this. The server sends the Port Mapping Response with relative expiration time set to 0. 

> >> T11. Section 5.4: Why isn't the "associated Nonce" included in the error
> >> response? Either that or the token needs to be included so that the
> >> client knows which token that didn't work. This as a client may have
> >> multiple outstanding requests and not necessarily with the same token.
> >
> > Normally a client would not need to send multiple different requests to the same FTAp to get a token. If the client puts a
> different nonce in each request, yes, the tokens will be different but what is the point in doing that? One token is enough for
> the client.
> 
> Please note that this is the verification error message. I agree that a
> sanely setup system you shouldn't use the same Feedback target
> address+port shouldn't get RTCP messages related to different services
> without them explicitly sharing the tokens. But, as currently defined
> this is not strictly disallowed I think.

We added the nonce to the verification failure message.

> >> Then I am missing the signalling aspect vulnerabilities on this solution
> >> and the requirement that puts on the signalling infrastructure.
> >
> > You mean if somebody messes with the portmapping-req attribute in the SDP?
> 
> Exactly, or c= attribute

Ok, added this to the new security section.

-acbegen