Subject: [TLS] I-D Action:draft-ietf-tls-dtls-heartbeat-00.txt

From: [email-masked]
Date: Fri, 18 Jun 2010 13:15:02 -0700 (PDT)

A New Internet-Draft is available from the on-line Internet-Drafts directories.
This draft is a work item of the Transport Layer Security Working Group of the IETF.

	Title           : Transport Layer Security (TLS) and Datagram Transport Layer Security (DTLS) Heartbeat Extension
	Author(s)       : R. Seggelmann, et al.
	Filename        : draft-ietf-tls-dtls-heartbeat-00.txt
	Pages           : 6
	Date            : 2010-06-18

This document describes the Heartbeat Extension for the Transport
Layer Security (TLS) and Datagram Transport Layer Security (DTLS)
protocol.

The Heartbeat Extension provides a new protocol for TLS/DTLS allowing
the usage of keep-alive functionality without performing a
renegotiation and a basis for path maximum transmission unit (PMTU)
discovery for DTLS.

A URL for this Internet-Draft is:
http://www.ietf.org/internet-drafts/draft-ietf-tls-dtls-heartbeat-00.txt

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

Below is the data which will enable a MIME compliant mail reader
implementation to automatically retrieve the ASCII version of the
Internet-Draft.

---

From: Juho Vähä-Herttua <[email-masked]>
Date: Sat, 19 Jun 2010 01:12:34 +0300

> Layer Security (TLS) and Datagram Transport Layer Security (DTLS)

I have a small problem with the section 4 of the draft. First of all there it's written:

 struct {
    HeartbeatMessageType type;
    opaque payload<0..2^14-5>;
    opaque padding<0..2^14-5>;
 } HeartbeatMessage;

 The length of payload and padding in total MUST NOT exceed 2^14-5
 bytes.

The notation "opaque payload<0..2^14-5>" is strange, usually this notation is used like "opaque payload<0..2^16-1>" which means "two length bytes followed by N data bytes". There is no common data type that would have a maximum value of "2^14-5", but there is a common data type with maximum value of "2^16-1", that is the unsigned 16-bit integer. Since the maximum total length of 2^14-5 is already explained in the following sentence, it should be no problem to change it as "2^16-1".

Personally I would also consider storing the padding more like in GenericBlockCipher, with padding_length stored in the end of the message. It requires less space and is much easier to process in the implementation.

Another question is mostly curiosity, what's the motivation behind having padding if it is discarded later? I would also like to have instructions on what to do if a HeartbeatResponse message has padding with non-zero length. Right now it is not mentioned at all.

Juho

---

From: Adam Langley <[email-masked]>
Date: Fri, 18 Jun 2010 19:58:16 -0400

> The notation "opaque payload<0..2^14-5>" is strange, usually this notation is used like "opaque payload<0..2^16-1>" which means "two length bytes followed by N data bytes". There is no common data…

RFC 5246 section 4.3 is quite clear about the meaning of 2^14-5. It's
better to be precise in the structure.

AGL

-- 
Adam Langley [email-masked] http://www.imperialviolet.org

---

From: Adam Langley <[email-masked]>
Date: Fri, 18 Jun 2010 20:08:42 -0400

> To perform PMTU discovery HeartbeatRequest messages containing

Missing a comma after 'discovery'

> HeartbeatRequest messages expires, this message is not longer

s/not/no/

The draft should name the extension; something like:

enum {
  heartbeat(TBD)
} ExtensionType;

The draft should also name the new record type:

enum {
  heartbeat(TBD)
} ContentType;

---

From: Juho Vähä-Herttua <[email-masked]>
Date: Sat, 19 Jun 2010 10:34:15 +0300

> better to be precise in the structure.

It does, however both 2^14-5 and 2^16-1 are not real ceilings because they depend on each other. In RFC 5246 variable-length vectors are only used in handshake protocol, which can be fragmented into several records, and therefore additional restrictions regarding the vector length are not necessary. Also if max_fragment_length of RFC-4366-bis is going to be also supported by the implementation, using length of 2^14-5 will also break the heartbeat protocol.

I would prefer something like:

   struct {
      HeartbeatMessageType type;
      opaque payload[HeartbeatParameters.payload_length];
      opaque padding[HeartbeatMessage.padding_length];
      uint16 padding_length;
   } HeartbeatMessage;

The padding_length could be uint8 as well, unless there's some reason for it to be uint16. Also it should be mentioned that the total length of HeartbeatMessage shouldn't exceed the length of maximum TLSPlaintext fragment size, instead of just using 2^14-5.

Juho

---

From: Michael Tüxen <[email-masked]>
Date: Sat, 19 Jun 2010 20:21:21 +0200

> The notation "opaque payload<0..2^14-5>" is strange, usually this notation is used like "opaque payload<0..2^16-1>" which means "two length bytes followed by N data bytes". There is no common data…
Looking at RFC 5246, section 4.3 I do not see any limitation on the array limits.
It is clearly stated how long the corresponding length field is. It is the
same for 2^14-5 or 2^16-1: It s two bytes.
> Another question is mostly curiosity, what's the motivation behind having padding if it is discarded later? I would also like to have instructions on what to do if a HeartbeatResponse message has p…
Hi Juho,

one application of the padding it to use Heartbeat messages for path MTU discovery.
I can send a test message of a particular size (by choosing the appropriate padding)
and get back a short ack. This way path MTU discovery works on asymmetric paths
without affecting user message transport. In particular this means that one need
padding which is longer than 255 bytes.

Best regards
Michael 
> 
> 
> Juho
> 
> 
> _______________________________________________
> TLS mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/tls
>

---

From: Michael Tüxen <[email-masked]>
Date: Sat, 19 Jun 2010 21:33:16 +0200

> Missing a comma after 'discovery'
Added. Thanks.
>> HeartbeatRequest messages expires, this message is not longer
Changed. Thanks.
> The draft should name the extension; something like:
Correct. We'll put this into the IANA section.
> The draft should also name the new record type:
Correct. We'll put this into the IANA section.

Thanks a lot for the comments.

Best regards
Michael

---

From: Michael Tüxen <[email-masked]>
Date: Sat, 19 Jun 2010 21:39:01 +0200

> It does, however both 2^14-5 and 2^16-1 are not real ceilings because they depend on each other. In RFC 5246 variable-length vectors are only used in handshake protocol, which can be fragmented int…
Hi Juhu,

need to look into max_fragment_length, but we can use a symbol instead of the constant.
> The padding_length could be uint8 as well, unless there's some reason for it to be uint16. Also it should be mentioned that the total length of HeartbeatMessage shouldn't exceed the length of maxim…
uint8 is not enough as stated earlier.
What I do not understand is what you gain by putting the padding_length at the end?
You do not save any space, I think. And it is also not complicated to use the
format the ID currently describes. At least it was implementable in a pretty
straight forward way in OpenSSL.

Best regards
Michael

---

From: Juho Vähä-Herttua <[email-masked]>
Date: Sun, 20 Jun 2010 01:51:51 +0300

> uint8 is not enough as stated earlier.

I agree, the MTU detection requires large padding to make sure the actual reply gets through.

> straight forward way in OpenSSL.

I don't like redundancy, and I like to have length variables in fixed locations if possible. It would also save 2 bytes to not store the payload length, wouldn't it? Based on the current ID I would implement the packet verification as follows:

1. read payload length
2. read padding length
3. verify that payload length + padding length is the same as TLSPlaintext.fragment_length-5

I also won't know the index of padding length before I've read the payload length first. If it would be stored like in block cipher padding I would do:

1. read padding length from the last 2 bytes
2. calculate payload length if necessary

In both cases I also have to check that TLSPlaintext.fragment doesn't go over 2^14 or the max_fragment_length, but I have to do that check anyway.

Of course both formats are ok, but I don't like to do step number 3, especially when the draft doesn't seem to say how I should react in case the lengths don't match. If there would be some more content, it would make more sense to store the both length values. But feel free to ignore me if you think my reasons are not good enough, I'm just trying to minimize the amount of error checking.

Juho