Message ID | CANkgwetkTUjj-bMrM4XTvk0vhGiJt3wNKPpRvzgTk-u7ZfrdXg@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | GSSAPI fix for pynfs nfs4.1 client code | expand |
On Thu, Sep 30, 2021 at 06:22:09PM +0300, Volodymyr Khomenko wrote: > commit b77dc49c775756f08bdd0c6ebbe67a96f0ffe41f > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > Date: Thu Sep 30 17:53:04 2021 +0300 > > Fixed GSSContext to start sequence numbering from 1 > > GSS sequence number 0 is usually used by NFS4 NULL request > during GSS context establishment (but ignored by server). > Client should never reuse GSS sequence number, so using > 0 for the next real operation (EXCHANGE_ID) is possible but > looks suspicious. Fixed the code so numbering for operations > is done from 1 to avoid confusion. So, I can verify that --security=krb5 works after this patch but not before, good. But why is that? As you say, the server is supposed to ignore the sequence number on context creation requests. And 0 is valid sequence number as far as I know. --b. > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > diff --git a/rpc/security.py b/rpc/security.py > index 0682f43..86f6592 100644 > --- a/rpc/security.py > +++ b/rpc/security.py > @@ -174,7 +174,9 @@ class GSSContext(object): > def __init__(self, context_ptr): > self.lock = threading.Lock() > self.ptr = context_ptr > - self.seqid = 0 # client - next seqid to use > + # Note - seqid=0 is usually used during GSS context establishment, > + # to have the unique number we need to use the next value now. > + self.seqid = 1 # client - next seqid to use > self.highest = 0 # server - highest seqid seen > self.seen = 0 # server - bitmask of seen requests > > commit a612cf9897f0fa5b5de94885e00ef9293e93ffa3 > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > Date: Thu Sep 30 16:29:07 2021 +0300 > > Fixed gssapi usage (RPCGSS) for nfs4.1 client > > 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: > ./nfs4.1/testserver.py server.fqdn:/export --maketree --security=krb5 all > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > diff --git a/rpc/security.py b/rpc/security.py > index fe4390c..0682f43 100644 > --- a/rpc/security.py > +++ b/rpc/security.py > @@ -10,6 +10,7 @@ from . import gss_type > from .gss_type import rpc_gss_init_res > try: > import gssapi > + from gssapi.raw.misc import GSSError > except ImportError: > print("Could not find gssapi module, proceeding without") > gssapi = None > @@ -242,11 +243,11 @@ class AuthGss(AuthNone): > > def init_cred(self, call, target="nfs@jupiter", source=None, oid=None): > # STUB - need intelligent way to set defaults > - good_major = [gssapi.GSS_S_COMPLETE, gssapi.GSS_S_CONTINUE_NEEDED] > + good_major = [GSS_S_COMPLETE, GSS_S_CONTINUE_NEEDED] > p = Packer() > up = GSSUnpacker('') > # Set target (of form nfs@SERVER) > - target = gssapi.Name(target, gssapi.NT_HOSTBASED_SERVICE) > + target = gssapi.Name(target, gssapi.NameType.hostbased_service) > # Set source (of form USERNAME) > if source is not None: > source = gssapi.Name(source, gssapi.NT_USER_NAME) > @@ -254,18 +255,26 @@ class AuthGss(AuthNone): > else: > # Just use default cred > gss_cred = None > - context = gssapi.Context() > - token = None > - handle = '' > + # 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=target, creds=gss_cred, flags=flags) > + input_token = None > + handle = b'' > proc = RPCSEC_GSS_INIT > - while True: > + while not context.complete: > # Call initSecContext. If it returns COMPLETE, we are done. > # If it returns CONTINUE_NEEDED, we must send d['token'] > # to the target, which will run it through acceptSecContext, > # and give us back a token we need to send through initSecContext. > # Repeat as necessary. > - token = context.init(target, token, gss_cred) > - if context.open: > + output_token = context.step(input_token) > + if context.complete: > # XXX if res.major == CONTINUE there is a bug in library code > # STUB - now what? Just use context? > # XXX need to use res.seq_window > @@ -277,16 +286,16 @@ class AuthGss(AuthNone): > gss_proc=proc) > proc = RPCSEC_GSS_CONTINUE_INIT > p.reset() > - p.pack_opaque(token) > + p.pack_opaque(output_token) > header, reply = call(p.get_buffer(), credinfo) > up.reset(reply) > res = up.unpack_rpc_gss_init_res() > up.done() > # res now holds relevent output from target's acceptSecContext call > if res.gss_major not in good_major: > - raise gssapi.Error(res.gss_major, res.gss_minor) > + raise GSSError(res.gss_major, res.gss_minor) > handle = res.handle # Should not change between calls > - token = res.gss_token # This needs to be sent to initSecContext > + input_token = res.gss_token # This needs to be sent to SecurityContext.step() > return CredInfo(self, context=handle) > > @staticmethod > @@ -361,7 +370,7 @@ class AuthGss(AuthNone): > except: > log_gss.exception("unsecure_data - initial unpacking") > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > - qop = context.verifyMIC(data, checksum) > + qop = context.verify_signature(data, checksum) > check_gssapi(qop) > data = pull_seqnum(data) > elif cred.service == rpc_gss_svc_privacy: > @@ -373,14 +382,14 @@ class AuthGss(AuthNone): > log_gss.exception("unsecure_data - initial unpacking") > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > # data, qop, conf = context.unwrap(data) > - data, qop = context.unwrap(data) > + data, encrypted, qop = context.unwrap(data) > check_gssapi(qop) > data = pull_seqnum(data) > else: > # Can't get here, but doesn't hurt > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > - except gssapi.Error as e: > - log_gss.warn("unsecure_data: gssapi call returned %s" % e.name) > + except GSSError as e: > + log_gss.warn("unsecure_data: gssapi call returned %s" % str(e)) > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > return data > > @@ -397,7 +406,7 @@ class AuthGss(AuthNone): > # data = opaque[gss_seq_num+data] + opaque[checksum] > p.pack_uint(cred.seq_num) > data = p.get_buffer() + data > - token = context.getMIC(data) # XXX BUG set qop > + token = context.get_signature(data) # XXX BUG set qop > p.reset() > p.pack_opaque(data) > p.pack_opaque(token) > @@ -406,16 +415,16 @@ class AuthGss(AuthNone): > # data = opaque[wrap([gss_seq_num+data])] > p.pack_uint(cred.seq_num) > data = p.get_buffer() + data > - token = context.wrap(data) # XXX BUG set qop > + wrap_res = context.wrap(data, encrypt=True) # XXX BUG set qop > p.reset() > - p.pack_opaque(token) > + p.pack_opaque(wrap_res.message) > data = p.get_buffer() > else: > # Can't get here, but doesn't hurt > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > - except gssapi.Error as e: > + except GSSError as e: > # XXX What now? > - log_gss.warn("secure_data: gssapi call returned %s" % e.name) > + log_gss.warn("secure_data: gssapi call returned %s" % str(e)) > raise > return data > > @@ -436,8 +445,8 @@ class AuthGss(AuthNone): > return rpclib.NULL_CRED > else: > data = self.partially_packed_header(xid, body) > - # XXX how handle gssapi.Error? > - token = self._get_context(body.cred.body.handle).getMIC(data) > + # XXX how handle GSSError? > + token = self._get_context(body.cred.body.handle).get_signature(data) > return opaque_auth(RPCSEC_GSS, token) > > def check_call_verf(self, xid, body): > @@ -448,10 +457,10 @@ class AuthGss(AuthNone): > return False > data = self.partially_packed_header(xid, body) > try: > - qop = self._get_context(body.cred.body.handle).verifyMIC(data, body.verf.body) > - except gssapi.Error as e: > + qop = self._get_context(body.cred.body.handle).verify_signature(data, body.verf.body) > + except GSSError as e: > log_gss.warn("Verifier checksum failed verification with %s" % > - e.name) > + str(e)) > return False > body.cred.body.qop = qop # XXX Where store this? > log_gss.debug("verifier checks out (qop=%i)" % qop) > @@ -522,10 +531,10 @@ class AuthGss(AuthNone): > context = self._get_context(cred.body.handle) > try: > token = context.accept(token) > - except gssapi.Error as e: > + except GSSError as e: > log_gss.debug("RPCSEC_GSS_INIT failed (%s, %i)!" % > - (e.name, e.minor)) > - res = rpc_gss_init_res('', e.major, e.minor, 0, '') > + (str(e), e.min_code)) > + res = rpc_gss_init_res('', e.maj_code, e.min_code, 0, '') > else: > log_gss.debug("RPCSEC_GSS_*INIT succeeded!") > if first: > @@ -538,9 +547,9 @@ class AuthGss(AuthNone): > else: > handle = cred.body.handle > if context.open: > - major = gssapi.GSS_S_COMPLETE > + major = GSS_S_COMPLETE > else: > - major = gssapi.GSS_S_CONTINUE_NEEDED > + major = GSS_S_CONTINUE_NEEDED > res = rpc_gss_init_res(handle, major, 0, # XXX can't see minor > WINDOWSIZE, token) > # Prepare response > @@ -559,15 +568,15 @@ class AuthGss(AuthNone): > # NOTE this relies on GSS_S_COMPLETE == rpc.SUCCESS == 0 > return rpclib.NULL_CRED > elif cred.gss_proc in (RPCSEC_GSS_INIT, RPCSEC_GSS_CONTINUE_INIT): > - # init requires getMIC(seq_window) > + # init requires get_signature(seq_window) > i = WINDOWSIZE > else: > - # Else return getMIC(cred.seq_num) > + # Else return get_signature(cred.seq_num) > i = cred.seq_num > p = Packer() > p.pack_uint(i) > # XXX BUG - need to set qop > - token = self._get_context(cred.handle).getMIC(p.get_buffer()) > + token = self._get_context(cred.handle).get_signature(p.get_buffer()) > return opaque_auth(RPCSEC_GSS, token) > > def check_reply_verf(self, msg, call_cred, data): > @@ -593,12 +602,12 @@ class AuthGss(AuthNone): > if res.gss_major != GSS_S_COMPLETE: > raise SecError("Expected NULL") > # BUG - context establishment is not finished on client > - # - so how get context? How run verifyMIC? > + # - so how get context? How run verify_signature? > # - This seems to be a protocol problem. Just ignore for now > else: > p = Packer() > p.pack_uint(call_cred.body.seq_num) > - qop = call_cred.context.verifyMIC(p.get_buffer(), verf.body) > + qop = call_cred.context.verify_signature(p.get_buffer(), verf.body) > if qop != call_cred.body.qop: > raise SecError("Mismatched qop") >
On Thu, Sep 30, 2021 at 05:11:23PM -0400, J. Bruce Fields wrote: > On Thu, Sep 30, 2021 at 06:22:09PM +0300, Volodymyr Khomenko wrote: > > commit b77dc49c775756f08bdd0c6ebbe67a96f0ffe41f > > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > > Date: Thu Sep 30 17:53:04 2021 +0300 > > > > Fixed GSSContext to start sequence numbering from 1 > > > > GSS sequence number 0 is usually used by NFS4 NULL request > > during GSS context establishment (but ignored by server). > > Client should never reuse GSS sequence number, so using > > 0 for the next real operation (EXCHANGE_ID) is possible but > > looks suspicious. Fixed the code so numbering for operations > > is done from 1 to avoid confusion. > > So, I can verify that --security=krb5 works after this patch but not > before, good. But why is that? As you say, the server is supposed to > ignore the sequence number on context creation requests. And 0 is valid > sequence number as far as I know. Looking at the network--my server's not responding to the first data message. I think the Linux server just has a bug. I'll make a patch.... --b.
> So, I can verify that --security=krb5 works after this patch but not > before, good. But why is that? As you say, the server is supposed to > ignore the sequence number on context creation requests. And 0 is valid > sequence number as far as I know. It is very confusing to have two different requests with the same GSS sequence number. Yes, one of them is ignored and technically this is possible, but it makes more mess in the traffic and it's more hard to analyze it manually. Moveover, Linux client does the same (EXCHANGE_ID comes with seq_num=1) when mounting NFS4 - I guess this is a good approach to follow. volodymyr. On Fri, Oct 1, 2021 at 12:11 AM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Thu, Sep 30, 2021 at 06:22:09PM +0300, Volodymyr Khomenko wrote: > > commit b77dc49c775756f08bdd0c6ebbe67a96f0ffe41f > > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > > Date: Thu Sep 30 17:53:04 2021 +0300 > > > > Fixed GSSContext to start sequence numbering from 1 > > > > GSS sequence number 0 is usually used by NFS4 NULL request > > during GSS context establishment (but ignored by server). > > Client should never reuse GSS sequence number, so using > > 0 for the next real operation (EXCHANGE_ID) is possible but > > looks suspicious. Fixed the code so numbering for operations > > is done from 1 to avoid confusion. > > So, I can verify that --security=krb5 works after this patch but not > before, good. But why is that? As you say, the server is supposed to > ignore the sequence number on context creation requests. And 0 is valid > sequence number as far as I know. > > --b. > > > > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > > > diff --git a/rpc/security.py b/rpc/security.py > > index 0682f43..86f6592 100644 > > --- a/rpc/security.py > > +++ b/rpc/security.py > > @@ -174,7 +174,9 @@ class GSSContext(object): > > def __init__(self, context_ptr): > > self.lock = threading.Lock() > > self.ptr = context_ptr > > - self.seqid = 0 # client - next seqid to use > > + # Note - seqid=0 is usually used during GSS context establishment, > > + # to have the unique number we need to use the next value now. > > + self.seqid = 1 # client - next seqid to use > > self.highest = 0 # server - highest seqid seen > > self.seen = 0 # server - bitmask of seen requests > > > > > commit a612cf9897f0fa5b5de94885e00ef9293e93ffa3 > > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > > Date: Thu Sep 30 16:29:07 2021 +0300 > > > > Fixed gssapi usage (RPCGSS) for nfs4.1 client > > > > 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: > > ./nfs4.1/testserver.py server.fqdn:/export --maketree --security=krb5 all > > > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > > > diff --git a/rpc/security.py b/rpc/security.py > > index fe4390c..0682f43 100644 > > --- a/rpc/security.py > > +++ b/rpc/security.py > > @@ -10,6 +10,7 @@ from . import gss_type > > from .gss_type import rpc_gss_init_res > > try: > > import gssapi > > + from gssapi.raw.misc import GSSError > > except ImportError: > > print("Could not find gssapi module, proceeding without") > > gssapi = None > > @@ -242,11 +243,11 @@ class AuthGss(AuthNone): > > > > def init_cred(self, call, target="nfs@jupiter", source=None, oid=None): > > # STUB - need intelligent way to set defaults > > - good_major = [gssapi.GSS_S_COMPLETE, gssapi.GSS_S_CONTINUE_NEEDED] > > + good_major = [GSS_S_COMPLETE, GSS_S_CONTINUE_NEEDED] > > p = Packer() > > up = GSSUnpacker('') > > # Set target (of form nfs@SERVER) > > - target = gssapi.Name(target, gssapi.NT_HOSTBASED_SERVICE) > > + target = gssapi.Name(target, gssapi.NameType.hostbased_service) > > # Set source (of form USERNAME) > > if source is not None: > > source = gssapi.Name(source, gssapi.NT_USER_NAME) > > @@ -254,18 +255,26 @@ class AuthGss(AuthNone): > > else: > > # Just use default cred > > gss_cred = None > > - context = gssapi.Context() > > - token = None > > - handle = '' > > + # 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=target, creds=gss_cred, flags=flags) > > + input_token = None > > + handle = b'' > > proc = RPCSEC_GSS_INIT > > - while True: > > + while not context.complete: > > # Call initSecContext. If it returns COMPLETE, we are done. > > # If it returns CONTINUE_NEEDED, we must send d['token'] > > # to the target, which will run it through acceptSecContext, > > # and give us back a token we need to send through initSecContext. > > # Repeat as necessary. > > - token = context.init(target, token, gss_cred) > > - if context.open: > > + output_token = context.step(input_token) > > + if context.complete: > > # XXX if res.major == CONTINUE there is a bug in library code > > # STUB - now what? Just use context? > > # XXX need to use res.seq_window > > @@ -277,16 +286,16 @@ class AuthGss(AuthNone): > > gss_proc=proc) > > proc = RPCSEC_GSS_CONTINUE_INIT > > p.reset() > > - p.pack_opaque(token) > > + p.pack_opaque(output_token) > > header, reply = call(p.get_buffer(), credinfo) > > up.reset(reply) > > res = up.unpack_rpc_gss_init_res() > > up.done() > > # res now holds relevent output from target's acceptSecContext call > > if res.gss_major not in good_major: > > - raise gssapi.Error(res.gss_major, res.gss_minor) > > + raise GSSError(res.gss_major, res.gss_minor) > > handle = res.handle # Should not change between calls > > - token = res.gss_token # This needs to be sent to initSecContext > > + input_token = res.gss_token # This needs to be sent to SecurityContext.step() > > return CredInfo(self, context=handle) > > > > @staticmethod > > @@ -361,7 +370,7 @@ class AuthGss(AuthNone): > > except: > > log_gss.exception("unsecure_data - initial unpacking") > > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > > - qop = context.verifyMIC(data, checksum) > > + qop = context.verify_signature(data, checksum) > > check_gssapi(qop) > > data = pull_seqnum(data) > > elif cred.service == rpc_gss_svc_privacy: > > @@ -373,14 +382,14 @@ class AuthGss(AuthNone): > > log_gss.exception("unsecure_data - initial unpacking") > > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > > # data, qop, conf = context.unwrap(data) > > - data, qop = context.unwrap(data) > > + data, encrypted, qop = context.unwrap(data) > > check_gssapi(qop) > > data = pull_seqnum(data) > > else: > > # Can't get here, but doesn't hurt > > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > > - except gssapi.Error as e: > > - log_gss.warn("unsecure_data: gssapi call returned %s" % e.name) > > + except GSSError as e: > > + log_gss.warn("unsecure_data: gssapi call returned %s" % str(e)) > > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > > return data > > > > @@ -397,7 +406,7 @@ class AuthGss(AuthNone): > > # data = opaque[gss_seq_num+data] + opaque[checksum] > > p.pack_uint(cred.seq_num) > > data = p.get_buffer() + data > > - token = context.getMIC(data) # XXX BUG set qop > > + token = context.get_signature(data) # XXX BUG set qop > > p.reset() > > p.pack_opaque(data) > > p.pack_opaque(token) > > @@ -406,16 +415,16 @@ class AuthGss(AuthNone): > > # data = opaque[wrap([gss_seq_num+data])] > > p.pack_uint(cred.seq_num) > > data = p.get_buffer() + data > > - token = context.wrap(data) # XXX BUG set qop > > + wrap_res = context.wrap(data, encrypt=True) # XXX BUG set qop > > p.reset() > > - p.pack_opaque(token) > > + p.pack_opaque(wrap_res.message) > > data = p.get_buffer() > > else: > > # Can't get here, but doesn't hurt > > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > > - except gssapi.Error as e: > > + except GSSError as e: > > # XXX What now? > > - log_gss.warn("secure_data: gssapi call returned %s" % e.name) > > + log_gss.warn("secure_data: gssapi call returned %s" % str(e)) > > raise > > return data > > > > @@ -436,8 +445,8 @@ class AuthGss(AuthNone): > > return rpclib.NULL_CRED > > else: > > data = self.partially_packed_header(xid, body) > > - # XXX how handle gssapi.Error? > > - token = self._get_context(body.cred.body.handle).getMIC(data) > > + # XXX how handle GSSError? > > + token = self._get_context(body.cred.body.handle).get_signature(data) > > return opaque_auth(RPCSEC_GSS, token) > > > > def check_call_verf(self, xid, body): > > @@ -448,10 +457,10 @@ class AuthGss(AuthNone): > > return False > > data = self.partially_packed_header(xid, body) > > try: > > - qop = self._get_context(body.cred.body.handle).verifyMIC(data, body.verf.body) > > - except gssapi.Error as e: > > + qop = self._get_context(body.cred.body.handle).verify_signature(data, body.verf.body) > > + except GSSError as e: > > log_gss.warn("Verifier checksum failed verification with %s" % > > - e.name) > > + str(e)) > > return False > > body.cred.body.qop = qop # XXX Where store this? > > log_gss.debug("verifier checks out (qop=%i)" % qop) > > @@ -522,10 +531,10 @@ class AuthGss(AuthNone): > > context = self._get_context(cred.body.handle) > > try: > > token = context.accept(token) > > - except gssapi.Error as e: > > + except GSSError as e: > > log_gss.debug("RPCSEC_GSS_INIT failed (%s, %i)!" % > > - (e.name, e.minor)) > > - res = rpc_gss_init_res('', e.major, e.minor, 0, '') > > + (str(e), e.min_code)) > > + res = rpc_gss_init_res('', e.maj_code, e.min_code, 0, '') > > else: > > log_gss.debug("RPCSEC_GSS_*INIT succeeded!") > > if first: > > @@ -538,9 +547,9 @@ class AuthGss(AuthNone): > > else: > > handle = cred.body.handle > > if context.open: > > - major = gssapi.GSS_S_COMPLETE > > + major = GSS_S_COMPLETE > > else: > > - major = gssapi.GSS_S_CONTINUE_NEEDED > > + major = GSS_S_CONTINUE_NEEDED > > res = rpc_gss_init_res(handle, major, 0, # XXX can't see minor > > WINDOWSIZE, token) > > # Prepare response > > @@ -559,15 +568,15 @@ class AuthGss(AuthNone): > > # NOTE this relies on GSS_S_COMPLETE == rpc.SUCCESS == 0 > > return rpclib.NULL_CRED > > elif cred.gss_proc in (RPCSEC_GSS_INIT, RPCSEC_GSS_CONTINUE_INIT): > > - # init requires getMIC(seq_window) > > + # init requires get_signature(seq_window) > > i = WINDOWSIZE > > else: > > - # Else return getMIC(cred.seq_num) > > + # Else return get_signature(cred.seq_num) > > i = cred.seq_num > > p = Packer() > > p.pack_uint(i) > > # XXX BUG - need to set qop > > - token = self._get_context(cred.handle).getMIC(p.get_buffer()) > > + token = self._get_context(cred.handle).get_signature(p.get_buffer()) > > return opaque_auth(RPCSEC_GSS, token) > > > > def check_reply_verf(self, msg, call_cred, data): > > @@ -593,12 +602,12 @@ class AuthGss(AuthNone): > > if res.gss_major != GSS_S_COMPLETE: > > raise SecError("Expected NULL") > > # BUG - context establishment is not finished on client > > - # - so how get context? How run verifyMIC? > > + # - so how get context? How run verify_signature? > > # - This seems to be a protocol problem. Just ignore for now > > else: > > p = Packer() > > p.pack_uint(call_cred.body.seq_num) > > - qop = call_cred.context.verifyMIC(p.get_buffer(), verf.body) > > + qop = call_cred.context.verify_signature(p.get_buffer(), verf.body) > > if qop != call_cred.body.qop: > > raise SecError("Mismatched qop") > > >
> Looking at the network--my server's not responding to the first data > message. I see that linux client (when doing mount -t nfs -o "vers=4.1,sec=krb") always does the same (i.e. the very 1st packet with EXCHANGE_ID operation comes with GSS sequence number=1, then CREATE_SESSION uses seq_num=2, and so on). If your server works normally with a regular mount, I don't think that my fix is not related at all to this problem volodymyr. On Fri, Oct 1, 2021 at 12:25 AM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Thu, Sep 30, 2021 at 05:11:23PM -0400, J. Bruce Fields wrote: > > On Thu, Sep 30, 2021 at 06:22:09PM +0300, Volodymyr Khomenko wrote: > > > commit b77dc49c775756f08bdd0c6ebbe67a96f0ffe41f > > > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > > > Date: Thu Sep 30 17:53:04 2021 +0300 > > > > > > Fixed GSSContext to start sequence numbering from 1 > > > > > > GSS sequence number 0 is usually used by NFS4 NULL request > > > during GSS context establishment (but ignored by server). > > > Client should never reuse GSS sequence number, so using > > > 0 for the next real operation (EXCHANGE_ID) is possible but > > > looks suspicious. Fixed the code so numbering for operations > > > is done from 1 to avoid confusion. > > > > So, I can verify that --security=krb5 works after this patch but not > > before, good. But why is that? As you say, the server is supposed to > > ignore the sequence number on context creation requests. And 0 is valid > > sequence number as far as I know. > > Looking at the network--my server's not responding to the first data > message. > > I think the Linux server just has a bug. I'll make a patch.... > > --b.
> So, I can verify that --security=krb5 works after this patch but not > before, good. But why is that? As you say, the server is supposed to > ignore the sequence number on context creation requests. And 0 is valid > sequence number as far as I know. By design of RPCGSS we have a 'last seen seq_number' counter on the server side per each GSS context and we must not accept any packet that was already seen before (we also have a bitmask of several last requests for this). This 'last seen counter' is unsigned int32 (the same as seq_num) so we can't just init it to -1 so next seq_num=0 will be valid. The most obvious implementation is to init it last_seen_seq_num to 0 so the very 1st packet after NFS4 NULL must be 1 to differ from last seen seq_number. A better implementation (theoretically) can set this counter to 'undefined' state by additional flag, but this is more resource-consuming (you need to process is_inited flag + last_seen_seq_num instead of just one counter). If the last seen seq_number is undefined during GSS initialization, then strictly speaking we can send ANY seq_num for the very 1st request after NFS4 NULL. volodymyr. On Fri, Oct 1, 2021 at 12:11 AM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Thu, Sep 30, 2021 at 06:22:09PM +0300, Volodymyr Khomenko wrote: > > commit b77dc49c775756f08bdd0c6ebbe67a96f0ffe41f > > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > > Date: Thu Sep 30 17:53:04 2021 +0300 > > > > Fixed GSSContext to start sequence numbering from 1 > > > > GSS sequence number 0 is usually used by NFS4 NULL request > > during GSS context establishment (but ignored by server). > > Client should never reuse GSS sequence number, so using > > 0 for the next real operation (EXCHANGE_ID) is possible but > > looks suspicious. Fixed the code so numbering for operations > > is done from 1 to avoid confusion. > > So, I can verify that --security=krb5 works after this patch but not > before, good. But why is that? As you say, the server is supposed to > ignore the sequence number on context creation requests. And 0 is valid > sequence number as far as I know. > > --b. > > > > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > > > diff --git a/rpc/security.py b/rpc/security.py > > index 0682f43..86f6592 100644 > > --- a/rpc/security.py > > +++ b/rpc/security.py > > @@ -174,7 +174,9 @@ class GSSContext(object): > > def __init__(self, context_ptr): > > self.lock = threading.Lock() > > self.ptr = context_ptr > > - self.seqid = 0 # client - next seqid to use > > + # Note - seqid=0 is usually used during GSS context establishment, > > + # to have the unique number we need to use the next value now. > > + self.seqid = 1 # client - next seqid to use > > self.highest = 0 # server - highest seqid seen > > self.seen = 0 # server - bitmask of seen requests > > > > > commit a612cf9897f0fa5b5de94885e00ef9293e93ffa3 > > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > > Date: Thu Sep 30 16:29:07 2021 +0300 > > > > Fixed gssapi usage (RPCGSS) for nfs4.1 client > > > > 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: > > ./nfs4.1/testserver.py server.fqdn:/export --maketree --security=krb5 all > > > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > > > diff --git a/rpc/security.py b/rpc/security.py > > index fe4390c..0682f43 100644 > > --- a/rpc/security.py > > +++ b/rpc/security.py > > @@ -10,6 +10,7 @@ from . import gss_type > > from .gss_type import rpc_gss_init_res > > try: > > import gssapi > > + from gssapi.raw.misc import GSSError > > except ImportError: > > print("Could not find gssapi module, proceeding without") > > gssapi = None > > @@ -242,11 +243,11 @@ class AuthGss(AuthNone): > > > > def init_cred(self, call, target="nfs@jupiter", source=None, oid=None): > > # STUB - need intelligent way to set defaults > > - good_major = [gssapi.GSS_S_COMPLETE, gssapi.GSS_S_CONTINUE_NEEDED] > > + good_major = [GSS_S_COMPLETE, GSS_S_CONTINUE_NEEDED] > > p = Packer() > > up = GSSUnpacker('') > > # Set target (of form nfs@SERVER) > > - target = gssapi.Name(target, gssapi.NT_HOSTBASED_SERVICE) > > + target = gssapi.Name(target, gssapi.NameType.hostbased_service) > > # Set source (of form USERNAME) > > if source is not None: > > source = gssapi.Name(source, gssapi.NT_USER_NAME) > > @@ -254,18 +255,26 @@ class AuthGss(AuthNone): > > else: > > # Just use default cred > > gss_cred = None > > - context = gssapi.Context() > > - token = None > > - handle = '' > > + # 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=target, creds=gss_cred, flags=flags) > > + input_token = None > > + handle = b'' > > proc = RPCSEC_GSS_INIT > > - while True: > > + while not context.complete: > > # Call initSecContext. If it returns COMPLETE, we are done. > > # If it returns CONTINUE_NEEDED, we must send d['token'] > > # to the target, which will run it through acceptSecContext, > > # and give us back a token we need to send through initSecContext. > > # Repeat as necessary. > > - token = context.init(target, token, gss_cred) > > - if context.open: > > + output_token = context.step(input_token) > > + if context.complete: > > # XXX if res.major == CONTINUE there is a bug in library code > > # STUB - now what? Just use context? > > # XXX need to use res.seq_window > > @@ -277,16 +286,16 @@ class AuthGss(AuthNone): > > gss_proc=proc) > > proc = RPCSEC_GSS_CONTINUE_INIT > > p.reset() > > - p.pack_opaque(token) > > + p.pack_opaque(output_token) > > header, reply = call(p.get_buffer(), credinfo) > > up.reset(reply) > > res = up.unpack_rpc_gss_init_res() > > up.done() > > # res now holds relevent output from target's acceptSecContext call > > if res.gss_major not in good_major: > > - raise gssapi.Error(res.gss_major, res.gss_minor) > > + raise GSSError(res.gss_major, res.gss_minor) > > handle = res.handle # Should not change between calls > > - token = res.gss_token # This needs to be sent to initSecContext > > + input_token = res.gss_token # This needs to be sent to SecurityContext.step() > > return CredInfo(self, context=handle) > > > > @staticmethod > > @@ -361,7 +370,7 @@ class AuthGss(AuthNone): > > except: > > log_gss.exception("unsecure_data - initial unpacking") > > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > > - qop = context.verifyMIC(data, checksum) > > + qop = context.verify_signature(data, checksum) > > check_gssapi(qop) > > data = pull_seqnum(data) > > elif cred.service == rpc_gss_svc_privacy: > > @@ -373,14 +382,14 @@ class AuthGss(AuthNone): > > log_gss.exception("unsecure_data - initial unpacking") > > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > > # data, qop, conf = context.unwrap(data) > > - data, qop = context.unwrap(data) > > + data, encrypted, qop = context.unwrap(data) > > check_gssapi(qop) > > data = pull_seqnum(data) > > else: > > # Can't get here, but doesn't hurt > > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > > - except gssapi.Error as e: > > - log_gss.warn("unsecure_data: gssapi call returned %s" % e.name) > > + except GSSError as e: > > + log_gss.warn("unsecure_data: gssapi call returned %s" % str(e)) > > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > > return data > > > > @@ -397,7 +406,7 @@ class AuthGss(AuthNone): > > # data = opaque[gss_seq_num+data] + opaque[checksum] > > p.pack_uint(cred.seq_num) > > data = p.get_buffer() + data > > - token = context.getMIC(data) # XXX BUG set qop > > + token = context.get_signature(data) # XXX BUG set qop > > p.reset() > > p.pack_opaque(data) > > p.pack_opaque(token) > > @@ -406,16 +415,16 @@ class AuthGss(AuthNone): > > # data = opaque[wrap([gss_seq_num+data])] > > p.pack_uint(cred.seq_num) > > data = p.get_buffer() + data > > - token = context.wrap(data) # XXX BUG set qop > > + wrap_res = context.wrap(data, encrypt=True) # XXX BUG set qop > > p.reset() > > - p.pack_opaque(token) > > + p.pack_opaque(wrap_res.message) > > data = p.get_buffer() > > else: > > # Can't get here, but doesn't hurt > > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > > - except gssapi.Error as e: > > + except GSSError as e: > > # XXX What now? > > - log_gss.warn("secure_data: gssapi call returned %s" % e.name) > > + log_gss.warn("secure_data: gssapi call returned %s" % str(e)) > > raise > > return data > > > > @@ -436,8 +445,8 @@ class AuthGss(AuthNone): > > return rpclib.NULL_CRED > > else: > > data = self.partially_packed_header(xid, body) > > - # XXX how handle gssapi.Error? > > - token = self._get_context(body.cred.body.handle).getMIC(data) > > + # XXX how handle GSSError? > > + token = self._get_context(body.cred.body.handle).get_signature(data) > > return opaque_auth(RPCSEC_GSS, token) > > > > def check_call_verf(self, xid, body): > > @@ -448,10 +457,10 @@ class AuthGss(AuthNone): > > return False > > data = self.partially_packed_header(xid, body) > > try: > > - qop = self._get_context(body.cred.body.handle).verifyMIC(data, body.verf.body) > > - except gssapi.Error as e: > > + qop = self._get_context(body.cred.body.handle).verify_signature(data, body.verf.body) > > + except GSSError as e: > > log_gss.warn("Verifier checksum failed verification with %s" % > > - e.name) > > + str(e)) > > return False > > body.cred.body.qop = qop # XXX Where store this? > > log_gss.debug("verifier checks out (qop=%i)" % qop) > > @@ -522,10 +531,10 @@ class AuthGss(AuthNone): > > context = self._get_context(cred.body.handle) > > try: > > token = context.accept(token) > > - except gssapi.Error as e: > > + except GSSError as e: > > log_gss.debug("RPCSEC_GSS_INIT failed (%s, %i)!" % > > - (e.name, e.minor)) > > - res = rpc_gss_init_res('', e.major, e.minor, 0, '') > > + (str(e), e.min_code)) > > + res = rpc_gss_init_res('', e.maj_code, e.min_code, 0, '') > > else: > > log_gss.debug("RPCSEC_GSS_*INIT succeeded!") > > if first: > > @@ -538,9 +547,9 @@ class AuthGss(AuthNone): > > else: > > handle = cred.body.handle > > if context.open: > > - major = gssapi.GSS_S_COMPLETE > > + major = GSS_S_COMPLETE > > else: > > - major = gssapi.GSS_S_CONTINUE_NEEDED > > + major = GSS_S_CONTINUE_NEEDED > > res = rpc_gss_init_res(handle, major, 0, # XXX can't see minor > > WINDOWSIZE, token) > > # Prepare response > > @@ -559,15 +568,15 @@ class AuthGss(AuthNone): > > # NOTE this relies on GSS_S_COMPLETE == rpc.SUCCESS == 0 > > return rpclib.NULL_CRED > > elif cred.gss_proc in (RPCSEC_GSS_INIT, RPCSEC_GSS_CONTINUE_INIT): > > - # init requires getMIC(seq_window) > > + # init requires get_signature(seq_window) > > i = WINDOWSIZE > > else: > > - # Else return getMIC(cred.seq_num) > > + # Else return get_signature(cred.seq_num) > > i = cred.seq_num > > p = Packer() > > p.pack_uint(i) > > # XXX BUG - need to set qop > > - token = self._get_context(cred.handle).getMIC(p.get_buffer()) > > + token = self._get_context(cred.handle).get_signature(p.get_buffer()) > > return opaque_auth(RPCSEC_GSS, token) > > > > def check_reply_verf(self, msg, call_cred, data): > > @@ -593,12 +602,12 @@ class AuthGss(AuthNone): > > if res.gss_major != GSS_S_COMPLETE: > > raise SecError("Expected NULL") > > # BUG - context establishment is not finished on client > > - # - so how get context? How run verifyMIC? > > + # - so how get context? How run verify_signature? > > # - This seems to be a protocol problem. Just ignore for now > > else: > > p = Packer() > > p.pack_uint(call_cred.body.seq_num) > > - qop = call_cred.context.verifyMIC(p.get_buffer(), verf.body) > > + qop = call_cred.context.verify_signature(p.get_buffer(), verf.body) > > if qop != call_cred.body.qop: > > raise SecError("Mismatched qop") > > >
On Fri, Oct 01, 2021 at 09:49:50AM +0300, Volodymyr Khomenko wrote: > > So, I can verify that --security=krb5 works after this patch but not > > before, good. But why is that? As you say, the server is supposed to > > ignore the sequence number on context creation requests. And 0 is valid > > sequence number as far as I know. > > By design of RPCGSS we have a 'last seen seq_number' counter on the > server side per each GSS context > and we must not accept any packet that was already seen before (we > also have a bitmask of several last requests for this). > This 'last seen counter' is unsigned int32 (the same as seq_num) so we > can't just init it to -1 so next seq_num=0 will be valid. > The most obvious implementation is to init it last_seen_seq_num to 0 > so the very 1st packet after NFS4 NULL must be 1 to differ from last > seen seq_number. Note in theory gssapi mechanisms can require multiple round trips (in the GSS_PROC_CONTINUE_INIT case), so this wouldn't actually avoid duplicate sequence numbers. In any case, the rfc is unambiguous here: "In a creation request, the seq_num and service fields are undefined and both must be ignored by the server." > A better implementation (theoretically) can set this counter to > 'undefined' state by additional flag, but this is more > resource-consuming > (you need to process is_inited flag + last_seen_seq_num instead of > just one counter). > If the last seen seq_number is undefined during GSS initialization, > then strictly speaking we can send ANY seq_num for the very 1st > request after NFS4 NULL. Right, again, from RFC 2203, " The seq_num field can start at any value below MAXSEQ." It can be implemented without the need for an is_inited flag. The initial sequence number of 0 really did find an actual bug in the server, so pynfs is definitely doing its job in this case! --b.
> The seq_num field can start at any value below MAXSEQ Yes, that's the statement I haven't found before in RFC... Probably we also need to write a test starting the seq_num from a big value (more than SEQUENCE_WINDOW) to make sure that it is really implemented properly without 'is_inited' flag (so what's the initial value?). However I still propose to keep the default behaviour of pynfs to be the same as for linux NFS4 client. I think the caller can change it when needed (to 0 or whatever needed), but the default value should be good... volodymyr. On Fri, Oct 1, 2021 at 5:13 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Fri, Oct 01, 2021 at 09:49:50AM +0300, Volodymyr Khomenko wrote: > > > So, I can verify that --security=krb5 works after this patch but not > > > before, good. But why is that? As you say, the server is supposed to > > > ignore the sequence number on context creation requests. And 0 is valid > > > sequence number as far as I know. > > > > By design of RPCGSS we have a 'last seen seq_number' counter on the > > server side per each GSS context > > and we must not accept any packet that was already seen before (we > > also have a bitmask of several last requests for this). > > This 'last seen counter' is unsigned int32 (the same as seq_num) so we > > can't just init it to -1 so next seq_num=0 will be valid. > > The most obvious implementation is to init it last_seen_seq_num to 0 > > so the very 1st packet after NFS4 NULL must be 1 to differ from last > > seen seq_number. > > Note in theory gssapi mechanisms can require multiple round trips (in > the GSS_PROC_CONTINUE_INIT case), so this wouldn't actually avoid > duplicate sequence numbers. > > In any case, the rfc is unambiguous here: "In a creation request, the > seq_num and service fields are undefined and both must be ignored by the > server." > > > A better implementation (theoretically) can set this counter to > > 'undefined' state by additional flag, but this is more > > resource-consuming > > (you need to process is_inited flag + last_seen_seq_num instead of > > just one counter). > > If the last seen seq_number is undefined during GSS initialization, > > then strictly speaking we can send ANY seq_num for the very 1st > > request after NFS4 NULL. > > Right, again, from RFC 2203, " The seq_num field can start at any value > below MAXSEQ." > > It can be implemented without the need for an is_inited flag. > > The initial sequence number of 0 really did find an actual bug in the > server, so pynfs is definitely doing its job in this case! > > --b.
On Fri, Oct 01, 2021 at 05:38:45PM +0300, Volodymyr Khomenko wrote: > > The seq_num field can start at any value below MAXSEQ > Yes, that's the statement I haven't found before in RFC... > Probably we also need to write a test starting the seq_num from a big > value (more than SEQUENCE_WINDOW) > to make sure that it is really implemented properly without > 'is_inited' flag (so what's the initial value?). > > However I still propose to keep the default behaviour of pynfs to be > the same as for linux NFS4 client. > I think the caller can change it when needed (to 0 or whatever > needed), but the default value should be good... That's what I'd choose if I were writing a "real" client. Everybody already tests with the Linux client, so its behavior is a safe bet. But I'd usually prefer a test client do different things than the client everyone already tests with. Like I say, the seqid=0 already caught a bug in my server, so I'm a fan. (And it's a bug that would also trigger if any of the first 128 rpcs were out of order. But that would probably manifest as some user reporting "once in a blue moon my krb5 mounts hang" and I think it would take a while to get from that report to this bug as the root cause. So I'm glad pynfs hit it....) --b.
I've applied the "Fixed gssapi usage" patch, by the way, thanks for doing that! --b. On Thu, Sep 30, 2021 at 06:22:09PM +0300, Volodymyr Khomenko wrote: > Hello pynfs devs, > > It turned out that the pynfs library doesn't work well with the > current version of the 'gssapi' package (API was changed some time > ago) so tests with security=krb5* didn't work. > > The latest version of gssapi (aka python-gssapi) and documentation for > it can be found here: > https://github.com/pythongssapi/python-gssapi > https://pythongssapi.github.io/python-gssapi/latest/ > > Attached patches fixed the problem (tested with CentOS 7.7 NFS4 server > with pynfs + gssapi 1.6.2). > > Note - changes are not a complete solution, only nfs4.1 client code is fixed. > nfs4.1 server code and all the code for nfs4.0 should be fixed separately. > > Thanks, > Volodymyr Khomenko. > commit b77dc49c775756f08bdd0c6ebbe67a96f0ffe41f > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > Date: Thu Sep 30 17:53:04 2021 +0300 > > Fixed GSSContext to start sequence numbering from 1 > > GSS sequence number 0 is usually used by NFS4 NULL request > during GSS context establishment (but ignored by server). > Client should never reuse GSS sequence number, so using > 0 for the next real operation (EXCHANGE_ID) is possible but > looks suspicious. Fixed the code so numbering for operations > is done from 1 to avoid confusion. > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > diff --git a/rpc/security.py b/rpc/security.py > index 0682f43..86f6592 100644 > --- a/rpc/security.py > +++ b/rpc/security.py > @@ -174,7 +174,9 @@ class GSSContext(object): > def __init__(self, context_ptr): > self.lock = threading.Lock() > self.ptr = context_ptr > - self.seqid = 0 # client - next seqid to use > + # Note - seqid=0 is usually used during GSS context establishment, > + # to have the unique number we need to use the next value now. > + self.seqid = 1 # client - next seqid to use > self.highest = 0 # server - highest seqid seen > self.seen = 0 # server - bitmask of seen requests > > commit a612cf9897f0fa5b5de94885e00ef9293e93ffa3 > Author: Volodymyr Khomenko <volodymyr@vastdata.com> > Date: Thu Sep 30 16:29:07 2021 +0300 > > Fixed gssapi usage (RPCGSS) for nfs4.1 client > > 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: > ./nfs4.1/testserver.py server.fqdn:/export --maketree --security=krb5 all > > Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> > > diff --git a/rpc/security.py b/rpc/security.py > index fe4390c..0682f43 100644 > --- a/rpc/security.py > +++ b/rpc/security.py > @@ -10,6 +10,7 @@ from . import gss_type > from .gss_type import rpc_gss_init_res > try: > import gssapi > + from gssapi.raw.misc import GSSError > except ImportError: > print("Could not find gssapi module, proceeding without") > gssapi = None > @@ -242,11 +243,11 @@ class AuthGss(AuthNone): > > def init_cred(self, call, target="nfs@jupiter", source=None, oid=None): > # STUB - need intelligent way to set defaults > - good_major = [gssapi.GSS_S_COMPLETE, gssapi.GSS_S_CONTINUE_NEEDED] > + good_major = [GSS_S_COMPLETE, GSS_S_CONTINUE_NEEDED] > p = Packer() > up = GSSUnpacker('') > # Set target (of form nfs@SERVER) > - target = gssapi.Name(target, gssapi.NT_HOSTBASED_SERVICE) > + target = gssapi.Name(target, gssapi.NameType.hostbased_service) > # Set source (of form USERNAME) > if source is not None: > source = gssapi.Name(source, gssapi.NT_USER_NAME) > @@ -254,18 +255,26 @@ class AuthGss(AuthNone): > else: > # Just use default cred > gss_cred = None > - context = gssapi.Context() > - token = None > - handle = '' > + # 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=target, creds=gss_cred, flags=flags) > + input_token = None > + handle = b'' > proc = RPCSEC_GSS_INIT > - while True: > + while not context.complete: > # Call initSecContext. If it returns COMPLETE, we are done. > # If it returns CONTINUE_NEEDED, we must send d['token'] > # to the target, which will run it through acceptSecContext, > # and give us back a token we need to send through initSecContext. > # Repeat as necessary. > - token = context.init(target, token, gss_cred) > - if context.open: > + output_token = context.step(input_token) > + if context.complete: > # XXX if res.major == CONTINUE there is a bug in library code > # STUB - now what? Just use context? > # XXX need to use res.seq_window > @@ -277,16 +286,16 @@ class AuthGss(AuthNone): > gss_proc=proc) > proc = RPCSEC_GSS_CONTINUE_INIT > p.reset() > - p.pack_opaque(token) > + p.pack_opaque(output_token) > header, reply = call(p.get_buffer(), credinfo) > up.reset(reply) > res = up.unpack_rpc_gss_init_res() > up.done() > # res now holds relevent output from target's acceptSecContext call > if res.gss_major not in good_major: > - raise gssapi.Error(res.gss_major, res.gss_minor) > + raise GSSError(res.gss_major, res.gss_minor) > handle = res.handle # Should not change between calls > - token = res.gss_token # This needs to be sent to initSecContext > + input_token = res.gss_token # This needs to be sent to SecurityContext.step() > return CredInfo(self, context=handle) > > @staticmethod > @@ -361,7 +370,7 @@ class AuthGss(AuthNone): > except: > log_gss.exception("unsecure_data - initial unpacking") > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > - qop = context.verifyMIC(data, checksum) > + qop = context.verify_signature(data, checksum) > check_gssapi(qop) > data = pull_seqnum(data) > elif cred.service == rpc_gss_svc_privacy: > @@ -373,14 +382,14 @@ class AuthGss(AuthNone): > log_gss.exception("unsecure_data - initial unpacking") > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > # data, qop, conf = context.unwrap(data) > - data, qop = context.unwrap(data) > + data, encrypted, qop = context.unwrap(data) > check_gssapi(qop) > data = pull_seqnum(data) > else: > # Can't get here, but doesn't hurt > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > - except gssapi.Error as e: > - log_gss.warn("unsecure_data: gssapi call returned %s" % e.name) > + except GSSError as e: > + log_gss.warn("unsecure_data: gssapi call returned %s" % str(e)) > raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) > return data > > @@ -397,7 +406,7 @@ class AuthGss(AuthNone): > # data = opaque[gss_seq_num+data] + opaque[checksum] > p.pack_uint(cred.seq_num) > data = p.get_buffer() + data > - token = context.getMIC(data) # XXX BUG set qop > + token = context.get_signature(data) # XXX BUG set qop > p.reset() > p.pack_opaque(data) > p.pack_opaque(token) > @@ -406,16 +415,16 @@ class AuthGss(AuthNone): > # data = opaque[wrap([gss_seq_num+data])] > p.pack_uint(cred.seq_num) > data = p.get_buffer() + data > - token = context.wrap(data) # XXX BUG set qop > + wrap_res = context.wrap(data, encrypt=True) # XXX BUG set qop > p.reset() > - p.pack_opaque(token) > + p.pack_opaque(wrap_res.message) > data = p.get_buffer() > else: > # Can't get here, but doesn't hurt > log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) > - except gssapi.Error as e: > + except GSSError as e: > # XXX What now? > - log_gss.warn("secure_data: gssapi call returned %s" % e.name) > + log_gss.warn("secure_data: gssapi call returned %s" % str(e)) > raise > return data > > @@ -436,8 +445,8 @@ class AuthGss(AuthNone): > return rpclib.NULL_CRED > else: > data = self.partially_packed_header(xid, body) > - # XXX how handle gssapi.Error? > - token = self._get_context(body.cred.body.handle).getMIC(data) > + # XXX how handle GSSError? > + token = self._get_context(body.cred.body.handle).get_signature(data) > return opaque_auth(RPCSEC_GSS, token) > > def check_call_verf(self, xid, body): > @@ -448,10 +457,10 @@ class AuthGss(AuthNone): > return False > data = self.partially_packed_header(xid, body) > try: > - qop = self._get_context(body.cred.body.handle).verifyMIC(data, body.verf.body) > - except gssapi.Error as e: > + qop = self._get_context(body.cred.body.handle).verify_signature(data, body.verf.body) > + except GSSError as e: > log_gss.warn("Verifier checksum failed verification with %s" % > - e.name) > + str(e)) > return False > body.cred.body.qop = qop # XXX Where store this? > log_gss.debug("verifier checks out (qop=%i)" % qop) > @@ -522,10 +531,10 @@ class AuthGss(AuthNone): > context = self._get_context(cred.body.handle) > try: > token = context.accept(token) > - except gssapi.Error as e: > + except GSSError as e: > log_gss.debug("RPCSEC_GSS_INIT failed (%s, %i)!" % > - (e.name, e.minor)) > - res = rpc_gss_init_res('', e.major, e.minor, 0, '') > + (str(e), e.min_code)) > + res = rpc_gss_init_res('', e.maj_code, e.min_code, 0, '') > else: > log_gss.debug("RPCSEC_GSS_*INIT succeeded!") > if first: > @@ -538,9 +547,9 @@ class AuthGss(AuthNone): > else: > handle = cred.body.handle > if context.open: > - major = gssapi.GSS_S_COMPLETE > + major = GSS_S_COMPLETE > else: > - major = gssapi.GSS_S_CONTINUE_NEEDED > + major = GSS_S_CONTINUE_NEEDED > res = rpc_gss_init_res(handle, major, 0, # XXX can't see minor > WINDOWSIZE, token) > # Prepare response > @@ -559,15 +568,15 @@ class AuthGss(AuthNone): > # NOTE this relies on GSS_S_COMPLETE == rpc.SUCCESS == 0 > return rpclib.NULL_CRED > elif cred.gss_proc in (RPCSEC_GSS_INIT, RPCSEC_GSS_CONTINUE_INIT): > - # init requires getMIC(seq_window) > + # init requires get_signature(seq_window) > i = WINDOWSIZE > else: > - # Else return getMIC(cred.seq_num) > + # Else return get_signature(cred.seq_num) > i = cred.seq_num > p = Packer() > p.pack_uint(i) > # XXX BUG - need to set qop > - token = self._get_context(cred.handle).getMIC(p.get_buffer()) > + token = self._get_context(cred.handle).get_signature(p.get_buffer()) > return opaque_auth(RPCSEC_GSS, token) > > def check_reply_verf(self, msg, call_cred, data): > @@ -593,12 +602,12 @@ class AuthGss(AuthNone): > if res.gss_major != GSS_S_COMPLETE: > raise SecError("Expected NULL") > # BUG - context establishment is not finished on client > - # - so how get context? How run verifyMIC? > + # - so how get context? How run verify_signature? > # - This seems to be a protocol problem. Just ignore for now > else: > p = Packer() > p.pack_uint(call_cred.body.seq_num) > - qop = call_cred.context.verifyMIC(p.get_buffer(), verf.body) > + qop = call_cred.context.verify_signature(p.get_buffer(), verf.body) > if qop != call_cred.body.qop: > raise SecError("Mismatched qop") >
Ok, I see your point - yes, it's better that pynfs has "unusual but still RFC-compliant" behaviour to catch the real bugs. I will check how this behaviour (using seqid=0 or even something else for EXCHANGE_ID) can be controlled from the caller side if we want to override it. P.S. Since the very 1st operation after NFS4 NULL is EXCHANGE_ID - it should be only single operation (client can't send few ECHANGE_ID because clientowner is only one per mount) and next CREATE_SESSION can't be sent until EXCHANGE_ID is replied from the server. So the use-case of 'any of the first 128 rpcs were out of order' is just a theoretical one and probably not possible in practice. And thanks for handling the patch! volodymyr. On Fri, Oct 1, 2021 at 6:48 PM J. Bruce Fields <bfields@fieldses.org> wrote: > > On Fri, Oct 01, 2021 at 05:38:45PM +0300, Volodymyr Khomenko wrote: > > > The seq_num field can start at any value below MAXSEQ > > Yes, that's the statement I haven't found before in RFC... > > Probably we also need to write a test starting the seq_num from a big > > value (more than SEQUENCE_WINDOW) > > to make sure that it is really implemented properly without > > 'is_inited' flag (so what's the initial value?). > > > > However I still propose to keep the default behaviour of pynfs to be > > the same as for linux NFS4 client. > > I think the caller can change it when needed (to 0 or whatever > > needed), but the default value should be good... > > That's what I'd choose if I were writing a "real" client. Everybody > already tests with the Linux client, so its behavior is a safe bet. > > But I'd usually prefer a test client do different things than the client > everyone already tests with. > > Like I say, the seqid=0 already caught a bug in my server, so I'm a fan. > > (And it's a bug that would also trigger if any of the first 128 rpcs > were out of order. But that would probably manifest as some user > reporting "once in a blue moon my krb5 mounts hang" and I think it would > take a while to get from that report to this bug as the root cause. So > I'm glad pynfs hit it....) > > --b.
On Sat, Oct 02, 2021 at 09:12:25AM +0300, Volodymyr Khomenko wrote: > P.S. Since the very 1st operation after NFS4 NULL is EXCHANGE_ID - it > should be only single operation > (client can't send few ECHANGE_ID because clientowner is only one per > mount) and next CREATE_SESSION can't be sent > until EXCHANGE_ID is replied from the server. > So the use-case of 'any of the first 128 rpcs were out of order' is > just a theoretical one and probably not possible in practice. So our server uses a fixed-size gss sequence number window of 128. We keep track of sd_max, the largest sequence number we've seen so far. Given an incoming rpc with sequence number seqno, we check: is seqno > sd_max? This is the normal case for in-order sequence numbers; update sd_max and our other sequence number data and continue normal processing. else is seqno < sd_max - 128? Oops, this is definitely too old; drop the request. else check our data about sequence numbers seen so far. But our specific bug was we were doing the second check using unsigned arithmetic, so if we hit the second check before sd_max hits 128, then (sd_max - 128) is something very large, and we drop the request. --b.
commit a612cf9897f0fa5b5de94885e00ef9293e93ffa3 Author: Volodymyr Khomenko <volodymyr@vastdata.com> Date: Thu Sep 30 16:29:07 2021 +0300 Fixed gssapi usage (RPCGSS) for nfs4.1 client 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: ./nfs4.1/testserver.py server.fqdn:/export --maketree --security=krb5 all Signed-off-by: Volodymyr Khomenko <volodymyr@vastdata.com> diff --git a/rpc/security.py b/rpc/security.py index fe4390c..0682f43 100644 --- a/rpc/security.py +++ b/rpc/security.py @@ -10,6 +10,7 @@ from . import gss_type from .gss_type import rpc_gss_init_res try: import gssapi + from gssapi.raw.misc import GSSError except ImportError: print("Could not find gssapi module, proceeding without") gssapi = None @@ -242,11 +243,11 @@ class AuthGss(AuthNone): def init_cred(self, call, target="nfs@jupiter", source=None, oid=None): # STUB - need intelligent way to set defaults - good_major = [gssapi.GSS_S_COMPLETE, gssapi.GSS_S_CONTINUE_NEEDED] + good_major = [GSS_S_COMPLETE, GSS_S_CONTINUE_NEEDED] p = Packer() up = GSSUnpacker('') # Set target (of form nfs@SERVER) - target = gssapi.Name(target, gssapi.NT_HOSTBASED_SERVICE) + target = gssapi.Name(target, gssapi.NameType.hostbased_service) # Set source (of form USERNAME) if source is not None: source = gssapi.Name(source, gssapi.NT_USER_NAME) @@ -254,18 +255,26 @@ class AuthGss(AuthNone): else: # Just use default cred gss_cred = None - context = gssapi.Context() - token = None - handle = '' + # 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=target, creds=gss_cred, flags=flags) + input_token = None + handle = b'' proc = RPCSEC_GSS_INIT - while True: + while not context.complete: # Call initSecContext. If it returns COMPLETE, we are done. # If it returns CONTINUE_NEEDED, we must send d['token'] # to the target, which will run it through acceptSecContext, # and give us back a token we need to send through initSecContext. # Repeat as necessary. - token = context.init(target, token, gss_cred) - if context.open: + output_token = context.step(input_token) + if context.complete: # XXX if res.major == CONTINUE there is a bug in library code # STUB - now what? Just use context? # XXX need to use res.seq_window @@ -277,16 +286,16 @@ class AuthGss(AuthNone): gss_proc=proc) proc = RPCSEC_GSS_CONTINUE_INIT p.reset() - p.pack_opaque(token) + p.pack_opaque(output_token) header, reply = call(p.get_buffer(), credinfo) up.reset(reply) res = up.unpack_rpc_gss_init_res() up.done() # res now holds relevent output from target's acceptSecContext call if res.gss_major not in good_major: - raise gssapi.Error(res.gss_major, res.gss_minor) + raise GSSError(res.gss_major, res.gss_minor) handle = res.handle # Should not change between calls - token = res.gss_token # This needs to be sent to initSecContext + input_token = res.gss_token # This needs to be sent to SecurityContext.step() return CredInfo(self, context=handle) @staticmethod @@ -361,7 +370,7 @@ class AuthGss(AuthNone): except: log_gss.exception("unsecure_data - initial unpacking") raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) - qop = context.verifyMIC(data, checksum) + qop = context.verify_signature(data, checksum) check_gssapi(qop) data = pull_seqnum(data) elif cred.service == rpc_gss_svc_privacy: @@ -373,14 +382,14 @@ class AuthGss(AuthNone): log_gss.exception("unsecure_data - initial unpacking") raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) # data, qop, conf = context.unwrap(data) - data, qop = context.unwrap(data) + data, encrypted, qop = context.unwrap(data) check_gssapi(qop) data = pull_seqnum(data) else: # Can't get here, but doesn't hurt log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) - except gssapi.Error as e: - log_gss.warn("unsecure_data: gssapi call returned %s" % e.name) + except GSSError as e: + log_gss.warn("unsecure_data: gssapi call returned %s" % str(e)) raise rpclib.RPCUnsuccessfulReply(GARBAGE_ARGS) return data @@ -397,7 +406,7 @@ class AuthGss(AuthNone): # data = opaque[gss_seq_num+data] + opaque[checksum] p.pack_uint(cred.seq_num) data = p.get_buffer() + data - token = context.getMIC(data) # XXX BUG set qop + token = context.get_signature(data) # XXX BUG set qop p.reset() p.pack_opaque(data) p.pack_opaque(token) @@ -406,16 +415,16 @@ class AuthGss(AuthNone): # data = opaque[wrap([gss_seq_num+data])] p.pack_uint(cred.seq_num) data = p.get_buffer() + data - token = context.wrap(data) # XXX BUG set qop + wrap_res = context.wrap(data, encrypt=True) # XXX BUG set qop p.reset() - p.pack_opaque(token) + p.pack_opaque(wrap_res.message) data = p.get_buffer() else: # Can't get here, but doesn't hurt log_gss.error("Unknown service %i for RPCSEC_GSS" % cred.service) - except gssapi.Error as e: + except GSSError as e: # XXX What now? - log_gss.warn("secure_data: gssapi call returned %s" % e.name) + log_gss.warn("secure_data: gssapi call returned %s" % str(e)) raise return data @@ -436,8 +445,8 @@ class AuthGss(AuthNone): return rpclib.NULL_CRED else: data = self.partially_packed_header(xid, body) - # XXX how handle gssapi.Error? - token = self._get_context(body.cred.body.handle).getMIC(data) + # XXX how handle GSSError? + token = self._get_context(body.cred.body.handle).get_signature(data) return opaque_auth(RPCSEC_GSS, token) def check_call_verf(self, xid, body): @@ -448,10 +457,10 @@ class AuthGss(AuthNone): return False data = self.partially_packed_header(xid, body) try: - qop = self._get_context(body.cred.body.handle).verifyMIC(data, body.verf.body) - except gssapi.Error as e: + qop = self._get_context(body.cred.body.handle).verify_signature(data, body.verf.body) + except GSSError as e: log_gss.warn("Verifier checksum failed verification with %s" % - e.name) + str(e)) return False body.cred.body.qop = qop # XXX Where store this? log_gss.debug("verifier checks out (qop=%i)" % qop) @@ -522,10 +531,10 @@ class AuthGss(AuthNone): context = self._get_context(cred.body.handle) try: token = context.accept(token) - except gssapi.Error as e: + except GSSError as e: log_gss.debug("RPCSEC_GSS_INIT failed (%s, %i)!" % - (e.name, e.minor)) - res = rpc_gss_init_res('', e.major, e.minor, 0, '') + (str(e), e.min_code)) + res = rpc_gss_init_res('', e.maj_code, e.min_code, 0, '') else: log_gss.debug("RPCSEC_GSS_*INIT succeeded!") if first: @@ -538,9 +547,9 @@ class AuthGss(AuthNone): else: handle = cred.body.handle if context.open: - major = gssapi.GSS_S_COMPLETE + major = GSS_S_COMPLETE else: - major = gssapi.GSS_S_CONTINUE_NEEDED + major = GSS_S_CONTINUE_NEEDED res = rpc_gss_init_res(handle, major, 0, # XXX can't see minor WINDOWSIZE, token) # Prepare response @@ -559,15 +568,15 @@ class AuthGss(AuthNone): # NOTE this relies on GSS_S_COMPLETE == rpc.SUCCESS == 0 return rpclib.NULL_CRED elif cred.gss_proc in (RPCSEC_GSS_INIT, RPCSEC_GSS_CONTINUE_INIT): - # init requires getMIC(seq_window) + # init requires get_signature(seq_window) i = WINDOWSIZE else: - # Else return getMIC(cred.seq_num) + # Else return get_signature(cred.seq_num) i = cred.seq_num p = Packer() p.pack_uint(i) # XXX BUG - need to set qop - token = self._get_context(cred.handle).getMIC(p.get_buffer()) + token = self._get_context(cred.handle).get_signature(p.get_buffer()) return opaque_auth(RPCSEC_GSS, token) def check_reply_verf(self, msg, call_cred, data): @@ -593,12 +602,12 @@ class AuthGss(AuthNone): if res.gss_major != GSS_S_COMPLETE: raise SecError("Expected NULL") # BUG - context establishment is not finished on client - # - so how get context? How run verifyMIC? + # - so how get context? How run verify_signature? # - This seems to be a protocol problem. Just ignore for now else: p = Packer() p.pack_uint(call_cred.body.seq_num) - qop = call_cred.context.verifyMIC(p.get_buffer(), verf.body) + qop = call_cred.context.verify_signature(p.get_buffer(), verf.body) if qop != call_cred.body.qop: raise SecError("Mismatched qop")