Null Value Type?

[Havoc Pennington]

Hi,

Simon McVittie wrote:
> This opens the same can of worms that the float32 type does, though:
> apps can't send a type that the bus daemon doesn't understand without being
> forcibly disconnected from the bus daemon, and if an app receives a type
> that *it* doesn't understand, it'll disconnect itself from the bus
> daemon. So, we need either some sort of feature negotiation, or a new
> protocol version.

fwiw I think we need the feature negotiation even if the bus didn't 
disconnect; I think it would be screwy to write:

  if (method call with float fails)
    resend sans float

inline in the app over and over, so the app just KEPT trying to use 
float and every method call was effectively call->error->recall->success

Instead you want to write:

  if (other end does not support float)
    do whatever
  else
    do whatever else

which means feature negotiation.

I mean, you could try "send one thing with a float, then set a flag for 
whether float works based on that" but that's just a hacky way to do 
feature negotiation. I would rather see a non-hacky way.

> It's perhaps unfortunate that libdbus doesn't respond more gracefully to
> incompatible messages, e.g. returning an error and dropping the message
> (the message header contains the number of bytes to skip, so you could do
> this as long as the first 8 bytes of the message header remain
> compatible in future versions), but the current behaviour is specified
> for security reasons.
> 
> Havoc, would you mind reminding me what those security reasons are?

It's simply paranoia, the same reason a setuid app is going to exit() 
the minute anything is fishy. The system bus daemon is in effect a 
setuid binary.

Note that it only summarily drops the connection if the problem is 
well-formedness; the equivalent of sending a string with no nul at the 
end, or a bad length, or whatever.

It is more lenient about things it simply doesn't understand. For 
example, it will ignore extra header fields.

In XML terms, the instant disconnection would come from something like:

  "<<>&*<>"

i.e. malformed junk, while the error or ignoring the extension would 
come just from not knowing a tag.

 > We could even spec an error
> org.freedesktop.DBus.Errors.ProtocolNotSupported for this case.

In a highly security sensitive piece of code, I don't much like 
attempting to heuristically recover from malformed data of unknown 
intent and unknown nature. We shouldn't design this behavior thinking 
about a known protocol extension, we should design this behavior 
thinking about a maliciously constructed message. The bus daemon can't 
tell the difference.

We're also trying to protect apps on the bus from weird stuff _they_ 
don't understand.

That said, I think there is a middle ground: just prior to 
disconnection, cram an error message down the socket, THEN disconnect. 
This avoids trying to continue in the face of potential confusion, but 
it solves the problem of app developers not knowing what's going on, at 
least most of the time (it won't work if the socket buffer is full, but 
we can just punt on that).

I would take a patch to do that. I think it would involve something like:

dbus_connection_disconnect_with_error(connection, serial, error_name, 
error_message)

Then everywhere where the bus daemon calls disconnect, change the code 
to pass in the error.

Even with the current behavior though, I believe most times an app 
developer sees a connection with no explanation it is a bug: libdbus 
should have caught the problem on the client side with a return_if_fail 
warning. In other words, libdbus is not supposed to ever produce 
non-well-formed protocol, if it gets junk from the app it is supposed to 
warn.

So, _please report_ anytime you get dropped while using libdbus, without 
an accompanying warning to stderr.

There are some fixes in this respect in the 1.1.x series. I know of one 
or two complicated situations that are hard to efficiently warn about, 
but I don't know of many other cases that fail to warn.

For something like a protocol extension, I also believe it would be a 
bug anytime you got disconnected, because feature negotiation should 
avoid ever sending the malformed protocol.

Anyway in short yes I think we could return an error before dropping the 
client, and would take a patch for that, but also I think when people 
are hitting this it's usually some other bug that should be fixed, so we 
could use patches on those bugs too.

Havoc