Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: Olga Kornievskaia <olga.kornievskaia@gmail.com>
Cc: Trond Myklebust <trond.myklebust@hammerspace.com>,
	Anna Schumaker <anna.schumaker@netapp.com>,
	Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH 1/1] SUNRPC: fix krb5p mount to provide large enough buffer in rq_rcvsize
Date: Thu, 26 Mar 2020 09:59:51 -0400
Message-ID: <F59C7AF3-E11B-47BB-BD92-38EDF7C9B07C@oracle.com> (raw)
In-Reply-To: <CAN-5tyEDRQZX-saVrbfn7G7pzmSOyROipEzxjVPxF1WV8rK+vg@mail.gmail.com>



> On Mar 26, 2020, at 8:04 AM, Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote:
> 
> On Wed, Mar 25, 2020 at 5:34 PM Chuck Lever <chuck.lever@oracle.com> wrote:
>> 
>> 
>> 
>>> On Mar 25, 2020, at 5:01 PM, Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote:
>>> 
>>> From: Olga Kornievskaia <kolga@netapp.com>
>>> 
>>> Ever since commit 2c94b8eca1a2 ("SUNRPC: Use au_rslack when computing
>>> reply buffer size"). It changed how "req->rq_rcvsize" is calculated. It
>>> used to use au_cslack value which was nice and large and changed it to
>>> au_rslack value which turns out to be too small.
>>> 
>>> Since 5.1, v3 mount with sec=krb5p fails against an Ontap server
>>> because client's receive buffer it too small.
>>> 
>>> For gss krb5p, we need to account for the mic token in the verifier,
>>> and the wrap token in the wrap token.
>>> 
>>> RFC 4121 defines:
>>> mic token
>>> Octet no   Name        Description
>>>        --------------------------------------------------------------
>>>        0..1     TOK_ID     Identification field.  Tokens emitted by
>>>                            GSS_GetMIC() contain the hex value 04 04
>>>                            expressed in big-endian order in this
>>>                            field.
>>>        2        Flags      Attributes field, as described in section
>>>                            4.2.2.
>>>        3..7     Filler     Contains five octets of hex value FF.
>>>        8..15    SND_SEQ    Sequence number field in clear text,
>>>                            expressed in big-endian order.
>>>        16..last SGN_CKSUM  Checksum of the "to-be-signed" data and
>>>                            octet 0..15, as described in section 4.2.4.
>>> 
>>> that's 16bytes (GSS_KRB5_TOK_HDR_LEN) + chksum
>>> 
>>> wrap token
>>> Octet no   Name        Description
>>>        --------------------------------------------------------------
>>>         0..1     TOK_ID    Identification field.  Tokens emitted by
>>>                            GSS_Wrap() contain the hex value 05 04
>>>                            expressed in big-endian order in this
>>>                            field.
>>>         2        Flags     Attributes field, as described in section
>>>                            4.2.2.
>>>         3        Filler    Contains the hex value FF.
>>>         4..5     EC        Contains the "extra count" field, in big-
>>>                            endian order as described in section 4.2.3.
>>>         6..7     RRC       Contains the "right rotation count" in big-
>>>                            endian order, as described in section
>>>                            4.2.5.
>>>         8..15    SND_SEQ   Sequence number field in clear text,
>>>                            expressed in big-endian order.
>>>         16..last Data      Encrypted data for Wrap tokens with
>>>                            confidentiality, or plaintext data followed
>>>                            by the checksum for Wrap tokens without
>>>                            confidentiality, as described in section
>>>                            4.2.4.
>>> 
>>> Also 16bytes of header (GSS_KRB5_TOK_HDR_LEN), encrypted data, and cksum
>>> (other things like padding)
>>> 
>>> RFC 3961 defines known cksum sizes:
>>> Checksum type              sumtype        checksum         section or
>>>                               value            size         reference
>>>  ---------------------------------------------------------------------
>>>  CRC32                            1               4           6.1.3
>>>  rsa-md4                          2              16           6.1.2
>>>  rsa-md4-des                      3              24           6.2.5
>>>  des-mac                          4              16           6.2.7
>>>  des-mac-k                        5               8           6.2.8
>>>  rsa-md4-des-k                    6              16           6.2.6
>>>  rsa-md5                          7              16           6.1.1
>>>  rsa-md5-des                      8              24           6.2.4
>>>  rsa-md5-des3                     9              24             ??
>>>  sha1 (unkeyed)                  10              20             ??
>>>  hmac-sha1-des3-kd               12              20            6.3
>>>  hmac-sha1-des3                  13              20             ??
>>>  sha1 (unkeyed)                  14              20             ??
>>>  hmac-sha1-96-aes128             15              20         [KRB5-AES]
>>>  hmac-sha1-96-aes256             16              20         [KRB5-AES]
>>>  [reserved]                  0x8003               ?         [GSS-KRB5]
>>> 
>>> Linux kernel now mainly supports type 15,16 so max cksum size is 20bytes.
>>> (GSS_KRB5_MAX_CKSUM_LEN)
>>> 
>>> Re-use already existing define of GSS_KRB5_MAX_SLACK_NEEDED that's used
>>> for encoding the gss_wrap tokens (same tokens are used in reply).
>>> 
>>> Fixes: 2c94b8eca1a2 ("SUNRPC: Use au_rslack when computing reply buffer size")
>>> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
>>> ---
>>> net/sunrpc/auth_gss/auth_gss.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
>>> index 24ca861..5a733a6 100644
>>> --- a/net/sunrpc/auth_gss/auth_gss.c
>>> +++ b/net/sunrpc/auth_gss/auth_gss.c
>>> @@ -20,6 +20,7 @@
>>> #include <linux/sunrpc/clnt.h>
>>> #include <linux/sunrpc/auth.h>
>>> #include <linux/sunrpc/auth_gss.h>
>>> +#include <linux/sunrpc/gss_krb5.h>
>>> #include <linux/sunrpc/svcauth_gss.h>
>>> #include <linux/sunrpc/gss_err.h>
>>> #include <linux/workqueue.h>
>>> @@ -51,6 +52,8 @@
>>> /* length of a krb5 verifier (48), plus data added before arguments when
>>> * using integrity (two 4-byte integers): */
>>> #define GSS_VERF_SLACK                100
>>> +/* covers lengths of gss_unwrap() extra kerberos mic and wrap token */
>>> +#define GSS_RESP_SLACK               (GSS_KRB5_MAX_SLACK_NEEDED << 2)
>> 
>> GSS_KRB5_MAX_SLACK_NEEDED is already in bytes. Shouldn't need the "<< 2" here.
> 
> 
> Ok yes just for my own understanding I convinced myself that indeed
> "<<2" is not needed here because clnt.c will do rq_rcvsize is <<=2.
> 
> Now question: Do I even need to introduce GSS_RES_SLACK at all or
> perhaps just use GSS_KRB5_MAX_SLACK_NEEDED to initialize?

For the moment, Kerberos is the only supported security flavor,
so we're using that value without any other modification. I guess
the extra "#define GSS_RESP_SLACK" seems pointless in that case.

I'm OK with using the KRB5_MAX_SLACK macro directly if there's no
objection from others.


>>> static DEFINE_HASHTABLE(gss_auth_hash_table, 4);
>>> static DEFINE_SPINLOCK(gss_auth_hash_lock);
>>> @@ -1050,7 +1053,7 @@ static void gss_pipe_free(struct gss_pipe *p)
>>>              goto err_put_mech;
>>>      auth = &gss_auth->rpc_auth;
>>>      auth->au_cslack = GSS_CRED_SLACK >> 2;
>>> -     auth->au_rslack = GSS_VERF_SLACK >> 2;
>>> +     auth->au_rslack = GSS_RESP_SLACK >> 2;
>>>      auth->au_verfsize = GSS_VERF_SLACK >> 2;
>>>      auth->au_ralign = GSS_VERF_SLACK >> 2;
>>>      auth->au_flags = 0;
>>> --
>>> 1.8.3.1
>>> 
>> 
>> --
>> Chuck Lever

--
Chuck Lever




      reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 21:01 Olga Kornievskaia
2020-03-25 21:33 ` Trond Myklebust
2020-03-25 21:43   ` Chuck Lever
2020-03-25 21:52     ` Trond Myklebust
2020-03-25 21:55       ` Chuck Lever
2020-03-26 12:10         ` Olga Kornievskaia
2020-03-25 21:34 ` Chuck Lever
2020-03-26 12:04   ` Olga Kornievskaia
2020-03-26 13:59     ` Chuck Lever [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=F59C7AF3-E11B-47BB-BD92-38EDF7C9B07C@oracle.com \
    --to=chuck.lever@oracle.com \
    --cc=anna.schumaker@netapp.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=olga.kornievskaia@gmail.com \
    --cc=trond.myklebust@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git