Subject: Re: [sipcore] Review of draft-ietf-sipcore-sec-flows-00

From: Brian Hibbard <[email-masked]>
Date: Thu, 29 Oct 2009 13:29:16 -0500

Thanks for the review, Ben.

I have some responses inline.  The minor text edits that you mention,  
unless I specifically address them, I will go ahead and accept unless  
someone has some objections.

On Sep 17, 2009, at 4:11 PM, Ben Campbell wrote:

> Hi,
>
> Someone pretending to be me agreed to review draft-ietf-sipcore-sec- 
> flows-00 at the SIPCORE meeting in Stockholm. Since the chairs  
> aren't buying my claims of identity theft, here's a review. I've  
> broken it into substantive and editorial sections.
>
> I did _not_ attempt mechanical verification of the data and scripts  
> in the draft--this is a human brain only review.
>
>
> -------------------------------------------------
>
> *** Substantive Comments:
>
> -- Status:
> Is this really intended as standards track, rather than  
> informational or possibly BCP? Is there anything normative in this  
> doc?

That's a good point.  I wasn't around for the document's inception,  
but I gather that this document was never intended to be normative.   
I'm OK with it being informational.  Does anyone else have opinions?   
Specifically, is this document to give examples of how things could be  
done, or how they should be done?

>
> -- Section 3, last paragraph: "This document does not show the  
> messages needed to check Certificate Revocation Lists (see [3]) as  
> that is not part of the SIP call flow"
>
> You explicitly list test cases for cert revocation--so I think it  
> would be fair to show them, or at least mention them in the flows. I  
> could argue that the TLS handshake is not part of the SIP flow  
> either. I suspect that one reason not to show them is that their may  
> not be a clear answer as to how to check CRLs--but if that is the  
> case we should shine a light on it.
>

I need to update the references because RFC 5280 obsoletes [3].   You  
have a point.  Perhaps we should show the checking of certificate  
revocation.  It only makes sense to me to show in the call flow if we  
are going to use OCSP.   If I understand correctly, Firefox, Safari  
and IE all use OCSP now.  What is vogue in the SIP community?   
Fetching CRLs via HTTP, out-of-band, wouldn't be very interesting to  
show.  How to check a certificate against a retrieved CRL is covered  
adequately in 5280.  We need to get some consensus on this.

> -- Section 4.1:
>
> Is "0" a reasonable serial number?
>

This was not intended.  The scripts that generate the certs somehow  
got misconfigured.  All the certs will need to be corrected in the  
next revision.

> The validity has a Not After date of 2013. Does that mean the RFC  
> expires then :-) More importantly, do the certs for testing in the  
> appendix share these dates? If so, I suggest they be set to further  
> in the future.
>

Ok.  I'll check to see what the latest date is that I can get the x509  
tool to accept.

> -- Section 4.2:
>
> The cert examples don't include chains longer than 2. I don't know  
> if it's really necessary to include examples for non-root issuer  
> certs, but it might be worth throwing in a paragraph that they can  
> exist and how they differ from root or end-entity certs.
>

I can add a paragraph about authorities that are not root CAs.

> -- Section 5.2: General
>
> I'd like to see a little more about how this MESSAGE request is  
> associated with the TLS session from the handshake example. I think  
> it might actually be useful to show the RFC3263 stuff somewhere,  
> either as an explicit part of the call flow or as a few sentences of  
> explanatory text. A few sentences about how the UA decides whether  
> to create a new TLS session or use an existing one might be helpful.

OK.  How about some text about RFC 3263.  Is connection reuse (not to  
be confused with "connection-reuse" draft)  for TLS any different than  
from vanilla TCP?  I can go look for a suitable description of that  
and add some text.

>
> -- Section 5.2, MESSAGE request:
>
> Contact: RFC3428 forbids Contact header fields in MESSAGE requests  
> or 2XX responses to MESSAGE. This brings up the question as to  
> whether MESSAGE is a good example, as you may wish to illustrate  
> SIPS rules concerning Contact. (This reoccurs in all MESSAGE and 200  
> OK examples)

We definitely need to figure out what we need to do here.  Elsewhere  
we talked about whether most people would really rather see INVITE.   
The more we change the messages, the more complex the draft will  
become and the more work it will take to produce it.  That's fine, if  
that's what needs to happen, but we do need to discuss this on the list.

>
> CSeq: I suggest a more "random looking" initial CSeq value.
>

Sure.

> -- Same section, 200 OK
>
> Contact: There's that Contact again. Also, if you _were_ going to  
> illustrate Contact, I think a SIPS URL rather than "transport=TLS"  
> would be appropriate, since the original r-URI was SIPS.

I'll look into why the stack did that and what it would take to make  
it go away.

>
> -- Section 6.1, first text paragraph at top of page 16: "Also note  
> that the sender’s certificate is not attached as it is optional in  
> [8]."
>
> It would be instructive to have an example with the certificate  
> included.

That doesn't sound like it would be too hard to get the software to do  
that.

>
>
> -- Section 7, title "Observed Interoperability Issues"
>
> Who observed them? Is this experience from SIPits, etc? I think it  
> would strengthen the idea that these are real-world, observed-in-the- 
> wild issues to give sources.
>
> -- Paragraph 2: "Some SIP clients..."
>
> Do mean client class devices, or user agents in general? Does this  
> exclude proxies? (same question throughout section.)
>
> -- Paragraph 5: "Implementations should send.... but must be  
> prepared to receive..."
>
> Is that intended as a normative statement?
>
> -- Last paragraph:
>
> I think there's some implicit stuff here that should be explicit. Do  
> you mean to recommend never attaching certs? It's probably worth  
> mentioning the message size limit issue. What do you mean by "it  
> cannot be relied upon"--that you can't rely on the peer sending it,  
> or it is unreliable when the peer does send it?

Is there anyone out there who can answer these questions about section  
7?  Cullen?  Kumiko?  Robert?

>
> -- Section 8, first bullet entry: "the peer's URI"
>
> What URI? An AoR for the user? From or To values? Contacts? Request- 
> URIs?
>

I agree, this should be specific.

> For request URIs, do we need to discuss the effects of retargeting?  
> Do we need to consider some of the current History-Info discussions?
>

Does anyone else in the working group have opinions on this?

> -- 2nd bullet:
>
> What if all you've got is an IP address? Do we disallow IPAddress  
> entries in SubjectAltName?
>
> -- 1st sub-bullet: "(Wildcard
> matching is not allowed against these dNSName entries)"
>
> Is there something that can be referenced here? In particular,  
> RFC2818 explicitly allows wildcards in dNSName entries. It is not  
> obvious to me whether the proscription against wildcards in RFC4474  
> should apply to general use of TLS, or just to identity.
>

I don't have answers right now.  We'll have to look into these.

> -- section 8, list of tests:
>
> What about cases where the basic constraints or allowed uses are not  
> appropriate? Is it worth putting in cases around self-signed certs?  
> (i.e. self-signed cert, explicitly trusted, not trusted, etc.) How  
> about cases where one or more certs in the chain to the root were  
> not provided and not available through other means?
>

Those seem like sensible cases to either include or explain why they  
aren't included.  The self-signed cert is definitely a popular case  
among developers.

> -- 13.2 (Informative References)
>
> The criteria in this draft for normative vs informative references  
> are not clear to me, particularly  in the cases of 17 and 18. OTOH,  
> are any of the references really normative for this draft?
>

The normative vs informative issue in general definitely needs to be  
clarified.

> Appendix A:
>
> Do you expect these scripts to work with all future versions of  
> OpenSSL :-)  If not, it may be worth mentioning the latest version  
> that this has been tested with.
>

:)  I will throw in that they worked with 0.9.8 j (or later if we move  
on to a later version).

> -- 3rd paragraph from end: "IE, Outlook, and Netscape..."
>
> Please give full names and version numbers tested. Also, did you  
> really mean Netscape instead of Firefox?
>
>

I think that reference should be changed to Firefox.  Specific  
versions would be a good idea.

>
> *** Editorial Comments:
>
> -- Title:
>
> The title is really a bit misleading, as it sounds like an  
> exhaustive set of security flows, when it's really pretty narrow.  
> Something more along the line of "...using SIP with TLS and S/MIME".  
> rather than "...using SIP security mechanisms." might be in order.
>

I agree that the title could be more descriptive of what the draft  
actually has become.  Can we get consensus on the list on exactly what  
the title should be?

> -- Section 2:
> I didn't find any 2119 normative language in the draft. If there's  
> no intent to have normative language, please remove section 2 and  
> the 2119 reference.
>

That's reasonable.

> -- Section 3:
> It's odd to see Security Considerations so early. I think rfc2223  
> requires it to be "near the end".
>

They're near the front end. :)   Ok, moving them is no problem.

> -- Section 5.1, first paragraph:
>
> (Personal pet peeve) Please avoid the form "defined in [6]" for  
> references. That may save the author some effort, but creates more  
> effort for the reader to have to flip all over the place.
>
> The best approach is "defined in <doc name or description>[6]",  
> although I've learned to put up with "defined in [mnemonic  
> reference]". I prefer the former,  as the sentence should makes  
> sense with the reference stripped completely. But at least the  
> latter lets the reader know what is being referenced without having  
> to flip to the end of the doc every time.
>
> -- Section 5.1, SSLDump output:
>
> A bit of explanation of the format whould be helpful. For example, a  
> bit about the C>S vs S>C fields would help. Or maybe a reference to  
> the SSLDump documentation.
>
>
> -- Section 6.1, 1st paragraph: "Example Signed Message."
>
> Sentence fragment.
>
> -- Section 6.1, first paragraph after example MESSAGE request:
>
> The paragraph is ambiguous about which "header", "boundary", and  
> "Content" type you refer to, since all of those occur multiple times  
> in the entire message.  I think you are referring to the text/plain  
> sub-part in all cases, but it could be more clear.
>
> -- Section 6.1, top of page 16:
>
> Here you re-state the contents of the text/plain sub-part I  
> mentioned in the previous comment. But it's not clear from the  
> formatting or text that this is the case--it looks like random  
> orphaned lines. This is aggravated by the poor luck of a page break  
> right before it. I suggest adding some text to the previous example  
> to the effect of "... in the body part shown below:" It would also  
> help to indent or otherwise set off the body part text from the text  
> around it.
>
> -- Section 6.1, first non-example paragraph at the top of page 16:  
> "ASN.1 parse of binary Blob 1."
>
> Sentence Fragment.
>
> -- Section 6.1, top of page 18: "The above dump of Blob 1 has SHA-1  
> parameters set to NULL. Below are the same contents signed with the  
> same key, but omitting the NULL according to [10]. This is the  
> preferred encoding."
>
> I would consider putting the preferred encoding first. (Do you even  
> need an example of the non-preferred encoding?--why not just show  
> the preferred on and mention the non-preferred in text, or in the  
> interop issues section?)
>

That's a good point about which should be first.  The reason for  
including the non-preferred encoding is because the legacy software  
widely deployed does it the non-preferred way, so that's what most  
people are going to see.   That said, I agree with you that the  
preferred encoding should come first, and unless someone really  
objects, I intend to rearrange the text that way.

> -- 6.2, first paragraph: 'Example encrypted text/plain message that  
> says "hello":'
>
> Sentence fragment.
>
> -- 6.2, "ASN.1 parse of binary Blob 2."
>
> Sentence fragment.
>
> -- Section 7, Paragraph 4: (starting with "When used with SIP,..."
>
> Is this an interop issue? You don't mention whether this is  
> something that devices get wrong. As it stands, it is probably more  
> suited for the SecCons section.
>

I see what you're getting at, but in a security-related document,  
someone could argue that all the interop issues are security  
considerations.  My belief is that since this document does not  
introduce new behavior, this probably really is an interop issue.  The  
fact that it is a security consideration also is hopefully covered in  
some other document.  Does anyone think I'm off in my reasoning?

> 2nd to last paragraph:
>
> Another candidate for SecCons.
>
> -- Section 8, first paragraph:
>
> Calling the section the "beginning of a list" implies you aren't  
> through with it. Perhaps you mean "non-exhaustive", or "an example  
> list" or something else?
>
> -- 2nd paragraph:
>
> I take this to mean some of this is truly folklore, and some come  
> from bona-fide normative language somewhere. Can you distinguish  
> between the two? The true folklore parts may point to future  
> normative work we need to do.
>
> For [16], can you reference the section number?
>
> Also, you mention [16] and [17] as normative works, but they are  
> informative references. I don't know if that's a problem, just  
> sayin'...
>
> -- section 8, 2nd bullet item:
>
> s/"as fed into 3263..."/"from the initial DNS query in the server  
> location process. [RFC3263]"

Good. Thanks.

>
> -- Section 12:
>
> I suspect this section should be removed from the RFC, as hopefully  
> the issue will be resolved. And since an RFC is written in stone,  
> I'm not sure the maintainability of the examples will matter anymore.
>

Yes, my bad.

> -- Appendix A, general:
>
> It would be handy to have a table(s) showing file names and  
> contents, rather than having to pick through paragraph streams to  
> find them.
>

OK.

> -- Appendix A, 3rd paragraph, last sentence: "...filed..."
>
> should "filed" be "file"?
>
>

It should.