[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Last Call: Combined DES-CBC, HMAC and Replay Prevention Security Transform to Proposed Standard
I have a number of comments on the draft. IMO, the draft could use
another revision (to clarify some points) before being promoted to
Proposed Standard. Most of my concerns are minor. However, the first
two points may be significant.
First, the draft says:
>1. Discussion
>
> This draft allows a combination of MD5 and DES-CBC. In addition to
> privacy, the goal of this transform is to ensure that the packet is
> authentic, can not be modified in transit, or replayed.
The last sentence is misleading. In contrast to the standalone AH
headers (which cover the invariant fields of the IP header), the
transform described in the draft apparently does not cover the IP
header (or if it does, this fact completely escaped me). Thus,
modifications to the IP header fields (including the source/dest
addresses, which are part of the TCP transport identifiers) would not
be detected by this transform.
Thus, I conclude that the combined transform provides *weaker*
authentication than the standalone transforms. This surprises me, and
seems a mistake.
Another important point:
> 1. (Optional step) Decrypt the first bock of data using the
> appropriate DES_key_ and IV_key_ (or IV) and then do a quick "san-
> ity check" of the count. If the count has decreased below the win-
> dow or has increased by more than 65k, then it is safe to discard
> this packet as either a replay, non-authentic or too old. If the
> count is within 65K, then the probability that the packet is
> authentic is 65535/65536. (The following replay check and HMAC
> check are both still required).
Maybe I'm missing something, but the above would appear to not handle
network partitions well. If a destination becomes unreachable (for
whatever reason), and the sender sends 65K packets during the outage,
packets sent after the destination becomes reachable again will
continue to be rejected. Thus, this "optimization" seems wrong.
The remainder of my comments are fairly minor.
The draft never mentions IPv6. It apparently is intended to cover both
IPv4 and IPv6, and I think that fact should be explicitely stated at
the beginning to minimize confusion.
>2. Packet Format
>
>
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+---
> | Security Parameters Index (SPI) | ^
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |
How about putting bit numbers at the top (as is standard in other
drafts) to make it obvious that fields are 32 bits in size.
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |
> | | |
> + Initialization Vector (Optional) + |
> | | |
> +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ |
I assume the IV needs to be a multiple of 64-bits (to insure that the
Data starts on a 64-bit boundary for IPv6)? If so, the draft should
state this explicitely (it doesn't seem to).
> This field is optional and is only used when an explicit IV is nego-
> tiated during key exchange. This field is contains random data or
^^
Extra word.
> For the packet which the explicit IV is received, the explicit IV is
^
Missing word "in"
>2.3. Replay Prevention
>
> Replay Prevention is an unsigned 32 bit incrementing counter starting
> at a mutual agreed upon value (see Key Material) and is enforced to
> be within a mutually negotiated window size.
This section should more clearly state that every distinct IP packet
(including retransmissions) must use a new counter value.
Retransmissions that use the same replay counter may result in
deadlock.
> If a value of 32 is negotiated, then the most recent 32 packets are
> allowed to arrive out of order. That is, these 32 packets can arrive
The first sentence above is misleading. The "most recent" 32 packets
aren't accepted. Only those packets whose replay counter is within 32
of the highest numbered packet received so far are accepted.
> in any sequence relative to each other except that these packets are
> guaranteed to arrive only once. Appendix A has actual code that
Change "guaranteed to arrive only once" to "guaranteed to be accepted
only once"
> field. This field is an integral number of bytes in length; the fol-
> lowing padding and pad length fields will help provide alignment to a
> double word boundary.
I found the above wording a bit confusing. How about "This field
specifies the size of the payload in bytes"
> tiator direction). The IV_key_ remains constant for all packets send
> in this direction.
"send" should be "sent".
> CBC chaining with an constant IV_key_ is used. if an IV is present in
> the packet, then the IV_key_ is not used and is replaced by the IV.
I found the above somewhat confusing. How about rewording something
like:
By default, CBC chaining with a constant IV_key_ is used. The value of
IV_key_ is negotiated as part of the SPI creation. It is also possible
(if agreed upon by both parties during SPI setup) to include an IV in
each packet, in which case the included IV is used. Note that the
packet itself contains no indication of whether or not an IV in is
included; this knowledge is maintained as part of the Security
Association.
> 1. (Optional step) Decrypt the first bock of data using the
> appropriate DES_key_ and IV_key_ (or IV) and then do a quick "san-
> ity check" of the count. If the count has decreased below the win-
> dow or has increased by more than 65k, then it is safe to discard
> this packet as either a replay, non-authentic or too old. If the
> count is within 65K, then the probability that the packet is
> authentic is 65535/65536. (The following replay check and HMAC
> check are both still required).
What does it mean for the count to be "decreased below the window"?
Also, I don't understand where the 65535/65536 probability comes from
or what it means.
> 3. Calculate the HMAC using the HMAC_key_ and create the digest
> from the SPI, IV (if present) count, data, pad, pad length, and
> payload type and checking the result at digest at the end of
^^^^^^^^
Typo. Perhaps reword as "compare the result with the digest"?
>Appendix A
>
> This is a routine that implements a 32 packet window. This is intend-
> ed on being an implementation sample.
This code in fact assumes that the replay counter always starts at
0. It should state this, since this assumption is not made elsewhere
in the draft.
Also, if it is reasonable to always start the replay counter at zero,
why does the draft not simply require this? Right now, the initial
counter value is negotiated.
Thomas
References: