Re: pynfs: fixed gssapi usage (RPCGSS) + GSS* tests for nfs4.0 server test

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 18, 2021 at 12:34:16AM +0200, Volodymyr Khomenko wrote:
> Please review attached patches for gssapi-related code of nfs4.0
> server test (pynfs).
> This is a continuation of previous work to make GSS tests work
> correctly with the recent gssapi python library (using python3).
> 
> $ nfs4.0/testserver.py server.fqdn:/export gss noGSS8 --security=krb5
> ...
> Command line asked for 8 of 673 tests
> Of those: 3 Skipped, 0 Failed, 0 Warned, 5 Passed
> 
> $ nfs4.0/testserver.py server.fqdn:/export GSS8 --security=krb5
> ...
> Command line asked for 1 of 673 tests
> Of those: 0 Skipped, 0 Failed, 0 Warned, 1 Passed
> 
> $ nfs4.0/testserver.py server.fqdn:/export gss noGSS8 --security=krb5i
> ...
> Command line asked for 8 of 673 tests
> Of those: 1 Skipped, 0 Failed, 0 Warned, 7 Passed

Looks good to me, thanks!

I've been worrying about these tests for a while, it's a great relief to
have someone looking into them.....

--b.

> 
> Thanks,
> Volodymyr Khomenko.

> From fe6e9fee4bd1979e2e02d9f46d76dace90b5909c Mon Sep 17 00:00:00 2001
> From: Volodymyr Khomenko <volodymyr@xxxxxxxxxxxx>
> Date: Thu, 18 Nov 2021 00:06:02 +0200
> Subject: [PATCH 2/2] Fixed gss flag for nfs4.0 server test (GSS* tests)
> 
> - Fixed scope/visibility issue of caught exception 'e' for try/except
> - Added OSError exception as a valid use-case for testBadGssSeqnum (aka GSS1):
>   server may close TCP connection as a possible reaction for bad GSS seq_num
> - Fixed python3 issues for concatenating bytes + string
> 
> Signed-off-by: Volodymyr Khomenko <volodymyr@xxxxxxxxxxxx>
> ---
>  nfs4.0/servertests/st_gss.py | 82 ++++++++++++++++++++++--------------
>  1 file changed, 50 insertions(+), 32 deletions(-)
> 
> diff --git a/nfs4.0/servertests/st_gss.py b/nfs4.0/servertests/st_gss.py
> index e10e849..f651e57 100644
> --- a/nfs4.0/servertests/st_gss.py
> +++ b/nfs4.0/servertests/st_gss.py
> @@ -78,6 +78,8 @@ def testBadGssSeqnum(t, env):
>              res = c.compound([op.putrootfh()])
>          except timeout:
>              success = True
> +        except OSError:
> +            success = True
>          if not success:
>              t.fail("Using old gss_seq_num %i should cause dropped reply" %
>                     (orig + 1))
> @@ -106,17 +108,19 @@ def testInconsistentGssSeqnum(t, env):
>  
>      try:
>          c.security.secure_data = bad_secure_data
> +        err = None
>          try:
>              res = c.compound([op.putrootfh()])
> -            e = "operation erroneously suceeding"
> +            err = "operation erroneously suceeding"
>          except rpc.RPCAcceptError as e:
>              if e.stat == rpc.GARBAGE_ARGS:
>                  # This is correct response
>                  return
> +            err = str(e)
>          except Exception as e:
> -            pass
> +            err = str(e)
>          t.fail("Using inconsistent gss_seq_nums in header and body of message "
> -               "should return GARBAGE_ARGS, instead got %s" % e)
> +               "should return GARBAGE_ARGS, instead got %s" % err)
>      finally:
>          c.security.secure_data = orig_funct
>  
> @@ -131,21 +135,23 @@ def testBadVerfChecksum(t, env):
>      orig_funct = c.security.make_verf
>      def bad_make_verf(data):
>          # Mess up verifier
> -        return orig_funct(data + "x")
> +        return orig_funct(data + b"x")
>  
>      try:
>          c.security.make_verf = bad_make_verf
> +        err = None
>          try:
>              res = c.compound([op.putrootfh()])
> -            e = "peration erroneously suceeding"
> +            err = "peration erroneously suceeding"
>          except rpc.RPCDeniedError as e:
>              if e.stat == rpc.AUTH_ERROR and e.astat == rpc.RPCSEC_GSS_CREDPROBLEM:
> -                # This is correct response
> -                return
> +               # This is correct response
> +               return
> +            err = str(e)
>          except Exception as e:
> -            pass
> +            err = str(e)
>          t.fail("Using bad verifier checksum in header "
> -               "should return RPCSEC_GSS_CREDPROBLEM, instead got %s" % e)
> +               "should return RPCSEC_GSS_CREDPROBLEM, instead got %s" % err)
>      finally:
>          c.security.make_verf = orig_funct
>  
> @@ -164,24 +170,26 @@ def testBadDataChecksum(t, env):
>          # Mess up checksum
>          data = orig_funct(data, seqnum)
>          if data[-4]:
> -            tail = chr(0) + data[-3:]
> +            tail = b'\x00' + data[-3:]
>          else:
> -            tail = chr(1) + data[-3:]
> +            tail = b'\x01' + data[-3:]
>          return data[:-4] + tail
>  
>      try:
>          c.security.secure_data = bad_secure_data
> +        err = None
>          try:
>              res = c.compound([op.putrootfh()])
> -            e = "operation erroneously suceeding"
> +            err = "operation erroneously suceeding"
>          except rpc.RPCAcceptError as e:
>              if e.stat == rpc.GARBAGE_ARGS:
>                  # This is correct response
>                  return
> +            err = str(e)
>          except Exception as e:
> -            pass
> +            err = str(e)
>          t.fail("Using bad data checksum for body of message "
> -               "should return GARBAGE_ARGS, instead got %s" % e)
> +               "should return GARBAGE_ARGS, instead got %s" % err)
>      finally:
>          c.security.secure_data = orig_funct
>  
> @@ -211,19 +219,22 @@ def testBadVersion(t, env):
>          c.security = BadGssHeader(orig, bad_version)
>          bad_versions = [0, 2, 3, 1024]
>          for version in bad_versions:
> +            err = None
>              try:
>                  res = c.compound([op.putrootfh()])
> -                e = "operation erroneously suceeding"
> +                err = "operation erroneously suceeding"
>              except rpc.RPCDeniedError as e:
>                  if e.stat == rpc.AUTH_ERROR and e.astat == rpc.AUTH_BADCRED:
>                      # This is correct response
> -                    e = None
> +                    pass
> +                else:
> +                    err = str(e)
>              except Exception as e:
> -                pass
> -            if e is not None:
> +                err = str(e)
> +            if err is not None:
>                  t.fail("Using bad gss version number %i "
>                         "should return AUTH_BADCRED, instead got %s" %
> -                       (version, e))
> +                       (version, err))
>      finally:
>          c.security = orig
>  
> @@ -238,17 +249,18 @@ def testHighSeqNum(t, env):
>      orig_seq = c.security.gss_seq_num
>      try:
>          c.security.gss_seq_num = gss.MAXSEQ + 1
> +        err = None
>          try:
>              res = c.compound([op.putrootfh()])
> -            e = "operation erroneously suceeding"
> +            err = "operation erroneously suceeding"
>          except rpc.RPCDeniedError as e:
>              if e.stat == rpc.AUTH_ERROR and e.astat == rpc.RPCSEC_GSS_CTXPROBLEM:
>                  # This is correct response
>                  return
>          except Exception as e:
> -            pass
> +            err = str(e)
>          t.fail("Using gss_seq_num over MAXSEQ "
> -               "should return RPCSEC_GSS_CTXPROBLEM, instead got %s" % e)
> +               "should return RPCSEC_GSS_CTXPROBLEM, instead got %s" % err)
>      finally:
>          c.security.gss_seq_num = orig_seq
>  
> @@ -274,21 +286,24 @@ def testBadProcedure(t, env):
>  
>      try:
>          c.security = BadGssHeader(orig, bad_proc)
> +        err = None
>          bad_procss = [4, 5, 1024]
>          for proc in bad_procss:
>              try:
>                  res = c.compound([op.putrootfh()])
> -                e = "operation erroneously suceeding"
> +                err = "operation erroneously suceeding"
>              except rpc.RPCDeniedError as e:
>                  if e.stat == rpc.AUTH_ERROR and e.astat == rpc.AUTH_BADCRED:
>                      # This is correct response
> -                    e = None
> +                    pass
> +                else:
> +                    err = str(e)
>              except Exception as e:
> -                pass
> -            if e is not None:
> +                err = str(e)
> +            if err is not None:
>                  t.fail("Using bad gss procedure number %i "
>                         "should return AUTH_BADCRED, instead got %s" %
> -                       (proc, e))
> +                       (proc, err))
>      finally:
>          c.security = orig
>  
> @@ -316,20 +331,23 @@ def testBadService(t, env):
>  
>      try:
>          c.security = BadGssHeader(orig, bad_service)
> +        err = None
>          bad_services = [0, 4, 5, 1024]
>          for service in bad_services:
>              try:
>                  res = c.compound([op.putrootfh()])
> -                e = "operation erroneously suceeding"
> +                err = "operation erroneously suceeding"
>              except rpc.RPCDeniedError as e:
>                  if e.stat == rpc.AUTH_ERROR and e.astat == rpc.AUTH_BADCRED:
>                      # This is correct response
> -                    e = None
> +                    pass
> +                else:
> +                    err = str(e)
>              except Exception as e:
> -                pass
> -            if e is not None:
> +                err = str(e)
> +            if err is not None:
>                  t.fail("Using bad gss service number %i "
>                         "should return AUTH_BADCRED, instead got %s" %
> -                       (service, e))
> +                       (service, err))
>      finally:
>          c.security = orig
> -- 
> 2.24.1
> 

> From 3f94e9aa811cfff033a8d5438b7679ce0fa55b73 Mon Sep 17 00:00:00 2001
> From: Volodymyr Khomenko <volodymyr@xxxxxxxxxxxx>
> Date: Wed, 17 Nov 2021 23:58:20 +0200
> Subject: [PATCH 1/2] Fixed gssapi usage (RPCGSS) for nfs4.0 server test
> 
> gssapi library used in the code has been changed and current code is not
> compatible with API of new library version.  Fixed the code
> to work with recent gssapi (tested with 1.6.2).
> Tested with krb5, krb5i and krb5p security, like this:
> nfs4.0/testserver.py server.fqdn:/export gss noGSS8 --security=krb5
> Note - GSS8 is known to cause the problems for other tests that run after it,
> however it's safe to run it alone.
> 
> Signed-off-by: Volodymyr Khomenko <volodymyr@xxxxxxxxxxxx>
> ---
>  nfs4.0/lib/rpc/rpcsec/sec_auth_gss.py | 116 ++++++++++----------------
>  1 file changed, 45 insertions(+), 71 deletions(-)
> 
> diff --git a/nfs4.0/lib/rpc/rpcsec/sec_auth_gss.py b/nfs4.0/lib/rpc/rpcsec/sec_auth_gss.py
> index 1b5eb93..6577fcf 100644
> --- a/nfs4.0/lib/rpc/rpcsec/sec_auth_gss.py
> +++ b/nfs4.0/lib/rpc/rpcsec/sec_auth_gss.py
> @@ -1,8 +1,8 @@
>  from .base import SecFlavor, SecError
>  from rpc.rpc_const import RPCSEC_GSS
>  from rpc.rpc_type import opaque_auth
> -from gss_const import *
> -import gss_pack
> +from .gss_const import *
> +from . import gss_pack
>  import gss_type
>  import gssapi
>  import threading
> @@ -125,50 +125,41 @@ class SecAuthGss(SecFlavor):
>      def initialize(self, client): # Note this is not thread safe
>          """Set seq_num, init, handle, and context"""
>          self.gss_seq_num = 0
> -        d = gssapi.importName("nfs@%s" % client.remotehost)
> -        if d['major'] != gssapi.GSS_S_COMPLETE:
> -            raise SecError("gssapi.importName returned: %s" % \
> -                  show_major(d['major']))
> -        name = d['name']
> -        # We need to send NULLPROCs with token from initSecContext
> -        good_major = [gssapi.GSS_S_COMPLETE, gssapi.GSS_S_CONTINUE_NEEDED]
> +        name = gssapi.Name("nfs@%s" % client.remotehost, gssapi.NameType.hostbased_service)
> +        # We need to send NULLPROCs with token from SecurityContext
> +        good_major = [GSS_S_COMPLETE, GSS_S_CONTINUE_NEEDED]
>          self.init = 1
> -        reply_token = None
> -        reply_major = ''
> -        context = None
> +        input_token = None
> +
> +        # RFC2203 5.2.2.  Context Creation Requests
> +        # When GSS_Init_sec_context() is called, the parameters
> +        # replay_det_req_flag and sequence_req_flag must be turned off.
> +
> +        # Note - by default, out_of_sequence_detection flag (sequence_req_flag) is used by gssapi.init_sec_context()
> +        # and we have 'An expected per-message token was not received' error (GSS_S_GAP_TOKEN).
> +        # To prevent this, we need to use default flags without out_of_sequence_detection bit.
> +        flags = gssapi.IntEnumFlagSet(gssapi.RequirementFlag, [gssapi.RequirementFlag.mutual_authentication])
> +        context = gssapi.SecurityContext(name=name, flags=flags)
>          while True:
> -            d = gssapi.initSecContext(name, context, reply_token)
> -            major = d['major']
> -            context = d['context']
> -            if major not in good_major:
> -                raise SecError("gssapi.initSecContext returned: %s" % \
> -                      show_major(major))
> -            if (major == gssapi.GSS_S_CONTINUE_NEEDED) and \
> -                   (reply_major == gssapi.GSS_S_COMPLETE):
> -                raise SecError("Unexpected GSS_S_COMPLETE from server")
> -            token = d['token']
> -            if reply_major != gssapi.GSS_S_COMPLETE:
> -                # FRED - sec 5.2.2 of RFC 2203 mentions possibility that
> -                # no token is returned.  But then how get handle?
> -                p = self.getpacker()
> -                p.reset()
> -                p.pack_opaque(token)
> -                data = p.get_buffer()
> -                reply = client.call(0, data)
> -                up = self.getunpacker()
> -                up.reset(reply)
> -                res = up.unpack_rpc_gss_init_res()
> -                up.done()
> -                reply_major = res.gss_major
> -                if reply_major not in good_major:
> -                    raise SecError("Server returned: %s" % \
> -                          show_major(reply_major))
> -                self.init = 2
> -                reply_token = res.gss_token
> -            if major == gssapi.GSS_S_COMPLETE:
> -                if reply_major != gssapi.GSS_S_COMPLETE:
> -                    raise SecError("Unexpected COMPLETE from client")
> +            # note - gssapi will raise an exception here automatically in case of failure
> +            output_token = context.step(input_token)
> +            if context.complete:
>                  break
> +            p = self.getpacker()
> +            p.reset()
> +            p.pack_opaque(output_token)
> +            data = p.get_buffer()
> +            reply = client.call(0, data)
> +            up = self.getunpacker()
> +            up.reset(reply)
> +            res = up.unpack_rpc_gss_init_res()
> +            up.done()
> +            reply_major = res.gss_major
> +            if reply_major not in good_major:
> +                raise SecError("Server returned: %s" % \
> +                    show_major(reply_major))
> +            self.init = 2
> +            input_token = res.gss_token
>          self.gss_context = context
>          self.gss_handle = res.handle
>          self.init = 0
> @@ -176,7 +167,7 @@ class SecAuthGss(SecFlavor):
>      def make_cred(self):
>          """Credential sent with each RPC call"""
>          if self.init == 1: # first call in context creation
> -            cred = self._make_cred_gss('', rpc_gss_svc_none, RPCSEC_GSS_INIT)
> +            cred = self._make_cred_gss(b'', rpc_gss_svc_none, RPCSEC_GSS_INIT)
>          elif self.init > 1: # subsequent calls in context creation
>              cred = self._make_cred_gss('', rpc_gss_svc_none,
>                                    RPCSEC_GSS_CONTINUE_INIT)
> @@ -237,12 +228,8 @@ class SecAuthGss(SecFlavor):
>          if self.init:
>              return self._none
>          else:
> -            d = gssapi.getMIC(self.gss_context, data)
> -            major = d['major']
> -            if major != gssapi.GSS_S_COMPLETE:
> -                raise SecError("gssapi.getMIC returned: %s" % \
> -                      show_major(major))
> -            return opaque_auth(RPCSEC_GSS, d['token'])
> +            token = self.gss_context.get_signature(data)
> +            return opaque_auth(RPCSEC_GSS, token)
>          
>      def _make_cred_gss(self, handle, service, gss_proc=RPCSEC_GSS_DATA, seq=0):
>          data = gss_type.rpc_gss_cred_vers_1_t(gss_proc, seq, service, handle)
> @@ -264,13 +251,10 @@ class SecAuthGss(SecFlavor):
>              p.reset()
>              p.pack_uint(gss_cred.seq_num)
>              data = p.get_buffer() + data
> -            d = gssapi.getMIC(self.gss_context, data)
> -            if d['major'] != gssapi.GSS_S_COMPLETE:
> -                raise SecError("gssapi.getMIC returned: %s" % \
> -                      show_major(d['major']))
> +            token = self.gss_context.get_signature(data)
>              p.reset()
>              p.pack_opaque(data)
> -            p.pack_opaque(d['token'])
> +            p.pack_opaque(token)
>              data = p.get_buffer()
>          elif gss_cred.service == rpc_gss_svc_privacy:
>              # data = opaque[wrap([gss_seq_num+data])]
> @@ -278,12 +262,9 @@ class SecAuthGss(SecFlavor):
>              p.reset()
>              p.pack_uint(gss_cred.seq_num)
>              data = p.get_buffer() + data
> -            d = gssapi.wrap(self.gss_context, data)
> -            if d['major'] != gssapi.GSS_S_COMPLETE:
> -                raise SecError("gssapi.wrap returned: %s" % \
> -                      show_major(d['major']))
> +            wrap_data = self.gss_context.wrap(data, encrypt=True)
>              p.reset()
> -            p.pack_opaque(d['msg'])
> +            p.pack_opaque(wrap_data.message)
>              data = p.get_buffer()
>          else:
>              # Not really necessary, should have already raised XDRError
> @@ -303,10 +284,7 @@ class SecAuthGss(SecFlavor):
>              data = p.unpack_opaque()
>              checksum = p.unpack_opaque()
>              p.done()
> -            d = gssapi.verifyMIC(self.gss_context, data, checksum)
> -            if d['major'] != gssapi.GSS_S_COMPLETE:
> -                raise SecError("gssapi.verifyMIC returned: %s" % \
> -                      show_major(d['major']))
> +            qop = self.gss_context.verify_signature(data, checksum)
>              p.reset(data)
>              seqnum = p.unpack_uint()
>              if seqnum != gss_cred.seq_num:
> @@ -320,11 +298,8 @@ class SecAuthGss(SecFlavor):
>              p.reset(data)
>              data = p.unpack_opaque()
>              p.done()
> -            d = gssapi.unwrap(self.gss_context, data)
> -            if d['major'] != gssapi.GSS_S_COMPLETE:
> -                raise SecError("gssapi.unwrap returned %s" % \
> -                      show_major(d['major']))
> -            p.reset(d['msg'])
> +            data, encrypted, qop = self.gss_context.unwrap(data)
> +            p.reset(data)
>              seqnum = p.unpack_uint()
>              if seqnum != gss_cred.seq_num:
>                  raise SecError(\
> @@ -373,8 +348,7 @@ class SecAuthGss(SecFlavor):
>              p = self.getpacker()
>              p.reset()
>              p.pack_uint(cred.seq_num)
> -            d = gssapi.verifyMIC(self.gss_context, p.get_buffer(), rverf.body)
> -            #print("Verify(%i):"%cred.seq_num, show_major(d['major']), show_minor(d['minor']))
> +            qop = self.gss_context.verify_signature(p.get_buffer(), rverf.body)
>              
>          else:
>              pass
> -- 
> 2.24.1
> 




[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux