> On Mar 26, 2020, at 8:04 AM, Olga Kornievskaia <olga.kornievskaia@xxxxxxxxx> wrote:
>
> On Wed, Mar 25, 2020 at 5:34 PM Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
>>
>>
>>
>>> On Mar 25, 2020, at 5:01 PM, Olga Kornievskaia <olga.kornievskaia@xxxxxxxxx> wrote:
>>>
>>> From: Olga Kornievskaia <kolga@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
>>> ---
>>> 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
