Bug 3364 - authz callback bug in case of negative authorization decisions?
: authz callback bug in case of negative authorization decisions?
Status: RESOLVED FIXED
: GridFTP
GridFTP
: 4.0.0
: Macintosh All
: P3 normal
: ---
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2005-05-16 20:15 by
Modified: 2005-05-23 16:54 (History)


Attachments
patch (1.35 KB, patch)
2005-05-17 16:28, Mike Link
Details


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2005-05-16 20:15:23
I have been playing with GSI authz callout functions. For easy testing, I wrote
a simple function that will always complain on a certain filename but allow
everything else.

If authorized (result=GLOBUS_SUCCESS), all is well.

If not authorized (result!=GLOBUS_SUCCESS), a 500 error is delivered to the
client and the FTP server dies here:
Assertion data_handle->outstanding_op != NULL failed in file globus_i_gfs_data.c
at line 2244

It seems to me that a non-access to a file should not result in a server thread
collapse? Or is it because globus-url-copy (my client testing tool) that simply
hangs up abruptly when it receives a 500 response?
------- Comment #1 From 2005-05-17 16:28:45 -------
Created an attachment (id=616) [details]
patch

This patch should fix the bug... It wasn't the authz callout failure that
triggered the assertion, rather bad state at cleanup time after g-u-c closed
the connection.  Let me know if this helps.
------- Comment #2 From 2005-05-18 08:10:01 -------
Applied the patch but that didn't fix it. BUT, looking closer at things in the
debugger, I noticed that the callback function that I'm getting is
globus_gfs_acl_cas_cb(): is that just bad naming or is it assumed that some
CAS-specific stuff is to happen here with e.g. the callback_args, and that just
any callout (i.e., mine) simply won't cut it?

The callback is called with arguments (callback_arg, handle, result) where

            result = globus_error_put(globus_error_construct_error(
                    GLOBUS_GSI_AUTHZ_MODULE,
                        NULL,
                        1234,
                        __FILE__,
                        "foo_authz_callout",
                        __LINE__,
                        "FOO module denied access to %s object %s",
                        action,
                        object));

Then I do:

    callback(callback_arg, handle, result);
    return result;

I.e., the result ends up at two places. This seems a bit odd to me and a
potential source of the problem (who is in charge of the error object cleanup?).
Also, if the authorization logic worked fine and we denied access, shouldn't we
report one GLOBUS_SUCCESS (the authz logic worked fine) and one error (the authz
decision)?

I have tried the following possible combinations (result below indicates a
return value != GLOBUS_SUCCESS)

#1: callback(...,result);
    return result;

#2: callback(...,result);
    return GLOBUS_SUCCESS;

#3: callback(...,GLOBUS_SUCCESS);
    return result;

#4: callback(...,GLOBUS_SUCCESS);
    return GLOBUS_SUCCESS;

#5: /* callback(...,GLOBUS_SUCCESS); */
    return GLOBUS_SUCCESS;

#6: callback(...,result);
    return globus_error_put(globus_object_copy(globus_error_peek(result)));

Only #4 does not result in a SEGV, assertion error, or deadlock, but that one is
not so interesting for obvious reasons... ;-)
------- Comment #3 From 2005-05-18 17:41:38 -------
I'm going to have to test this some more, I don't see how else it would fail 
like that.  The cas func name is just the namespace we have for our gsi-authz 
based acl module, there is nothing cas specific (we just refered to the authz 
callouts as cas as a matter of habit at that point)... as far as return val and 
callback result value, the rule is that if you return failure, you must not 
call the callback, and if you return success, you must call it.

In the meantime, can you set the env GLOBUS_GRIDFTP_SERVER_DEBUG=ALL,<filename> 
and attach <filename> when the assertion occurs.
------- Comment #4 From 2005-05-23 14:28:25 -------
I was able to reproduce this, and the patch correctly fixes it for me.  Can you 
verify you applied the patch successfully?  If you're still asserting with the 
patch, something else must be going on.  Send me your callout and I'll try with 
that.

Mike
------- Comment #5 From 2005-05-23 16:36:26 -------
I doublechecked, and yes, my code works fine with your patch. As indicated
previously, I tried a bunch of different ways to properly propagate a return
value!=GLOBUS_SUCCESS, the only one I didn't try was the correct one...
 
This bug can be probably be closed, but I would suggest that a simple "if
(result) return;" statement is added at the top of the receiving callback
function, to make the system less sensitive to bad developers. :-)
------- Comment #6 From 2005-05-23 16:54:30 -------
Unfortunately its hard to support errors involving callbacks coming when they 
shouldn't or not coming when they should.  An failure result to the callback is 
still acceptable (and indeed, possibly the most 'correct' from an async 
standpoint), as long as the callout returned success in the first place to 
indicate a callback would come.

Thanks for testing.  The patch here will be included with the next bugfix 
release.

Mike