[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
Thanks for the comments. They are indeed relevant. I will incorporate them
at the next revision. Thanks.
> 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.
In the case of IP in IP, the inner IP packet is the one protected by this
transform. I will add a statement that IP in IP is what was ment...
> 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.
2 comments, First, my choice of 65000 was arbitrary. That number can be up
to 16M with little change. That is, by making sure that the number was
within 16M, then the probabilitiy of a bad packet being caught is 255/256,
which makes this still reasonable.
Second, if 2 units have lost contact and have continued to send many
thousands of packets, then the far side comes back and you still allow the
old key to be used, seems to me to be to be scarry. Other devices that I
know of, if they lose contact, after a resonable period (less than 65000
packets), will force a new key to be negotiated (really it is forcing a new
authentication, which, after a major outage, seems reasonable).
Would you suggest a statement be added or the number be changes or?
> 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.
Granted.
> >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).
Interesting point. I assumed that the IV was a full DES block or 64 bits. I
will add this. I am also lucky, the SPI and Replay are 32 so the data
starts on a 64 bit boundry>
> > 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.
OK.
> > For the packet which the explicit IV is received, the explicit IV is
> ^
> Missing word "in"
OK.
> >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.
OK.
> > 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.
OK.
> > 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"
OK.
> > 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"
OK.
> > tiator direction). The IV_key_ remains constant for all packets send
> > in this direction.
>
> "send" should be "sent".
OK.
> > 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.
OK.
> > 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.
Assuming that the packet received count (x) and the highest received replay
count (r) are 32 bit unsigned number, if the packet is between r < x <
r+65K then this checks to see if x is within the 64k possible. If the field
has been randomized through modification of the first block -or- the
attacker does not know the key, then the value of x will be any random 32
bit value or one of 4B numbers. The probability of 2 random 32 bit numbers
being within 65K of each other is 4B/65K or 65K.
If the range check is increased to 16M (24 bits) then the ratio is 4B/16M
or 256.
> > 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"?
OK.
> >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.
Good point. I assumed that the caller would normalize this (by subtracting
the starting value. Since this is an unsigned value, this works.) Again, I
will add commments regarding this.
Again, thanks for the input, I will be adding this to the document next
go-around.
jim
Follow-Ups: