Hi,

in addition to Niels comments, here are some unstructured thoughts - I
do realize that this is work in progress, but I want to point it out
so I don't forget. Also consider this food for thought, I may be wrong
about things.

> static void
> do_handle_gssapi_token(struct packet_handler *s,
>                      struct ssh_connection *connection,
>                      struct lsh_string *packet)
> {
>   CAST(gssapi_token_handler, self, s);
>   OM_uint32 maj_stat;
>   OM_uint32 min_stat;
>   OM_uint32 retflags;
>   gss_ctx_id_t ctx = GSS_C_NO_CONTEXT;
>   gss_cred_id_t cred = GSS_C_NO_CREDENTIAL;
>   gss_buffer_desc inbuf, outbuf;
>   gss_name_t client;
>
>   maj_stat = gss_acquire_cred (&min_stat, GSS_C_NO_NAME, 0,
>                              GSS_C_NULL_OID_SET, GSS_C_ACCEPT,
>                              &cred, NULL, NULL);
>   if (GSS_ERROR(maj_stat))
>     {
>       display_status("acquire_cred", maj_stat, min_stat);
>       /* XXX send SSH_MSG_USERAUTH_GSSAPI_ERROR */
>       PROTOCOL_ERROR(connection->e, "GSSAPI acquire_cred failed.");
>     }

I think you should reset
connection->dispatch[SSH_MSG_USERAUTH_GSSAPI_TOKEN] here and have
connection->explicit returns after PROTOCOL_ERRORS.


>   inbuf.value = packet->data + 1 + 4;
>   inbuf.length = packet->length - 1 - 4;
>   maj_stat = gss_accept_sec_context (&min_stat,
>                                    &ctx,
>                                    cred,
>                                    &inbuf,
>                                    GSS_C_NO_CHANNEL_BINDINGS,
>                                    &client,
>                                    GSS_C_NO_OID,
>                                    &outbuf,
>                                    &retflags,
>                                    NULL,
>                                    NULL);
>   if (maj_stat != GSS_S_COMPLETE && maj_stat != GSS_S_CONTINUE_NEEDED)
>     {
>       display_status("accept_sec_context", maj_stat, min_stat);
>       /* XXX send SSH_MSG_USERAUTH_GSSAPI_ERROR */
>       PROTOCOL_ERROR(connection->e, "GSSAPI accept_sec_context failed.");
>     }
>
>   if (maj_stat == GSS_S_COMPLETE)
>     {
>       connection->dispatch[SSH_MSG_USERAUTH_GSSAPI_TOKEN] =
>       &connection_unimplemented_handler;
>       connection->dispatch[SSH_MSG_USERAUTH_GSSAPI_EXCHANGE_COMPLETE] =
>       make_gssapi_finish_handler(self->gssapi);
>       C_WRITE(connection, ssh_format("%c%s", SSH_MSG_USERAUTH_GSSAPI_TOKEN,
>                                    outbuf.length, outbuf.value));
>       return;
>     }
> }

Missing case maj_stat == GSS_S_CONTINUE_NEEDED? It's probably a good
idea to reset both connection->dispatch[SSH_MSG_USERAUTH_GSSAPI_TOKEN]
and connection->dispatch[SSH_MSG_USERAUTH_GSSAPI_EXCHANGE_COMPLETE] at
the beginning and set the correct one before returning if all goes well.

>   if (parse_uint32(args, &number_of_mechanisms))
>     {
>       int i;
>       uint32_t len;
>       uint8_t *oid;
>
>       printf("hepp %d\n", number_of_mechanisms);
>
>       for (i = 0; i < number_of_mechanisms; i++)
>       if (!parse_string(args, &len, &oid))
>         goto fail;

I assume there will be a check if the given mechanism is available
here?

If I'm not missing something, the current implementation would allow
an attacker to request authentication as user foo and use GSS to
authenticate herself as bar, and gain access as foo, which would be a
bad thing, I assume GSS can be told to only accept authentication for
a given user or that one can check after authentication who was
authenticated?

Anyway, very nice work, having GSS support seems to be a Good Thing.
Kudos to you.

        /Pontus
-- 

Pontus Sk�ld, see <URL:http://soua.net/> for more information.
_______________________________________________
lsh-bugs mailing list
[EMAIL PROTECTED]
http://lists.lysator.liu.se/mailman/listinfo/lsh-bugs

Reply via email to