Subject: AW: [Sip] comments on draft-ietf-sip-saml-00

From: "Tschofenig, Hannes" <[email-masked]>
Date: Mon, 21 Aug 2006 11:24:49 +0200

Hi Jonathan, 

thanks for your feedback. I noticed that I never replied to your mail. 
Please find my response below: 
 
> -----Ursprüngliche Nachricht-----
> Von: Jonathan Rosenberg [mailto:[email-masked]] 
> Gesendet: Freitag, 7. Juli 2006 21:30
> An: IETF SIP List
> Betreff: [Sip] comments on draft-ietf-sip-saml-00
> 
> Overall, this is a HUGE improvement over 
> draft-tschofenig-sip-saml-04, 
> which is the last version I read (I see that -05 was similar to 
> draft-ietf-sip-saml). This looks like a viable solution 
> overall, and I 
> like how it dovetails with sip-identity.
> 
> 
> 
> Major comments:
> 
> * I'm a little worried that the SAML assertion itself doesn't include 
> any content from the SIP message which clearly limits its scope of 
> applicability to the request. You are relying on authentication via 
> sip-identity for that, but there are attacks where this won't work.
> 
> For example, consider a rogue proxy or MITM which sees a SIP 
> request fly 
> by. The AS was executed by an upstream proxy, so this request has an 
> Identity and Identity-Info header field. The proxy modifies the 
> Indentity-Info header field, which contains a URL to an older 
> assertion 
> generated by the AS for the same subject, say, for a previous call it 
> just made.
> 
> This is not a major threat, but there may be others like it. Is there 
> harm in including things like call-id, contact and other 
> parameters into 
> the saml assertion so they are signed too?

I fully agree with you. 

> 
> * I'm not clear on how this mechanism works with older sip-identity 
> implementations. The main issue is whether the URI in the 
> identity-info 
> header field will return just a certificate, as specified in 
> sip-identity, or whether it returns a saml assertion. An older 
> sip-identity verifier will expect a certificate but it will get an 
> assertion if the AS implements sip-saml. Perhaps you are 
> assuming that 
> the Accept headers or something are used for this? Somehow 
> this needs to 
> be handled.

My understanding was that this got fixed with the SIP Identity draft. I might, however, be worth to double-check it. 

> 
> 
>  > 6.1.4.1.3.  Element: <Conditions>
>  >
>  >    The <Conditions> element SHOULD contain an <AudienceRestriction>
>  >    element, which itself SHOULD contain an <Audience> element.  The
>  >    value of the <Audience> element SHOULD be the same as 
> the addr-spec
>  >    of the SIP request's To header field.
> 
> Hmm. Here is our infamous retargeting problem again. If the 
> request is 
> retargeted, does this mean that the assertion cannot be used by the 
> forwarded party? This restriction doesn't exist for normal 
> sip-identity.

That's a good point. This is somewhat a difficult problem: Ideally, you would like to use the assertion in a very flexible way. By doing this you introduce some security problems (since the assertion can be re-used or use outside a context that was somehow envisioned). If you put some restrictions into the assertion then experience these types of problems. 

> 
> 
> Minor comments:
> 
> > The scope of this specification is:
> > 
> >    o  Specify a SIP profile of SAML -- aka a "SIP SAML 
> profile" -- such
> >       that a subject's profile attributes, and their domain's
> >       certificate, can be conveyed to a relying party using 
> SAML.  In
> >       doing so, satisfy the requirements outlined in 
> [I-D.ietf-sipping-
> >       trait-authz], and compose with [I-D.ietf-sip-identity].
> 
> perhaps I've misunderstood, but don't we also need to specify 
> a SIP SAML 
> binding?

Yes; we need to specify a binding an a profile. 

> 
> >  SIP-specific SAML profile(s) and binding(s).  This is technically
> >    straigh-forward as both SAML and SIP are explicitly extensible.
> 
> straightforward
fixed. 

> 
> > [RFC3261].  Depending on the chosen SIP security mechanism
> >            either digest authentication, S/MIME or Transport Layer
> >            Security is used to provide the AS with a strong 
> assurance
> >            about the identity of Alice.
> 
> well, TLS in this use case is not normally used to provide 
> the AS with 
> the identity of ALice.

It depends how you use TLS. 

> Perhaps you are referring to mutual 
> authentication with tls?

Yes. 

> Or maybe you just mean digest over tls?

Maybe we need to phrase the sentence differently but we referred here mainly to the client authentication mechanisms. 

What about the following sentence:

"
Depending on the chosen SIP security mechanism for client authentication either digest authentication, client side authentication of Transport Layer Security, or a combination of both is used to provide the AS with a strong assurance about the identity of Alice.
"

> 
> >  signature" for the message and add Identity and Identity-Info
> >            headers to the message.  The AS also at this 
> time constructs
> 
> I feel like a broken record, but its header field and not header
> 

fixed. 

> 
> * sections 6.1.3, 6.1.2 and 5 all seem pretty redundant, as 
> they contain 
> the same flow with nearly the same levels of detail. Are all 
> three needed?

Hmmm. Maybe we can shorten the draft here a bit. 
We previously had more functionality in the draft. A lot of it got removed to get more specific with some parts of the document (but might be added again later).  

> 
> 
> >  Attribute: NotOnOrAfter
> > 
> >       The value of the NotOnOrAfter XML attribute MUST be 
> set to a time
> >       instant later than the value for NotBefore.
> 
> guidance on how to set this?
> 
> >   This section specifies the "SAML HTTP-URI-based SIP Binding",
> >    (SHUSB).
> 
> SHUSB? Geez, this acronym is awful.

Do you have an idea for a nice acronym? 

> 
> > Section 3.7.5.4 Error Reporting:
> > 
> >       All SHOULDs in this section are to be interpreted as MUSTs.
> > 
> 
> This is somewhat mysterious. Is there something SIP specific 
> which would 
> cause you to want to do this, or just a beef with the fact that 
> everything there is a SHOULD and not a MUST? Also perhaps a few words 
> about whats in that section would be helpful.
> 
> Indeed its not obvious to me why we can't just use the 
> existing SAML URI 
> binding if we're just doing a vanilla http query to retrieve 
> the assertion.

With the restrictions listed in this section you define a new profile.

There is, however, the question why these restrictions are so important. To me it sounds reasonable to just put MUST (instead of a SHOULD) in the text about error reporting: 
"
3.7.5.4 Error Reporting

As an HTTP protocol exchange, the appropriate HTTP status code SHOULD be used to indicate the result of a request. For example, a SAML responder that refuses to perform a message exchange with the SAML requester SHOULD return a "403 Forbidden" response. If the assertion specified is unknown to the responder, then a "404 Not Found" response SHOULD be returned. In these cases, the content of the HTTP body is not significant.
"

There still remains the question why the SAML guys have written the text with a SHOULD in the first place.

Maybe Jeff can help us here. 

> 
> > The value of the <NameID> element MUST be the same as the Address of
> >    Record (AoR) value used in computing the "signed-identity-digest"
> >    which forms the value of the Identity header.  See Section 9 of
> >    [I-D.ietf-sip-identity].
> 
> I read this as being that this is the literal AoR, i.e., 
> sip:user@domain. But, in the example later on:
> 
> > 1 <Assertion ID="_a75adf55-01d7-40cc-929f-dbd8372ebdfc"
> >     2    IssueInstant="2003-04-17T00:46:02Z" Version="2.0"
> >     3    xmlns="urn:oasis:names:tc:SAML:2.0:assertion">
> >     4    <Issuer>
> >     5       example.com
> >     6    </Issuer>
> >     7    <Subject>
> >     8       <NameID
> >     9         Format=
> >    10         
> "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">
> >    11         [email-masked]
> >    12       </NameID>
> 
> there is no sip:, and the format attribute has a URN which explicitly 
> says emailAddress and not SIPURI. I think this should be a SIP URI.

I think we need to define a few new SIP specific formats in here. I wouldn't want to overloa the emailAddress field here. 

Would that be OK for you? 

Ciao
Hannes

PS: I have created a few new issues in the issue tracker at: 
http://www.tschofenig.priv.at:8080/saml-sip
I will speak to my co-authors about updating the document. Then, I will get back to you and the group. 

> 
> Thanks,
> Jonathan R.
> -- 
> Jonathan D. Rosenberg, Ph.D.                   600 Lanidex Plaza
> Cisco Fellow                                   Parsippany, NJ 
> 07054-2711
> Cisco Systems
> [email-masked]                              FAX:   (973) 952-5050
> http://www.jdrosen.net                         PHONE: (973) 952-5000
> http://www.cisco.com
> 
> _______________________________________________
> Sip mailing list  https://www1.ietf.org/mailman/listinfo/sip
> This list is for NEW development of the core SIP Protocol
> Use [email-masked] for questions on current sip
> Use [email-masked] for new developments on the application of sip
> 

_______________________________________________
Sip mailing list  https://www1.ietf.org/mailman/listinfo/sip
This list is for NEW development of the core SIP Protocol
Use [email-masked] for questions on current sip
Use [email-masked] for new developments on the application of sip