[SSSD] [PATCH] Guard against invalid DP messages

Jakub Hrozek jhrozek at redhat.com
Mon Nov 9 14:14:31 UTC 2015


On Mon, Nov 09, 2015 at 12:44:17PM +0100, Sumit Bose wrote:
> On Fri, Nov 06, 2015 at 09:32:23PM +0100, Jakub Hrozek wrote:
> > On Fri, Nov 06, 2015 at 09:13:07PM +0100, Jakub Hrozek wrote:
> > > On Fri, Nov 06, 2015 at 04:59:58PM +0100, Sumit Bose wrote:
> > > > > @@ -658,11 +678,7 @@ static void get_subdomains_callback(struct be_req *req,
> > > > >           */
> > > > >          err_maj = dp_err_type;
> > > > >          err_min = errnum;
> > > > > -        if (errstr) {
> > > > > -            err_msg = errstr;
> > > > > -        } else {
> > > > > -            err_msg = dp_err_to_string(dp_err_type);
> > > > > -        }
> > > > > +        err_msg = safe_be_req_err_msg(errstr, dp_err_type);
> > > > 
> > > > Wouldn't it be even more safe to do this check in
> > > > sbus_request_return_and_finish() and not rely on the callers?
> > > 
> > > Something like the attached patch? I think we should check in
> > > sbus_request_return_and_finish() in addition to checking in the callers,
> > > not instead, because the string in the callback is just a diagnostics
> > > message and we can recover by using some fallback.
> > > 
> > > If we get all the way to sbus_request_return_and_finish(), then failing
> > > gracefully is better then abort() that libdbus calls, but it's even better
> > > to finish the request with wrong error message then fail completely.
> > > 
> > > (I also still dislike that libdbus aborts...)
> > 
> > Also, self-nack. dbus_validate_utf8() is not available on RHEL-6:
> >     http://sssd-ci.duckdns.org/logs/job/32/17/rhel6/ci-build-debug/ci-make-tests.log
> > 
> > I guess we can limit these checks to new OSs or use sss_utf8_check() on
> > older platforms..
> 
> Why not always use sss_utf8_check()? Is there an advantage using
> dbus_validate_utf8()?

I don't know how dbus_validate_utf8() works internally, so I tried to
stick to the dbus-provided version to check dbus types.

But if both do the same thing (If they don't it would be a bug, sure),
we can stick to sss_utf8_check()

(Another advantage might be that we could get away with linking against
libdbus only)


More information about the sssd-devel mailing list