From 80fc7d6bf08834acc5696c8f0c73680236f84375 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 13 Nov 2013 16:46:45 -0500 Subject: [PATCH 1/2] Propagate auth error codes as GIT_EUSER in winhttp --- src/remote.c | 71 ++++++++++++++++++++------------- src/transports/smart_protocol.c | 18 +++++---- src/transports/winhttp.c | 4 +- tests/online/clone.c | 33 +++++++++++++++ 4 files changed, 89 insertions(+), 37 deletions(-) diff --git a/src/remote.c b/src/remote.c index c0f35e5ea..d072eb9d5 100644 --- a/src/remote.c +++ b/src/remote.c @@ -82,29 +82,35 @@ static int ensure_remote_name_is_valid(const char *name) return error; } -static int get_check_cert(git_repository *repo) +static int get_check_cert(int *out, git_repository *repo) { git_config *cfg; const char *val; - int check_cert; + int error = 0; - assert(repo); + assert(out && repo); + + /* By default, we *DO* want to verify the certificate. */ + *out = 1; /* Go through the possible sources for SSL verification settings, from * most specific to least specific. */ /* GIT_SSL_NO_VERIFY environment variable */ - if ((val = getenv("GIT_SSL_NO_VERIFY")) && - !git_config_parse_bool(&check_cert, val)) - return !check_cert; + if (val = getenv("GIT_SSL_NO_VERIFY")) + return git_config_parse_bool(out, val); /* http.sslVerify config setting */ - if (!git_repository_config__weakptr(&cfg, repo) && - !git_config_get_bool(&check_cert, cfg, "http.sslVerify")) - return check_cert; + if ((error = git_repository_config__weakptr(&cfg, repo)) < 0) + return error; - /* By default, we *DO* want to verify the certificate. */ - return 1; + if ((error = git_config_get_bool(out, cfg, "http.sslVerify")) == 0) + return 0; + else if (error != GIT_ENOTFOUND) + return error; + + giterr_clear(); + return 0; } static int create_internal(git_remote **out, git_repository *repo, const char *name, const char *url, const char *fetch) @@ -120,9 +126,11 @@ static int create_internal(git_remote **out, git_repository *repo, const char *n GITERR_CHECK_ALLOC(remote); remote->repo = repo; - remote->check_cert = (unsigned)get_check_cert(repo); remote->update_fetchhead = 1; + if (get_check_cert(&remote->check_cert, repo) < 0) + goto on_error; + if (git_vector_init(&remote->refs, 32, NULL) < 0) goto on_error; @@ -314,11 +322,13 @@ int git_remote_load(git_remote **out, git_repository *repo, const char *name) GITERR_CHECK_ALLOC(remote); memset(remote, 0x0, sizeof(git_remote)); - remote->check_cert = (unsigned)get_check_cert(repo); remote->update_fetchhead = 1; remote->name = git__strdup(name); GITERR_CHECK_ALLOC(remote->name); + if ((error = get_check_cert(&remote->check_cert, repo)) < 0) + goto cleanup; + if ((git_vector_init(&remote->refs, 32, NULL) < 0) || (git_vector_init(&remote->refspecs, 2, NULL) < 0) || (git_vector_init(&remote->active_refspecs, 2, NULL) < 0)) { @@ -610,13 +620,14 @@ int git_remote_connect(git_remote *remote, git_direction direction) git_transport *t; const char *url; int flags = GIT_TRANSPORTFLAGS_NONE; + int error; assert(remote); t = remote->transport; url = git_remote__urlfordirection(remote, direction); - if (url == NULL ) { + if (url == NULL) { giterr_set(GITERR_INVALID, "Malformed remote '%s' - missing URL", remote->name); return -1; @@ -624,17 +635,17 @@ int git_remote_connect(git_remote *remote, git_direction direction) /* A transport could have been supplied in advance with * git_remote_set_transport */ - if (!t && git_transport_new(&t, remote, url) < 0) - return -1; + if (!t && (error = git_transport_new(&t, remote, url)) < 0) + return error; if (t->set_callbacks && - t->set_callbacks(t, remote->callbacks.progress, NULL, remote->callbacks.payload) < 0) + (error = t->set_callbacks(t, remote->callbacks.progress, NULL, remote->callbacks.payload)) < 0) goto on_error; if (!remote->check_cert) flags |= GIT_TRANSPORTFLAGS_NO_CHECK_CERT; - if (t->connect(t, url, remote->callbacks.credentials, remote->callbacks.payload, direction, flags) < 0) + if ((error = t->connect(t, url, remote->callbacks.credentials, remote->callbacks.payload, direction, flags)) < 0) goto on_error; remote->transport = t; @@ -647,7 +658,7 @@ on_error: if (t == remote->transport) remote->transport = NULL; - return -1; + return error; } int git_remote_ls(const git_remote_head ***out, size_t *size, git_remote *remote) @@ -661,6 +672,7 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_ur { git_config *cfg; const char *val; + int error; assert(remote); @@ -669,8 +681,8 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_ur *proxy_url = NULL; - if (git_repository_config__weakptr(&cfg, remote->repo) < 0) - return -1; + if ((error = git_repository_config__weakptr(&cfg, remote->repo)) < 0) + return error; /* Go through the possible sources for proxy configuration, from most specific * to least specific. */ @@ -679,28 +691,33 @@ int git_remote__get_http_proxy(git_remote *remote, bool use_ssl, char **proxy_ur if (remote->name && 0 != *(remote->name)) { git_buf buf = GIT_BUF_INIT; - if (git_buf_printf(&buf, "remote.%s.proxy", remote->name) < 0) - return -1; + if ((error = git_buf_printf(&buf, "remote.%s.proxy", remote->name)) < 0) + return error; - if (!git_config_get_string(&val, cfg, git_buf_cstr(&buf)) && + if ((error = git_config_get_string(&val, cfg, git_buf_cstr(&buf))) == 0 && val && ('\0' != *val)) { git_buf_free(&buf); *proxy_url = git__strdup(val); GITERR_CHECK_ALLOC(*proxy_url); return 0; - } + } else if (error != GIT_ENOTFOUND) + return error; + giterr_clear(); git_buf_free(&buf); } /* http.proxy config setting */ - if (!git_config_get_string(&val, cfg, "http.proxy") && + if ((error = git_config_get_string(&val, cfg, "http.proxy")) == 0 && val && ('\0' != *val)) { *proxy_url = git__strdup(val); GITERR_CHECK_ALLOC(*proxy_url); return 0; - } + } else if (error != GIT_ENOTFOUND) + return error; + + giterr_clear(); /* HTTP_PROXY / HTTPS_PROXY environment variables */ val = use_ssl ? getenv("HTTPS_PROXY") : getenv("HTTP_PROXY"); diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 7288a4820..3bf1f9329 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -49,7 +49,7 @@ int git_smart__store_refs(transport_smart *t, int flushes) if (error == GIT_EBUFS) { if ((recvd = gitno_recv(buf)) < 0) - return -1; + return recvd; if (recvd == 0 && !flush) { giterr_set(GITERR_NET, "Early EOF"); @@ -164,10 +164,10 @@ static int recv_pkt(git_pkt **out, gitno_buffer *buf) break; /* return the pkt */ if (error < 0 && error != GIT_EBUFS) - return -1; + return error; if ((ret = gitno_recv(buf)) < 0) - return -1; + return ret; } while (error); gitno_consume(buf, line_end); @@ -184,10 +184,11 @@ static int store_common(transport_smart *t) { git_pkt *pkt = NULL; gitno_buffer *buf = &t->buffer; + int error; do { - if (recv_pkt(&pkt, buf) < 0) - return -1; + if ((error = recv_pkt(&pkt, buf)) < 0) + return error; if (pkt->type == GIT_PKT_ACK) { if (git_vector_insert(&t->common, pkt) < 0) @@ -227,6 +228,7 @@ static int fetch_setup_walk(git_revwalk **out, git_repository *repo) if (git_reference_type(ref) == GIT_REF_SYMBOLIC) continue; + if (git_revwalk_push(walk, git_reference_target(ref)) < 0) goto on_error; @@ -436,10 +438,10 @@ static int no_sideband(transport_smart *t, struct git_odb_writepack *writepack, gitno_consume_n(buf, buf->offset); if ((recvd = gitno_recv(buf)) < 0) - return -1; + return recvd; } while(recvd > 0); - if (writepack->commit(writepack, stats)) + if (writepack->commit(writepack, stats) < 0) return -1; return 0; @@ -697,7 +699,7 @@ static int parse_report(gitno_buffer *buf, git_push *push) if (error == GIT_EBUFS) { if ((recvd = gitno_recv(buf)) < 0) - return -1; + return recvd; if (recvd == 0) { giterr_set(GITERR_NET, "Early EOF"); diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index a25fa301e..f7566458e 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -640,8 +640,8 @@ replay: (!t->cred || 0 == (t->cred->credtype & allowed_types))) { if (t->owner->cred_acquire_cb(&t->cred, t->owner->url, t->connection_data.user, allowed_types, - t->owner->cred_acquire_payload) < 0) - return -1; + t->owner->cred_acquire_payload) < 0) + return GIT_EUSER; assert(t->cred); diff --git a/tests/online/clone.c b/tests/online/clone.c index aa3d6b26a..d036a5a47 100644 --- a/tests/online/clone.c +++ b/tests/online/clone.c @@ -183,6 +183,39 @@ void test_online_clone__custom_remote_callbacks(void) cl_assert(callcount > 0); } +static int cred_failure_cb( + git_cred **cred, + const char *url, + const char *username_from_url, + unsigned int allowed_types, + void *data) +{ + return -1; +} + +void test_online_clone__cred_callback_failure_is_euser(void) +{ + const char *remote_url = cl_getenv("GITTEST_REMOTE_URL"); + const char *remote_user = cl_getenv("GITTEST_REMOTE_USER"); + const char *remote_default = cl_getenv("GITTEST_REMOTE_DEFAULT"); + int error; + + if (!remote_url) { + printf("GITTEST_REMOTE_URL unset; skipping clone test\n"); + return; + } + + if (!remote_user && !remote_default) { + printf("GITTEST_REMOTE_USER and GITTEST_REMOTE_DEFAULT unset; skipping clone test\n"); + return; + } + + g_options.remote_callbacks.credentials = cred_failure_cb; + + cl_git_fail(error = git_clone(&g_repo, remote_url, "./foo", &g_options)); + cl_assert_equal_i(error, GIT_EUSER); +} + void test_online_clone__credentials(void) { /* Remote URL environment variable must be set. User and password are optional. */ From 84efffc33ab352d68cf981da0d6c8d358244cabc Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 13 Nov 2013 16:57:51 -0500 Subject: [PATCH 2/2] Introduce git_cred_default for NTLM/SPNEGO auth --- include/git2/transport.h | 20 +++++++++++++++++--- src/transports/cred.c | 27 +++++++++++++++++++++++++++ src/transports/winhttp.c | 28 ++++++++++++++++++++++++++++ tests/online/push.c | 16 +++++++++++++++- 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/include/git2/transport.h b/include/git2/transport.h index 81ebf4dc9..fe4883f99 100644 --- a/include/git2/transport.h +++ b/include/git2/transport.h @@ -31,13 +31,16 @@ GIT_BEGIN_DECL /** Authentication type requested */ typedef enum { /* git_cred_userpass_plaintext */ - GIT_CREDTYPE_USERPASS_PLAINTEXT = (1u << 0), + GIT_CREDTYPE_USERPASS_PLAINTEXT = (1u << 0), /* git_cred_ssh_key */ GIT_CREDTYPE_SSH_KEY = (1u << 1), /* git_cred_ssh_custom */ - GIT_CREDTYPE_SSH_CUSTOM = (1u << 2), + GIT_CREDTYPE_SSH_CUSTOM = (1u << 2), + + /* git_cred_default */ + GIT_CREDTYPE_DEFAULT = (1u << 3), } git_credtype_t; /* The base structure for all credential types */ @@ -48,7 +51,7 @@ struct git_cred { void (*free)(git_cred *cred); }; -/* A plaintext username and password */ +/** A plaintext username and password */ typedef struct { git_cred parent; char *username; @@ -84,6 +87,9 @@ typedef struct git_cred_ssh_custom { void *sign_data; } git_cred_ssh_custom; +/** A key for NTLM/Kerberos "default" credentials */ +typedef struct git_cred git_cred_default; + /** * Check whether a credential object contains username information. * @@ -150,6 +156,14 @@ GIT_EXTERN(int) git_cred_ssh_custom_new( git_cred_sign_callback sign_fn, void *sign_data); +/** + * Create a "default" credential usable for Negotiate mechanisms like NTLM + * or Kerberos authentication. + * + * @return 0 for success or an error code for failure + */ +GIT_EXTERN(int) git_cred_default_new(git_cred **out); + /** * Signature of a function which acquires a credential object. * diff --git a/src/transports/cred.c b/src/transports/cred.c index cc7fdab4b..05d2c8dc6 100644 --- a/src/transports/cred.c +++ b/src/transports/cred.c @@ -29,6 +29,10 @@ int git_cred_has_username(git_cred *cred) ret = !!c->username; break; } + case GIT_CREDTYPE_DEFAULT: { + ret = 0; + break; + } } return ret; @@ -115,6 +119,13 @@ static void ssh_custom_free(struct git_cred *cred) git__free(c); } +static void default_free(struct git_cred *cred) +{ + git_cred_default *c = (git_cred_default *)cred; + + git__free(c); +} + int git_cred_ssh_key_new( git_cred **cred, const char *username, @@ -191,3 +202,19 @@ int git_cred_ssh_custom_new( *cred = &c->parent; return 0; } + +int git_cred_default_new(git_cred **cred) +{ + git_cred_default *c; + + assert(cred); + + c = git__calloc(1, sizeof(git_cred_default)); + GITERR_CHECK_ALLOC(c); + + c->credtype = GIT_CREDTYPE_DEFAULT; + c->free = default_free; + + *cred = c; + return 0; +} diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index f7566458e..673cd0faf 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -52,6 +52,7 @@ static const int no_check_cert_flags = SECURITY_FLAG_IGNORE_CERT_CN_INVALID | typedef enum { GIT_WINHTTP_AUTH_BASIC = 1, + GIT_WINHTTP_AUTH_NEGOTIATE = 2, } winhttp_authmechanism_t; typedef struct { @@ -138,6 +139,22 @@ on_error: return error; } +static int apply_default_credentials(HINTERNET request) +{ + /* If we are explicitly asked to deliver default credentials, turn set + * the security level to low which will guarantee they are delivered. + * The default is "medium" which applies to the intranet and sounds + * like it would correspond to Internet Explorer security zones, but + * in fact does not. + */ + DWORD data = WINHTTP_AUTOLOGON_SECURITY_LEVEL_LOW; + + if (!WinHttpSetOption(request, WINHTTP_OPTION_AUTOLOGON_POLICY, &data, sizeof(DWORD))) + return -1; + + return 0; +} + static int winhttp_stream_connect(winhttp_stream *s) { winhttp_subtransport *t = OWNING_SUBTRANSPORT(s); @@ -317,6 +334,11 @@ static int winhttp_stream_connect(winhttp_stream *s) t->auth_mechanism == GIT_WINHTTP_AUTH_BASIC && apply_basic_credential(s->request, t->cred) < 0) goto on_error; + else if (t->cred && + t->cred->credtype == GIT_CREDTYPE_DEFAULT && + t->auth_mechanism == GIT_WINHTTP_AUTH_NEGOTIATE && + apply_default_credentials(s->request) < 0) + goto on_error; /* If no other credentials have been applied and the URL has username and * password, use those */ @@ -361,6 +383,12 @@ static int parse_unauthorized_response( *auth_mechanism = GIT_WINHTTP_AUTH_BASIC; } + if ((WINHTTP_AUTH_SCHEME_NTLM & supported) || + (WINHTTP_AUTH_SCHEME_NEGOTIATE & supported)) { + *allowed_types |= GIT_CREDTYPE_DEFAULT; + *auth_mechanism = GIT_WINHTTP_AUTH_NEGOTIATE; + } + return 0; } diff --git a/tests/online/push.c b/tests/online/push.c index aeb1ab47d..be505c3a1 100644 --- a/tests/online/push.c +++ b/tests/online/push.c @@ -9,14 +9,17 @@ static git_repository *_repo; +static char *_remote_url; + static char *_remote_ssh_key; static char *_remote_ssh_pubkey; static char *_remote_ssh_passphrase; -static char *_remote_url; static char *_remote_user; static char *_remote_pass; +static char *_remote_default; + static int cred_acquire_cb(git_cred **, const char *, const char *, unsigned int, void *); static git_remote *_remote; @@ -47,11 +50,21 @@ static int cred_acquire_cb( GIT_UNUSED(user_from_url); GIT_UNUSED(payload); + if (GIT_CREDTYPE_DEFAULT & allowed_types) { + if (!_remote_default) { + printf("GITTEST_REMOTE_DEFAULT must be set to use NTLM/Negotiate credentials\n"); + return -1; + } + + return git_cred_default_new(cred); + } + if (GIT_CREDTYPE_SSH_KEY & allowed_types) { if (!_remote_user || !_remote_ssh_pubkey || !_remote_ssh_key || !_remote_ssh_passphrase) { printf("GITTEST_REMOTE_USER, GITTEST_REMOTE_SSH_PUBKEY, GITTEST_REMOTE_SSH_KEY and GITTEST_REMOTE_SSH_PASSPHRASE must be set\n"); return -1; } + return git_cred_ssh_key_new(cred, _remote_user, _remote_ssh_pubkey, _remote_ssh_key, _remote_ssh_passphrase); } @@ -298,6 +311,7 @@ void test_online_push__initialize(void) _remote_ssh_key = cl_getenv("GITTEST_REMOTE_SSH_KEY"); _remote_ssh_pubkey = cl_getenv("GITTEST_REMOTE_SSH_PUBKEY"); _remote_ssh_passphrase = cl_getenv("GITTEST_REMOTE_SSH_PASSPHRASE"); + _remote_default = cl_getenv("GITTEST_REMOTE_DEFAULT"); _remote = NULL; if (_remote_url) {