On Wed, 16 Oct 2013 18:43:26 +0000
"Myklebust, Trond" <Trond.Myklebust@xxxxxxxxxx> wrote:
> On Thu, 2013-10-10 at 13:56 -0400, Jeff Layton wrote:
> > As Bruce points out in RFC 4121, section 4.2.3:
> >
> > "In Wrap tokens that provide for confidentiality, the first 16 octets
> > of the Wrap token (the "header", as defined in section 4.2.6), SHALL
> > be appended to the plaintext data before encryption. Filler octets
> > MAY be inserted between the plaintext data and the "header.""
> >
> > ...and...
> >
> > "In Wrap tokens with confidentiality, the EC field SHALL be used to
> > encode the number of octets in the filler..."
> >
> > It's possible for the client to stuff different data in that area on a
> > retransmission, which could make the checksum come out wrong in the DRC
> > code.
> >
> > After decrypting the blob, we should trim off any extra count bytes in
> > addition to the checksum blob.
> >
> > Reported-by: "J. Bruce Fields" <bfields@xxxxxxxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > net/sunrpc/auth_gss/gss_krb5_wrap.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> > index 1da52d1..ec1f4d0 100644
> > --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
> > +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> > @@ -574,8 +574,8 @@ gss_unwrap_kerberos_v2(struct krb5_ctx *kctx, int offset, struct xdr_buf *buf)
> > buf->head[0].iov_len -= GSS_KRB5_TOK_HDR_LEN + headskip;
> > buf->len -= GSS_KRB5_TOK_HDR_LEN + headskip;
> >
> > - /* Trim off the checksum blob */
> > - xdr_buf_trim(buf, GSS_KRB5_TOK_HDR_LEN + tailskip);
> > + /* Trim off the trailing "extra count" and checksum blob */
> > + xdr_buf_trim(buf, ec + GSS_KRB5_TOK_HDR_LEN + tailskip);
> > return GSS_S_COMPLETE;
> > }
> >
>
> If this is just padding, then why would it be anything other than '0',
> even on a retransmission? Clients are not supposed to leak random data
> to the server...
>
The Linux client routines are actually set up to fill this with 'X'
characters, not nulls. See gss_krb5_aes_encrypt, which does this:
memset(ecptr, 'X', ec);
AFAICT, the spec doesn't actually say what's supposed to be in the
padding, just that it's to be ignored. I imagine that most clients will
just set this to a constant value like Linux does, but there doesn't
seem to be any guarantee of that, and there's no real point in keeping
it around.
--
Jeff Layton <jlayton@xxxxxxxxxx>
--
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
