| Message ID | 1291132155-31277-4-git-send-email-martin@strongswan.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Nov 30, 2010 at 03:49:13PM +0000, Martin Willi wrote: > > + if (skb->len >= tfcpadto) { > + clen = ALIGN(skb->len + 2, blksize); > + } else if (x->tfc.flags & XFRM_TFC_ESPV3 && > + x->props.mode == XFRM_MODE_TUNNEL) { > + /* ESPv3 TFC padding, append bytes to payload */ > + tfclen = tfcpadto - skb->len; > + clen = ALIGN(skb->len + 2 + tfclen, blksize); > + } else { > + /* ESPv2 TFC padding. If we exceed the 255 byte maximum, use > + * random padding to hide payload length as good as possible. */ > + clen = ALIGN(skb->len + 2 + tfcpadto - skb->len, blksize); > + if (clen - skb->len - 2 > 255) { > + clen = ALIGN(skb->len + (u8)random32() + 2, blksize); > + if (clen - skb->len - 2 > 255) > + clen -= blksize; > + } What is the basis of this random length padding? Also, what happens when padto exceeds the MTU? Doesn't this effectively disable PMTU-discovery? I know that your last patch allows the padto to be set by PMTU. But why would we ever want to use a padto that isn't clamped by PMTU? Cheers,
> What is the basis of this random length padding? Let assume a peer does not support ESPv3 padding, but we have to pad a small packet with more than 255 bytes. We can't, the ESP padding length field is limited to 255. We could add 255 fixed bytes, but an eavesdropper could just subtract the 255 bytes from all packets smaller than the boundary, rendering our TFC efforts useless. By inserting a random length padding in the range possible, the eavesdropper knows that the packet has a length between "length" and "length - 255", but can't estimated its exact size. I'm aware that this is not optimal, but probably the best we can do(?). > Also, what happens when padto exceeds the MTU? Doesn't this > effectively disable PMTU-discovery? Yes. An administrator setting a padto value larger than PMTU can currently break PMTU discovery. > I know that your last patch allows the padto to be set by PMTU. > But why would we ever want to use a padto that isn't clamped by > PMTU? Probably never, valid point. I'll add PMTU clamping to the next revision. We probably can drop the PMTU flag then and just use USHRT_MAX instead. Thanks! Martin -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 03, 2010 at 09:32:55AM +0100, Martin Willi wrote: > > > What is the basis of this random length padding? > > Let assume a peer does not support ESPv3 padding, but we have to pad a > small packet with more than 255 bytes. We can't, the ESP padding length > field is limited to 255. > We could add 255 fixed bytes, but an eavesdropper could just subtract > the 255 bytes from all packets smaller than the boundary, rendering our > TFC efforts useless. > By inserting a random length padding in the range possible, the > eavesdropper knows that the packet has a length between "length" and > "length - 255", but can't estimated its exact size. I'm aware that this > is not optimal, but probably the best we can do(?). I know why you want to do this, what I'm asking is do you have any research behind this with regards to security (e.g., you're using an insecure RNG to generate a value that is then used as the basis for concealment)? Has this scheme been discussed on a public forum somewhere? > > I know that your last patch allows the padto to be set by PMTU. > > But why would we ever want to use a padto that isn't clamped by > > PMTU? > > Probably never, valid point. > > I'll add PMTU clamping to the next revision. We probably can drop the > PMTU flag then and just use USHRT_MAX instead. Sounds good. Thanks,
Hi Herbert, > I know why you want to do this, what I'm asking is do you have any > research behind this with regards to security > > Has this scheme been discussed on a public forum somewhere? No, sorry, I haven't found much valuable discussion about TFC padding. Nothing at all how to overcome the ESPv2 padding limit. > using an insecure RNG to generate a value that is then used as the > basis for concealment Using get_random_bytes() adds another ~10% processing overhead due to the underlying sha_transform. But this is probably negligible, we add much more with the additional padding to encrypt/MAC. I'll re-spin the patchset with get_random_bytes(). Even if the ESPv2 padding fallback makes TFC in this case less efficient, it shouldn't harm. Or do you see this differently? Regards Martin -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 06, 2010 at 04:10:25PM +0100, Martin Willi wrote: > > > > Has this scheme been discussed on a public forum somewhere? > > No, sorry, I haven't found much valuable discussion about TFC padding. > Nothing at all how to overcome the ESPv2 padding limit. OK. > I'll re-spin the patchset with get_random_bytes(). Even if the ESPv2 > padding fallback makes TFC in this case less efficient, it shouldn't > harm. Or do you see this differently? Indeed I don't think we should do anything for the ESPv2 case at all without having this discussed in an appropriate forum first. So please remove that part completely from your submission for now. Thanks,
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c index 67e4c12..a6adfbc 100644 --- a/net/ipv4/esp4.c +++ b/net/ipv4/esp4.c @@ -117,23 +117,43 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb) int blksize; int clen; int alen; + int plen; + int tfclen; + int tfcpadto; int nfrags; /* skb is pure payload to encrypt */ err = -ENOMEM; - /* Round to block size */ - clen = skb->len; - esp = x->data; aead = esp->aead; alen = crypto_aead_authsize(aead); blksize = ALIGN(crypto_aead_blocksize(aead), 4); - clen = ALIGN(clen + 2, blksize); - - if ((err = skb_cow_data(skb, clen - skb->len + alen, &trailer)) < 0) + tfclen = 0; + tfcpadto = x->tfc.pad; + + if (skb->len >= tfcpadto) { + clen = ALIGN(skb->len + 2, blksize); + } else if (x->tfc.flags & XFRM_TFC_ESPV3 && + x->props.mode == XFRM_MODE_TUNNEL) { + /* ESPv3 TFC padding, append bytes to payload */ + tfclen = tfcpadto - skb->len; + clen = ALIGN(skb->len + 2 + tfclen, blksize); + } else { + /* ESPv2 TFC padding. If we exceed the 255 byte maximum, use + * random padding to hide payload length as good as possible. */ + clen = ALIGN(skb->len + 2 + tfcpadto - skb->len, blksize); + if (clen - skb->len - 2 > 255) { + clen = ALIGN(skb->len + (u8)random32() + 2, blksize); + if (clen - skb->len - 2 > 255) + clen -= blksize; + } + } + plen = clen - skb->len - tfclen; + err = skb_cow_data(skb, tfclen + plen + alen, &trailer); + if (err < 0) goto error; nfrags = err; @@ -148,13 +168,17 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb) /* Fill padding... */ tail = skb_tail_pointer(trailer); + if (tfclen) { + memset(tail, 0, tfclen); + tail += tfclen; + } do { int i; - for (i=0; i<clen-skb->len - 2; i++) + for (i = 0; i < plen - 2; i++) tail[i] = i + 1; } while (0); - tail[clen - skb->len - 2] = (clen - skb->len) - 2; - tail[clen - skb->len - 1] = *skb_mac_header(skb); + tail[plen - 2] = plen - 2; + tail[plen - 1] = *skb_mac_header(skb); pskb_put(skb, trailer, clen - skb->len + alen); skb_push(skb, -skb_network_offset(skb));
If configured on xfrm state, increase the length of all packets to a given boundary using TFC padding as specified in RFC4303. For transport mode, or if the XFRM_TFC_ESPV3 is not set, grow the ESP padding field instead. Signed-off-by: Martin Willi <martin@strongswan.org> --- net/ipv4/esp4.c | 42 +++++++++++++++++++++++++++++++++--------- 1 files changed, 33 insertions(+), 9 deletions(-)