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