From 478408c01006f39ab916675bc84b8a7340bae086 Mon Sep 17 00:00:00 2001 From: Jacques Germishuys Date: Thu, 17 Apr 2014 23:03:44 +0200 Subject: [PATCH 1/4] Introduce git_cred_ssh_interactive_new() This allows for keyboard-interactive based SSH authentication --- include/git2/transport.h | 30 ++++++++++++++++++++++++++++++ src/transports/cred.c | 36 ++++++++++++++++++++++++++++++++++++ src/transports/ssh.c | 22 ++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/include/git2/transport.h b/include/git2/transport.h index 1f4d03eea..eba08cd27 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -41,6 +41,9 @@ typedef enum { /* git_cred_default */ GIT_CREDTYPE_DEFAULT = (1u << 3), + + /* git_cred_ssh_interactive */ + GIT_CREDTYPE_SSH_INTERACTIVE = (1u << 4), } git_credtype_t; /* The base structure for all credential types */ @@ -60,8 +63,10 @@ typedef struct { #ifdef GIT_SSH typedef LIBSSH2_USERAUTH_PUBLICKEY_SIGN_FUNC((*git_cred_sign_callback)); +typedef LIBSSH2_USERAUTH_KBDINT_RESPONSE_FUNC((*git_cred_ssh_interactive_callback)); #else typedef int (*git_cred_sign_callback)(void *, ...); +typedef int (*git_cred_ssh_interactive_callback)(void *, ...); #endif /** @@ -75,6 +80,16 @@ typedef struct git_cred_ssh_key { char *passphrase; } git_cred_ssh_key; +/** + * Keyboard-interactive based ssh authentication + */ +typedef struct git_cred_ssh_interactive { + git_cred parent; + char *username; + void *prompt_callback; + void *payload; +} git_cred_ssh_interactive; + /** * A key with a custom signature function */ @@ -130,6 +145,21 @@ GIT_EXTERN(int) git_cred_ssh_key_new( const char *privatekey, const char *passphrase); +/** + * Create a new ssh keyboard-interactive based credential object. + * The supplied credential parameter will be internally duplicated. + * + * @param username Username to use to authenticate. + * @param prompt_callback The callback method used for prompts. + * @param payload Additional data to pass to the callback. + * @return 0 for success or an error code for failure. + */ +GIT_EXTERN(int) git_cred_ssh_interactive_new( + git_cred **out, + const char *username, + git_cred_ssh_interactive_callback prompt_callback, + void *payload); + /** * Create a new ssh key credential object used for querying an ssh-agent. * The supplied credential parameter will be internally duplicated. diff --git a/src/transports/cred.c b/src/transports/cred.c index 460ed04a2..528d6af73 100644 --- a/src/transports/cred.c +++ b/src/transports/cred.c @@ -87,6 +87,16 @@ static void ssh_key_free(struct git_cred *cred) git__free(c); } +static void ssh_interactive_free(struct git_cred *cred) +{ + git_cred_ssh_interactive *c = (git_cred_ssh_interactive *)cred; + + git__free(c->username); + + git__memzero(c, sizeof(*c)); + git__free(c); +} + static void ssh_custom_free(struct git_cred *cred) { git_cred_ssh_custom *c = (git_cred_ssh_custom *)cred; @@ -142,6 +152,32 @@ int git_cred_ssh_key_new( return 0; } +int git_cred_ssh_interactive_new( + git_cred **out, + const char *username, + git_cred_ssh_interactive_callback prompt_callback, + void *payload) +{ + git_cred_ssh_interactive *c; + + assert(out && username && prompt_callback); + + c = git__calloc(1, sizeof(git_cred_ssh_interactive)); + GITERR_CHECK_ALLOC(c); + + c->parent.credtype = GIT_CREDTYPE_SSH_INTERACTIVE; + c->parent.free = ssh_interactive_free; + + c->username = git__strdup(username); + GITERR_CHECK_ALLOC(c->username); + + c->prompt_callback = prompt_callback; + c->payload = payload; + + *out = &c->parent; + return 0; +} + int git_cred_ssh_key_from_agent(git_cred **cred, const char *username) { git_cred_ssh_key *c; diff --git a/src/transports/ssh.c b/src/transports/ssh.c index 879af9059..48e51f2ae 100644 --- a/src/transports/ssh.c +++ b/src/transports/ssh.c @@ -313,6 +313,27 @@ static int _git_ssh_authenticate_session( c->publickey_len, c->sign_callback, &c->sign_data); break; } + case GIT_CREDTYPE_SSH_INTERACTIVE: { + void **abstract = libssh2_session_abstract(session); + git_cred_ssh_interactive *c = (git_cred_ssh_interactive *)cred; + + /* ideally, we should be able to set this by calling + * libssh2_session_init_ex() instead of libssh2_session_init(). + * libssh2's API is inconsistent here i.e. libssh2_userauth_publickey() + * allows you to pass the `abstract` as part of the call, whereas + * libssh2_userauth_keyboard_interactive() does not! + * + * The only way to set the `abstract` pointer is by calling + * libssh2_session_abstract(), which will replace the existing + * pointer as is done below. This is safe for now (at time of writing), + * but may not be valid in future. + */ + *abstract = c->payload; + + rc = libssh2_userauth_keyboard_interactive( + session, c->username, c->prompt_callback); + break; + } default: rc = LIBSSH2_ERROR_AUTHENTICATION_FAILED; } @@ -397,6 +418,7 @@ static int _git_ssh_setup_conn( &t->cred, t->owner->url, user, GIT_CREDTYPE_USERPASS_PLAINTEXT | GIT_CREDTYPE_SSH_KEY | + GIT_CREDTYPE_SSH_INTERACTIVE | GIT_CREDTYPE_SSH_CUSTOM, t->owner->cred_acquire_payload) < 0) goto on_error; From 8ec0a5527333afeca3f4ff3bf36fb8e1ac1c5939 Mon Sep 17 00:00:00 2001 From: Jacques Germishuys Date: Fri, 18 Apr 2014 00:49:07 +0200 Subject: [PATCH 2/4] Make git_cred_ssh_custom_new() naming more consistent --- include/git2/transport.h | 10 +++++----- src/transports/cred.c | 4 ++-- src/transports/ssh.c | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/git2/transport.h b/include/git2/transport.h index eba08cd27..80299c41c 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -99,7 +99,7 @@ typedef struct git_cred_ssh_custom { char *publickey; size_t publickey_len; void *sign_callback; - void *sign_data; + void *payload; } git_cred_ssh_custom; /** A key for NTLM/Kerberos "default" credentials */ @@ -186,8 +186,8 @@ GIT_EXTERN(int) git_cred_ssh_key_from_agent( * @param username username to use to authenticate * @param publickey The bytes of the public key. * @param publickey_len The length of the public key in bytes. - * @param sign_fn The callback method to sign the data during the challenge. - * @param sign_data The data to pass to the sign function. + * @param sign_callback The callback method to sign the data during the challenge. + * @param payload Additional data to pass to the callback. * @return 0 for success or an error code for failure */ GIT_EXTERN(int) git_cred_ssh_custom_new( @@ -195,8 +195,8 @@ GIT_EXTERN(int) git_cred_ssh_custom_new( const char *username, const char *publickey, size_t publickey_len, - git_cred_sign_callback sign_fn, - void *sign_data); + git_cred_sign_callback sign_callback, + void *payload); /** * Create a "default" credential usable for Negotiate mechanisms like NTLM diff --git a/src/transports/cred.c b/src/transports/cred.c index 528d6af73..05090ba8a 100644 --- a/src/transports/cred.c +++ b/src/transports/cred.c @@ -204,7 +204,7 @@ int git_cred_ssh_custom_new( const char *publickey, size_t publickey_len, git_cred_sign_callback sign_callback, - void *sign_data) + void *payload) { git_cred_ssh_custom *c; @@ -228,7 +228,7 @@ int git_cred_ssh_custom_new( c->publickey_len = publickey_len; c->sign_callback = sign_callback; - c->sign_data = sign_data; + c->payload = payload; *cred = &c->parent; return 0; diff --git a/src/transports/ssh.c b/src/transports/ssh.c index 48e51f2ae..dea990275 100644 --- a/src/transports/ssh.c +++ b/src/transports/ssh.c @@ -310,7 +310,7 @@ static int _git_ssh_authenticate_session( rc = libssh2_userauth_publickey( session, c->username, (const unsigned char *)c->publickey, - c->publickey_len, c->sign_callback, &c->sign_data); + c->publickey_len, c->sign_callback, &c->payload); break; } case GIT_CREDTYPE_SSH_INTERACTIVE: { From 043112dc1c87433f92e1ea3b3ab76efe62edc448 Mon Sep 17 00:00:00 2001 From: Jacques Germishuys Date: Fri, 18 Apr 2014 17:57:39 +0200 Subject: [PATCH 3/4] Replace void * with proper callback types --- include/git2/transport.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/git2/transport.h b/include/git2/transport.h index 80299c41c..1665f97b3 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -86,7 +86,7 @@ typedef struct git_cred_ssh_key { typedef struct git_cred_ssh_interactive { git_cred parent; char *username; - void *prompt_callback; + git_cred_ssh_interactive_callback prompt_callback; void *payload; } git_cred_ssh_interactive; @@ -98,7 +98,7 @@ typedef struct git_cred_ssh_custom { char *username; char *publickey; size_t publickey_len; - void *sign_callback; + git_cred_sign_callback sign_callback; void *payload; } git_cred_ssh_custom; From a622ff17a1d4c70686959eefd03214898794c792 Mon Sep 17 00:00:00 2001 From: Jacques Germishuys Date: Fri, 18 Apr 2014 20:05:28 +0200 Subject: [PATCH 4/4] Only zero sensitive information on destruction (and memory actually allocated by us) --- src/transports/cred.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/transports/cred.c b/src/transports/cred.c index 05090ba8a..913ec36cc 100644 --- a/src/transports/cred.c +++ b/src/transports/cred.c @@ -30,7 +30,6 @@ static void plaintext_free(struct git_cred *cred) git__free(c->password); } - git__memzero(c, sizeof(*c)); git__free(c); } @@ -73,8 +72,13 @@ static void ssh_key_free(struct git_cred *cred) (git_cred_ssh_key *)cred; git__free(c->username); - git__free(c->publickey); - git__free(c->privatekey); + + if (c->privatekey) { + /* Zero the memory which previously held the private key */ + size_t key_len = strlen(c->privatekey); + git__memzero(c->privatekey, key_len); + git__free(c->privatekey); + } if (c->passphrase) { /* Zero the memory which previously held the passphrase */ @@ -83,7 +87,13 @@ static void ssh_key_free(struct git_cred *cred) git__free(c->passphrase); } - git__memzero(c, sizeof(*c)); + if (c->publickey) { + /* Zero the memory which previously held the public key */ + size_t key_len = strlen(c->publickey); + git__memzero(c->publickey, key_len); + git__free(c->publickey); + } + git__free(c); } @@ -93,7 +103,6 @@ static void ssh_interactive_free(struct git_cred *cred) git__free(c->username); - git__memzero(c, sizeof(*c)); git__free(c); } @@ -102,9 +111,14 @@ static void ssh_custom_free(struct git_cred *cred) git_cred_ssh_custom *c = (git_cred_ssh_custom *)cred; git__free(c->username); - git__free(c->publickey); - git__memzero(c, sizeof(*c)); + if (c->publickey) { + /* Zero the memory which previously held the publickey */ + size_t key_len = strlen(c->publickey); + git__memzero(c->publickey, key_len); + git__free(c->publickey); + } + git__free(c); }