On Wed, 2013-10-16 at 14:53 -0400, Jeff Layton wrote: +AD4- On Wed, 16 Oct 2013 18:43:26 +-0000 +AD4- +ACI-Myklebust, Trond+ACI- +ADw-Trond.Myklebust+AEA-netapp.com+AD4- wrote: +AD4- +AD4- +AD4- On Thu, 2013-10-10 at 13:56 -0400, Jeff Layton wrote: +AD4- +AD4- +AD4- As Bruce points out in RFC 4121, section 4.2.3: +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +ACI-In Wrap tokens that provide for confidentiality, the first 16 octets +AD4- +AD4- +AD4- of the Wrap token (the +ACI-header+ACI-, as defined in section 4.2.6), SHALL +AD4- +AD4- +AD4- be appended to the plaintext data before encryption. Filler octets +AD4- +AD4- +AD4- MAY be inserted between the plaintext data and the +ACI-header.+ACIAIg- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- ...and... +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +ACI-In Wrap tokens with confidentiality, the EC field SHALL be used to +AD4- +AD4- +AD4- encode the number of octets in the filler...+ACI- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- It's possible for the client to stuff different data in that area on a +AD4- +AD4- +AD4- retransmission, which could make the checksum come out wrong in the DRC +AD4- +AD4- +AD4- code. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- After decrypting the blob, we should trim off any extra count bytes in +AD4- +AD4- +AD4- addition to the checksum blob. +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- Reported-by: +ACI-J. Bruce Fields+ACI- +ADw-bfields+AEA-fieldses.org+AD4- +AD4- +AD4- +AD4- Signed-off-by: Jeff Layton +ADw-jlayton+AEA-redhat.com+AD4- +AD4- +AD4- +AD4- --- +AD4- +AD4- +AD4- net/sunrpc/auth+AF8-gss/gss+AF8-krb5+AF8-wrap.c +AHw- 4 +-+--- +AD4- +AD4- +AD4- 1 file changed, 2 insertions(+-), 2 deletions(-) +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- diff --git a/net/sunrpc/auth+AF8-gss/gss+AF8-krb5+AF8-wrap.c b/net/sunrpc/auth+AF8-gss/gss+AF8-krb5+AF8-wrap.c +AD4- +AD4- +AD4- index 1da52d1..ec1f4d0 100644 +AD4- +AD4- +AD4- --- a/net/sunrpc/auth+AF8-gss/gss+AF8-krb5+AF8-wrap.c +AD4- +AD4- +AD4- +-+-+- b/net/sunrpc/auth+AF8-gss/gss+AF8-krb5+AF8-wrap.c +AD4- +AD4- +AD4- +AEAAQA- -574,8 +-574,8 +AEAAQA- gss+AF8-unwrap+AF8-kerberos+AF8-v2(struct krb5+AF8-ctx +ACo-kctx, int offset, struct xdr+AF8-buf +ACo-buf) +AD4- +AD4- +AD4- buf-+AD4-head+AFs-0+AF0-.iov+AF8-len -+AD0- GSS+AF8-KRB5+AF8-TOK+AF8-HDR+AF8-LEN +- headskip+ADs- +AD4- +AD4- +AD4- buf-+AD4-len -+AD0- GSS+AF8-KRB5+AF8-TOK+AF8-HDR+AF8-LEN +- headskip+ADs- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- - /+ACo- Trim off the checksum blob +ACo-/ +AD4- +AD4- +AD4- - xdr+AF8-buf+AF8-trim(buf, GSS+AF8-KRB5+AF8-TOK+AF8-HDR+AF8-LEN +- tailskip)+ADs- +AD4- +AD4- +AD4- +- /+ACo- Trim off the trailing +ACI-extra count+ACI- and checksum blob +ACo-/ +AD4- +AD4- +AD4- +- xdr+AF8-buf+AF8-trim(buf, ec +- GSS+AF8-KRB5+AF8-TOK+AF8-HDR+AF8-LEN +- tailskip)+ADs- +AD4- +AD4- +AD4- return GSS+AF8-S+AF8-COMPLETE+ADs- +AD4- +AD4- +AD4- +AH0- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- +AD4- If this is just padding, then why would it be anything other than '0', +AD4- +AD4- even on a retransmission? Clients are not supposed to leak random data +AD4- +AD4- to the server... +AD4- +AD4- +AD4- +AD4- The Linux client routines are actually set up to fill this with 'X' +AD4- characters, not nulls. See gss+AF8-krb5+AF8-aes+AF8-encrypt, which does this: +AD4- +AD4- memset(ecptr, 'X', ec)+ADs- +AD4- +AD4- AFAICT, the spec doesn't actually say what's supposed to be in the +AD4- padding, just that it's to be ignored. I imagine that most clients will +AD4- just set this to a constant value like Linux does, but there doesn't +AD4- seem to be any guarantee of that, and there's no real point in keeping +AD4- it around. The only caller of gss+AF8-krb5+AF8-aes+AF8-encrypt is gss+AF8-wrap+AF8-kerberos+AF8-v2(), which sets 'ec' to 0. -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust+AEA-netapp.com www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
