Subject: [Gen-art] gen-art review of draft-ietf-syslog-transport-tls-13.txt

From: Scott Brim <[email-masked]>
Date: Thu, 31 Jul 2008 16:58:34 +0100

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document: draft-ietf-syslog-tls-13.txt
Reviewer: Scott Brim
Review Date: 30 July 2008

Summary:

This draft is basically ready for publication, but has nits that should 
be fixed before publication.

Comments:

There is only one item that is significant, which is the last of the
SHOULD comments (just below).  As for the rest, they are not
showstoppers but imho if you take them into consideration you'll have a
better draft.

First, MUST/SHOULD
------------------

 >    TLS typically uses certificates [RFC5280] to authenticate peers.
 >    Implementations MUST support TLS 1.2 [I-D.ietf-tls-rfc4346-bis] and
 >    are REQUIRED to support the mandatory to implement cipher suite,

Small: Change "are REQUIRED to" to "MUST".

 >    The transport receiver and transport sender SHOULD provide
 >    mechanisms to record the end-entity certificate for the purpose of
 >    correlating it with the sent or received data.

Medium: In general it's good to explain the conditions under which a 
SHOULD is not expected.  Otherwise you get some implementations that 
treat it as a MUST and many others that just ignore it.  What happens if 
the sender or receiver does not record the end-entity certificate? 
Under what conditions is that a problem?

 >    If the peer does not meet the requirements of the security policy,
 >    the TLS handshake SHOULD be aborted with an appropriate TLS alert.

Medium-Large: This SHOULD surprises me.  You allow the peer not to
abort the handshake under some conditions?  What could those
conditions be?  I think you should explain this SHOULD carefully so as
to avoid a security hole through lack of documentation.

Second, terminology
-------------------

You have

   Transport sender	    Transport receiver
   Server    		    Client
   Peer			    Peer

These terms overlap.  I can't tell if the first two pairs are
synonymous or not.  I can see reasons to use all of these, based on
different relationships and functions, but you seem to mix and match
sometimes.  Sometimes limiting your terminology can make your spec
tighter and clearer, so consider getting rid of one pair.  Here are some 
examples:

 > 1.1 Terminology
 >
 >    o  A "TLS client" is an application that can initiate a TLS
 >       connection by sending a Client Hello to a peer.
 >
 >    o  A "TLS server" is an application that can receive a Client
 >       Hello from a peer and reply with a Server Hello.

Small: If you're going to use client, then use server, and vice versa.
A TLS client initiates a Client Hello to a *server*, and a server
receives a Client Hello from a *client*.

 > 4.1 Port Assignment
 >
 >    A syslog transport sender is always a TLS client and a transport
 >    receiver is always a TLS server.

I would have thought that since a server occasionally sends a message
to the client (for example a close_notify), that the server could be a
"transport sender" too ... and similarly the client receiving packets
could be a "transport receiver".

 > 4.2 Initiation
 >
 >    The transport sender should initiate a connection to the
 >    transport receiver and then send the TLS Client Hello to begin
 >    the TLS handshake.  When the TLS handshake has finished the
 >    transport sender MAY then send the first syslog message.

There are two actions here, "initiate a connection" and then have a
TLS handshake.  It would make more sense to me to use the terms
client/server than sender/receiver.

 >    A TLS client MUST close the associated TLS connection if the
 >    connection is not expected to deliver any syslog messages later.

Just above, the "transport sender" was told to set the message length,
but here the "TLS client" is told to close the TLS connection.  That 
feels inconsistent.

_______________________________________________
Gen-art mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/gen-art