[RFC] State machine rewrite of dbus-auth.c

[Kristian Høgsberg]

Hi all,

I've been reading/auditing/reworking the dbus auth implementation over
the last week with the intention of rewriting it as a state machine as
mentioned in the TODO.  This mail is a description of the work I've done
so far, and I would appreciate any comments/feedback you may have.

Just to explain my take on state machines: A state is a configuration
where you anticipate a number of events; transition from one state to
another happens when you receive one of these events.  Determined by the
state and the event, you perform an action and go to a new state.

Thus, I have folded some of the states in the specification state
machine, e.g. NeedSendBegin is not a state in my proposal, but just an
action performed while transitioning to another state.

Also, I've introduced a bit of notation to more precisely describe the
interaction between the protocol state machine and the authentication
mechanisms: MECH(RESP) means that the client response RESP was fed to
the the mechanism MECH, which returns one of

        CONTINUE(CHALL) means continue the auth conversation and send
        CHALL as the challenge to the client;

        OK means that the client has been successfully authenticated;

        REJECT means that the client failed to authenticate or there was
        an error in RESP.

Both RESP and CHALL may be empty.  Currently MECH can be either EXTERNAL
or DBUS_COOKIE_SHA1.  For the client, MECH(CHALL) means that the server
challenge CHALL was fed to the mechanism MECH, which returns one of

        CONTINUE(RESP) means continue the auth conversation and the RESP
        as the response to the server;
        
        OK(RESP) means that after sending RESP to the server the client
        side of the auth conversation is finished and the server should
        return "OK";
        
        ERROR means that CHALL was invalid and could not be processed.

With that out of the way, here are the server states.  It's mostly
similar to the states in the specification.

Server states:

State WaitingForAuth:
1 Receive "AUTH"
  -> send "REJECTED [mechs]", goto WaitingForAuth
2 Receive "AUTH MECH RESP" but MECH not valid mechanism
  -> send "REJECTED [mechs]", goto WaitingForAuth
3 Receive "AUTH MECH RESP" and MECH(RESP) returns CONTINUE(CHALL)
  -> send "DATA CHALL", goto WaitingForData
4 Receive "AUTH MECH RESP" and MECH(RESP) returns OK
  -> send "OK", goto WaitingForBegin
5 Receive "AUTH MECH RESP" and MECH(RESP) returns REJECT
  -> send "REJECTED [mechs]", goto WaitingForAuth
6 Receive "BEGIN"
  -> terminate auth conversation, disconnect
7 Receive anything else
  -> send "ERROR", goto WaitingForAuth

Comments: RESP may be empty.  For the EXTERNAL mechanism it is possible
to authenticate immediately from this state (rule 4).  Some mechanisms
require the server to send an initial challenge - if the client sends an
initial response for such a mechanism it is rejected (rule 5).

State WaitingForData:
1 Receive "DATA RESP" and MECH(RESP) returns CONTINUE(CHALL)
  -> send "DATA CHALL", goto WaitingForData
2 Receive "DATA RESP" and MECH(RESP) returns OK
  -> send "OK", goto WaitingForBegin
3 Receive "DATA RESP" and MECH(RESP) returns REJECT
  -> send "REJECTED [mechs]", goto WaitingForAuth
4 Receive "CANCEL"
  -> send "REJECTED [mechs]", goto WaitingForAuth
5 Receive "ERROR"
  -> send "REJECTED [mechs]", goto WaitingForAuth
6 Receive anything else
  -> send "ERROR", goto WaitingForData

State WaitingForBegin:
1 Receive "BEGIN"
  -> terminate auth conversation, client authenticated
2 Receive "CANCEL"
  -> send "REJECTED [mechs]", goto WaitingForAuth
3 Receive anything else
  -> send "ERROR", goto WaitingForBegin

Comments: Compared with the spec, I added rule 2 so that a client can
bail out if it wants.  Rule 3 allows the protocol to be extended so the
server and client can exchange other inital handshake information as
suggested in dbus/TODO.  This type of dialogue should only take place
after the client has been authenticated to minimize the options
available to an unauthenticated client, and in this state we know the
client is good.  Further revisions of this protocol could add commands
for the client to use after it receives OK and before it sends BEGIN. 
If the server does not understand such a command it must send ERROR, and
the client would then send BEGIN.


The Client starts by getting an initial response from the default
mechanism and sends "AUTH mech resp" (again, resp could be empty).  If
the mechanism returned CONTINUE, the client starts in state
WaitingForData, if the mechanism returns OK the client starts in state
WaitingForOK.

Client states:

State WaitingForData:
1 Receive "DATA CHALL" and MECH(CHALL) returns CONTINUE(RESP)
  -> send "DATA RESP", goto WaitingForData
2 Receive "DATA CHALL" and MECH(CHALL) returns OK
  -> send "DATA", goto WaitingForOK
3 Receive "DATA CHALL" and MECH(RESP) returns ERROR
  -> send "ERROR [msg]", goto WaitingForData
4 Receive "REJECTED [mechs]"
  -> send "AUTH [next mech]", goto WaitingForData or WaitingForOK
5 Receive "ERROR"
  -> send "CANCEL", goto WaitingForReject
6 Receive "OK"
  -> send "BEGIN", terminate auth conversation, authenticated
7 Receive anything else
  -> send "ERROR", goto WaitingForData

Comments: Rule 6 handles unexpected succesful authentication, i.e. the
client mechanism expected further challenges, but the server already
reports "OK".

State WaitingForOK:
1 Receive "OK"
  -> send "BEGIN", terminate auth conversation, authenticated
2 Receive "REJECT [mechs]"
  -> send "AUTH [next mech]", goto WaitingForData or WaitingForOK
3 Receive anything else
  -> send "CANCEL", goto WaitingForReject

State WaitingForReject:
1 Receive "REJECT [mechs]"
  -> send "AUTH [next mech]", goto WaitingForData
2 Receive anything else
  -> send "CANCEL", goto WaitingForReject


Implementation

I'm currently implementing the stuff described above, so I'll have a
patch ready in a day or so.  For now, here's an overview of the changes
I would like to make:

- Add send_data(), send_auth() etc. functions to more clearly signal
that we're sending a reponse to the client.  For example:

dbus_bool_t
send_data (DBusAuth *auth, DBusString *data)
{
  if (data)
    return _dbus_string_append_printf (&auth->outgoing, "DATA %s\r\n",
                                       _dbus_string_get_const_data (data));
  else
    return _dbus_string_append (&auth->outgoing, "DATA\r\n");
}

- Move the protocol handling (everything about sending replies) out of
the mechanisms' data functions, have them return DBusString output and
OK, REJECT or CONTINUE (like Cyrus SASL mechanisms, basically) like so:

typedef dbus_bool_t
(* DBusAuthDataFunction) (DBusAuth         *auth,
                          const DBusString *input,
                          DBusString       *output,
                          DBusError        *error);

Where error can be DBUS_ERROR_AUTH_FAILED or DBUS_ERROR_NO_MEMORY.

- Implement each state described above as a function like

dbus_bool_t
handle_server_state_waiting_for_auth(DBusAuth        *auth,
                                     DBusAuthCommand  command,
                                     DBusString      *args)
{
  _dbus_assert (auth->mech == NULL);

  switch (command)
    {
    case DBUS_AUTH_COMMAND_AUTH:
      ...
      if (send_reject (auth))
        {
          goto_state (auth, &server_state_waiting_for_auth);
          return TRUE;
        }
      else
        return FALSE;
    }
}

Comments?
Kristian