Skip to content

Commit

Permalink
Use modern API in userauth_keyboard_interactive() (#663)
Browse files Browse the repository at this point in the history 
Files: userauth_kbd_packet.c, userauth_kbd_packet.h, test_keyboard_interactive_auth_info_request.c, userauth.c

Notes:
This refactors `SSH_MSG_USERAUTH_INFO_REQUEST` processing in `userauth_keyboard_interactive()` in order to improve robustness, correctness and readability or the code.

* Refactor userauth_keyboard_interactive to use new api for packet parsing
* add unit test for userauth_keyboard_interactive_parse_response()
* add _libssh2_get_boolean() and _libssh2_get_byte() utility functions

Credit:
xalopp
  • Loading branch information
@xalopp
xalopp committed Feb 19, 2022
1 parent ead7000 commit 83853f8
Show file tree
Hide file tree
Showing 11 changed files with 616 additions and 212 deletions.
1 change: 1 addition & 0 deletions Makefile.inc
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
CSOURCES = channel.c comp.c crypt.c hostkey.c kex.c mac.c misc.c \
packet.c publickey.c scp.c session.c sftp.c userauth.c transport.c \
userauth_kbd_packet.c \
version.c knownhost.c agent.c $(CRYPTO_CSOURCES) pem.c keepalive.c global.c \
blowfish.c bcrypt_pbkdf.c agent_win.c

Expand Down
4 changes: 2 additions & 2 deletions include/libssh2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,8 @@ typedef off_t libssh2_struct_stat_size;

typedef struct _LIBSSH2_USERAUTH_KBDINT_PROMPT
{
char *text;
unsigned int length;
unsigned char *text;
size_t length;
unsigned char echo;
} LIBSSH2_USERAUTH_KBDINT_PROMPT;

Expand Down
2 changes: 2 additions & 0 deletions src/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ set(SOURCES
sftp.h
transport.c
transport.h
userauth_kbd_packet.c
userauth_kbd_packet.h
userauth.c
userauth.h
version.c)
Expand Down
8 changes: 4 additions & 4 deletions src/libssh2_priv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -760,10 +760,10 @@ struct _LIBSSH2_SESSION
size_t userauth_kybd_data_len;
unsigned char *userauth_kybd_packet;
size_t userauth_kybd_packet_len;
unsigned int userauth_kybd_auth_name_len;
char *userauth_kybd_auth_name;
unsigned userauth_kybd_auth_instruction_len;
char *userauth_kybd_auth_instruction;
size_t userauth_kybd_auth_name_len;
unsigned char *userauth_kybd_auth_name;
size_t userauth_kybd_auth_instruction_len;
unsigned char *userauth_kybd_auth_instruction;
unsigned int userauth_kybd_num_prompts;
int userauth_kybd_auth_failure;
LIBSSH2_USERAUTH_KBDINT_PROMPT *userauth_kybd_prompts;
Expand Down
23 changes: 23 additions & 0 deletions src/misc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,29 @@ void _libssh2_string_buf_free(LIBSSH2_SESSION *session, struct string_buf *buf)
buf = NULL;
}

int _libssh2_get_byte(struct string_buf *buf, unsigned char *out)
{
if(!_libssh2_check_length(buf, 1)) {
return -1;
}

*out = buf->dataptr[0];
buf->dataptr += 1;
return 0;
}

int _libssh2_get_boolean(struct string_buf *buf, unsigned char *out)
{
if(!_libssh2_check_length(buf, 1)) {
return -1;
}


*out = buf->dataptr[0] == 0 ? 0 : 1;
buf->dataptr += 1;
return 0;
}

int _libssh2_get_u32(struct string_buf *buf, uint32_t *out)
{
if(!_libssh2_check_length(buf, 4)) {
Expand Down
2 changes: 2 additions & 0 deletions src/misc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ void _libssh2_explicit_zero(void *buf, size_t size);
struct string_buf* _libssh2_string_buf_new(LIBSSH2_SESSION *session);
void _libssh2_string_buf_free(LIBSSH2_SESSION *session,
struct string_buf *buf);
int _libssh2_get_boolean(struct string_buf *buf, unsigned char *out);
int _libssh2_get_byte(struct string_buf *buf, unsigned char *out);
int _libssh2_get_u32(struct string_buf *buf, uint32_t *out);
int _libssh2_get_u64(struct string_buf *buf, libssh2_uint64_t *out);
int _libssh2_match_string(struct string_buf *buf, const char *match);
Expand Down
212 changes: 6 additions & 206 deletions src/userauth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
#include "transport.h"
#include "session.h"
#include "userauth.h"
#include "userauth_kbd_packet.h"

/* libssh2_userauth_list
*
Expand Down Expand Up @@ -1878,13 +1879,13 @@ userauth_keyboard_interactive(LIBSSH2_SESSION * session,
((*response_callback)))
{
unsigned char *s;

int rc;

static const unsigned char reply_codes[4] = {
SSH_MSG_USERAUTH_SUCCESS,
SSH_MSG_USERAUTH_FAILURE, SSH_MSG_USERAUTH_INFO_REQUEST, 0
};
unsigned int language_tag_len;
unsigned int i;

if(session->userauth_kybd_state == libssh2_NB_state_idle) {
Expand Down Expand Up @@ -2007,215 +2008,14 @@ userauth_keyboard_interactive(LIBSSH2_SESSION * session,
}

/* server requested PAM-like conversation */
s = session->userauth_kybd_data + 1;

if(session->userauth_kybd_data_len >= 5) {
/* string name (ISO-10646 UTF-8) */
session->userauth_kybd_auth_name_len = _libssh2_ntohu32(s);
if(session->userauth_kybd_auth_name_len >
session->userauth_kybd_data_len - 5)
return _libssh2_error(session,
LIBSSH2_ERROR_OUT_OF_BOUNDARY,
"Bad keyboard auth name");
s += 4;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too small"
"to get length");
goto cleanup;
}

if(session->userauth_kybd_auth_name_len) {
session->userauth_kybd_auth_name =
LIBSSH2_ALLOC(session,
session->userauth_kybd_auth_name_len);
if(!session->userauth_kybd_auth_name) {
_libssh2_error(session, LIBSSH2_ERROR_ALLOC,
"Unable to allocate memory for "
"keyboard-interactive 'name' "
"request field");
goto cleanup;
}
if(s + session->userauth_kybd_auth_name_len <=
session->userauth_kybd_data +
session->userauth_kybd_data_len) {
memcpy(session->userauth_kybd_auth_name, s,
session->userauth_kybd_auth_name_len);
s += session->userauth_kybd_auth_name_len;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too small"
"for auth name");
goto cleanup;
}
}

if(s + 4 <= session->userauth_kybd_data +
session->userauth_kybd_data_len) {
/* string instruction (ISO-10646 UTF-8) */
session->userauth_kybd_auth_instruction_len =
_libssh2_ntohu32(s);
s += 4;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too small"
"for auth instruction length");
if(userauth_keyboard_interactive_decode_info_request(session)
< 0) {
goto cleanup;
}

if(session->userauth_kybd_auth_instruction_len) {
session->userauth_kybd_auth_instruction =
LIBSSH2_ALLOC(session,
session->userauth_kybd_auth_instruction_len);
if(!session->userauth_kybd_auth_instruction) {
_libssh2_error(session, LIBSSH2_ERROR_ALLOC,
"Unable to allocate memory for "
"keyboard-interactive 'instruction' "
"request field");
goto cleanup;
}
if(s + session->userauth_kybd_auth_instruction_len <=
session->userauth_kybd_data +
session->userauth_kybd_data_len) {
memcpy(session->userauth_kybd_auth_instruction, s,
session->userauth_kybd_auth_instruction_len);
s += session->userauth_kybd_auth_instruction_len;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too small"
"for auth instruction");
goto cleanup;
}
}

if(s + 4 <= session->userauth_kybd_data +
session->userauth_kybd_data_len) {
/* string language tag (as defined in [RFC-3066]) */
language_tag_len = _libssh2_ntohu32(s);
s += 4;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too small"
"for auth language tag length");
goto cleanup;
}

if(s + language_tag_len <= session->userauth_kybd_data +
session->userauth_kybd_data_len) {
/* ignoring this field as deprecated */
s += language_tag_len;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too small"
"for auth language tag");
goto cleanup;
}

if(s + 4 <= session->userauth_kybd_data +
session->userauth_kybd_data_len) {
/* int num-prompts */
session->userauth_kybd_num_prompts = _libssh2_ntohu32(s);
s += 4;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too small"
"for auth num keyboard prompts");
goto cleanup;
}

if(session->userauth_kybd_num_prompts > 100) {
_libssh2_error(session, LIBSSH2_ERROR_OUT_OF_BOUNDARY,
"Too many replies for "
"keyboard-interactive prompts");
goto cleanup;
}

if(session->userauth_kybd_num_prompts) {
session->userauth_kybd_prompts =
LIBSSH2_CALLOC(session,
sizeof(LIBSSH2_USERAUTH_KBDINT_PROMPT) *
session->userauth_kybd_num_prompts);
if(!session->userauth_kybd_prompts) {
_libssh2_error(session, LIBSSH2_ERROR_ALLOC,
"Unable to allocate memory for "
"keyboard-interactive prompts array");
goto cleanup;
}

session->userauth_kybd_responses =
LIBSSH2_CALLOC(session,
sizeof(LIBSSH2_USERAUTH_KBDINT_RESPONSE) *
session->userauth_kybd_num_prompts);
if(!session->userauth_kybd_responses) {
_libssh2_error(session, LIBSSH2_ERROR_ALLOC,
"Unable to allocate memory for "
"keyboard-interactive responses array");
goto cleanup;
}

for(i = 0; i < session->userauth_kybd_num_prompts; i++) {
if(s + 4 <= session->userauth_kybd_data +
session->userauth_kybd_data_len) {
/* string prompt[1] (ISO-10646 UTF-8) */
session->userauth_kybd_prompts[i].length =
_libssh2_ntohu32(s);
s += 4;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too "
"small for auth keyboard "
"prompt length");
goto cleanup;
}

session->userauth_kybd_prompts[i].text =
LIBSSH2_CALLOC(session,
session->userauth_kybd_prompts[i].
length);
if(!session->userauth_kybd_prompts[i].text) {
_libssh2_error(session, LIBSSH2_ERROR_ALLOC,
"Unable to allocate memory for "
"keyboard-interactive prompt message");
goto cleanup;
}

if(s + session->userauth_kybd_prompts[i].length <=
session->userauth_kybd_data +
session->userauth_kybd_data_len) {
memcpy(session->userauth_kybd_prompts[i].text, s,
session->userauth_kybd_prompts[i].length);
s += session->userauth_kybd_prompts[i].length;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too "
"small for auth keyboard prompt");
goto cleanup;
}
if(s < session->userauth_kybd_data +
session->userauth_kybd_data_len) {
/* boolean echo[1] */
session->userauth_kybd_prompts[i].echo = *s++;
}
else {
_libssh2_error(session, LIBSSH2_ERROR_BUFFER_TOO_SMALL,
"userauth keyboard data buffer too "
"small for auth keyboard prompt echo");
goto cleanup;
}
}
}

response_callback(session->userauth_kybd_auth_name,
response_callback((const char *)session->userauth_kybd_auth_name,
session->userauth_kybd_auth_name_len,
(const char *)
session->userauth_kybd_auth_instruction,
session->userauth_kybd_auth_instruction_len,
session->userauth_kybd_num_prompts,
Expand Down

0 comments on commit 83853f8

Please sign in to comment.