From 7bc1d0d62ae9037ccf86bd16bb5acdbd88a17fd8 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix Date: Sat, 19 Jun 2010 16:29:45 -0700 Subject: [PATCH] security: Rewrite of the file. List of changes: - removed out_buffer from struct connectdata as it was never filled, removed dead code that dealt with it - fixed the #if header guard to match the other files - splitted some of the code in specialized functions (ftp_send_command, choose_mech) - added tons of comments (both for the methods and in the code) - renamed some of the methods to better match the usage - added lots of FIXME's - fixed a potential memory leak in sec_get_data - tried to remove some implicit conversion int -> size_t in the code - removed usage of read replaced by Curl_read_plain - removed usage of write replaced by Curl_write_plain - globally extended the usage of CURLcode in the internal code - removed notice in README --- README | 6 - lib/security.c | 642 ++++++++++++++++++++++++++++++-------------------------- lib/urldata.h | 3 +- 3 files changed, 348 insertions(+), 303 deletions(-) diff --git a/README b/README index 2ffacc3..551c68d 100644 --- a/README +++ b/README @@ -41,9 +41,3 @@ GIT git clone git://github.com/bagder/curl.git (you'll get a directory named curl created, filled with the source code) - -NOTICE - - Curl contains pieces of source code that is Copyright (c) 1998, 1999 - Kungliga Tekniska Högskolan. This notice is included here to comply with the - distribution terms. diff --git a/lib/security.c b/lib/security.c index eceb013..799dfda 100644 --- a/lib/security.c +++ b/lib/security.c @@ -1,76 +1,61 @@ -/* This source code was modified by Martin Hedenfalk for - * use in Curl. His latest changes were done 2000-09-18. +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| * - * It has since been patched and modified a lot by Daniel Stenberg - * to make it better applied to curl conditions, and to make - * it not use globals, pollute name space and more. This source code awaits a - * rewrite to work around the paragraph 2 in the BSD licenses as explained - * below. + * Copyright (C) 2010, Daniel Stenberg, , et al. * - * Copyright (c) 1998, 1999 Kungliga Tekniska Högskolan - * (Royal Institute of Technology, Stockholm, Sweden). - * - * Copyright (C) 2001 - 2010, Daniel Stenberg, , et al. - * - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at http://curl.haxx.se/docs/copyright.html. * - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. * - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. * - * 3. Neither the name of the Institute nor the names of its contributors - * may be used to endorse or promote products derived from this software - * without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE INSTITUTE AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE INSTITUTE OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. */ + ***************************************************************************/ +/* Based on previous work by Kungliga Tekniska Högskolan + * (Royal Institute of Technology, Stockholm, Sweden). + */ #include "setup.h" #ifndef CURL_DISABLE_FTP #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) -#define _MPRINTF_REPLACE /* we want curl-functions instead of native ones */ -#include - -#include -#include - -#ifdef HAVE_NETDB_H -#include -#endif - -#ifdef HAVE_UNISTD_H -#include -#endif - #include "urldata.h" -#include "krb4.h" #include "curl_base64.h" +#include "krb4.h" #include "sendf.h" -#include "ftp.h" -#include "curl_memory.h" -#include "rawstr.h" /* The last #include file should be: */ #include "memdebug.h" +static char level_to_char(int level) { + switch(level) { + case prot_clear: + return 'C'; + case prot_safe: + return 'S'; + case prot_confidential: + return 'E'; + case prot_private: + return 'P'; + case prot_cmd: + /* Fall through */ + default: + /* Those 2 cases should not be reached! */ + break; + } + DEBUGASSERT(0); + return 'C'; +} + static const struct { enum protection_level level; const char *name; @@ -81,106 +66,123 @@ static const struct { { prot_private, "private" } }; -static enum protection_level -name_to_level(const char *name) +static int name_to_level(const char *level) { - int i; - for(i = 0; i < (int)sizeof(level_names)/(int)sizeof(level_names[0]); i++) - if(checkprefix(name, level_names[i].name)) + size_t i; + + for(i = 0; i < sizeof(level_names)/sizeof(level_names[0]); ++i) + if(!strcmp(level, level_names[i].name)) return level_names[i].level; - return (enum protection_level)-1; + + return -1; } static const struct Curl_sec_client_mech * const mechs[] = { -#ifdef HAVE_GSSAPI +#if defined(HAVE_GSSAPI) &Curl_krb5_client_mech, #endif -#ifdef HAVE_KRB4 +#if defined(HAVE_KRB4) &Curl_krb4_client_mech, #endif NULL }; -static int -block_read(int fd, void *buf, size_t len) +/* Read |len| from the socket |fd| and store it in |to|. Return a + CURLcode saying whether an error occured or CURLE_OK if |len| was read. */ +static CURLcode +socket_read(curl_socket_t fd, void *to, size_t len) { - unsigned char *p = buf; - int b; - while(len) { - b = read(fd, p, len); - if(b == 0) - return 0; - else if(b < 0 && (errno == EINTR || errno == EAGAIN)) - /* TODO: this will busy loop in the EAGAIN case */ - continue; - else if(b < 0) - return -1; - len -= b; - p += b; + char *to_p = to; + CURLcode code; + ssize_t nread; + + while(len > 0) { + code = Curl_read_plain(fd, to_p, len, &nread); + if(code == CURLE_OK) { + len -= nread; + to_p += nread; + } + else { + /* FIXME: We are doing a busy wait */ + if(code == CURLE_AGAIN) + continue; + return code; + } } - return p - (unsigned char*)buf; + return CURLE_OK; } -static int -block_write(int fd, const void *buf, size_t len) +/* Write |len| bytes from the buffer |to| to the socket |fd|. Return a + CURLcode saying whether an error occured or CURLE_OK if |len| was written. */ +static CURLcode +socket_write(struct connectdata *conn, curl_socket_t fd, const void *to, size_t len) { - const unsigned char *p = buf; - int b; - while(len) { - b = write(fd, p, len); - if(b < 0 && (errno == EINTR || errno == EAGAIN)) - continue; - else if(b < 0) - return -1; - len -= b; - p += b; + const char *to_p = to; + CURLcode code; + ssize_t written; + + while(len > 0) { + code = Curl_write_plain(conn, fd, to_p, len, &written); + if(code == CURLE_OK) { + len -= written; + to_p += written; + } + else { + /* FIXME: We are doing a busy wait */ + if(code == CURLE_AGAIN) + continue; + return code; + } } - return p - (unsigned char*)buf; + return CURLE_OK; } -static int -sec_get_data(struct connectdata *conn, - int fd, struct krb4buffer *buf) +static CURLcode read_data(struct connectdata *conn, curl_socket_t fd, + struct krb4buffer *buf) { int len; - int b; + void *tmp; + CURLcode ret; + + ret = socket_read(fd, &len, sizeof(len)); + if(ret != CURLE_OK) + return ret; - b = block_read(fd, &len, sizeof(len)); - if(b == 0) - return 0; - else if(b < 0) - return -1; len = ntohl(len); - /* TODO: This realloc will cause a memory leak in an out of memory - * condition */ - buf->data = realloc(buf->data, len); - b = buf->data ? block_read(fd, buf->data, len) : -1; - if(b == 0) - return 0; - else if(b < 0) - return -1; + tmp = realloc(buf->data, len); + if(!tmp) + return CURLE_OUT_OF_MEMORY; + buf->data = tmp; + + ret = socket_read(fd, buf->data, len); + if(ret != CURLE_OK) + return ret; + buf->size = (conn->mech->decode)(conn->app_data, buf->data, len, conn->data_prot, conn); buf->index = 0; - return 0; + return CURLE_OK; } static size_t -buffer_read(struct krb4buffer *buf, void *data, size_t len) +buffer_read(struct krb4buffer *buf, const char *data, size_t len) { - if(buf->size - buf->index < len) - len = buf->size - buf->index; - memcpy(data, (char*)buf->data + buf->index, len); + size_t buf_capacity = buf->size - buf->index; + DEBUGASSERT(buf->size > buf->index); + if(buf_capacity < len) + len = buf_capacity; + memcpy(buf, data, len); buf->index += len; return len; } -static ssize_t sec_read(struct connectdata *conn, int num, - char *buffer, size_t length, CURLcode *err) +/* Matches Curl_rev signature */ +static ssize_t sec_recv(struct connectdata *conn, int sockindex, + char *buffer, size_t len, CURLcode *err) { - size_t len; - int rx = 0; - curl_socket_t fd = conn->sock[num]; + size_t bytes_read; + size_t total_read = 0; + curl_socket_t fd = conn->sock[sockindex]; *err = CURLE_OK; @@ -189,81 +191,89 @@ static ssize_t sec_read(struct connectdata *conn, int num, return 0; } - len = buffer_read(&conn->in_buffer, buffer, length); - length -= len; - rx += len; - buffer = (char*)buffer + len; + bytes_read = buffer_read(&conn->in_buffer, buffer, len); + len -= bytes_read; + total_read += bytes_read; + buffer += bytes_read; - while(length) { - if(sec_get_data(conn, fd, &conn->in_buffer) < 0) + while(len > 0) { + if(read_data(conn, fd, &conn->in_buffer) != CURLE_OK) return -1; if(conn->in_buffer.size == 0) { - if(rx) + if(bytes_read > 0) conn->in_buffer.eof_flag = 1; - return rx; + return bytes_read; } - len = buffer_read(&conn->in_buffer, buffer, length); - length -= len; - rx += len; - buffer = (char*)buffer + len; + bytes_read = buffer_read(&conn->in_buffer, buffer, len); + len -= bytes_read; + total_read += bytes_read; + buffer += bytes_read; } - return rx; + /* FIXME: Check for overflow */ + return total_read; } -static int -sec_send(struct connectdata *conn, int fd, const char *from, int length) + +/* FIXME: Find a better name for this method */ +/* Send |length| bytes from |from| to the |fd| socket taking care of encoding + and negociating with the server. |from| can be NULL. */ +static void sec_send_internal(struct connectdata *conn, curl_socket_t fd, + const char *from, int length) { - int bytes; - void *buf; - enum protection_level protlevel = conn->data_prot; - int iscmd = protlevel == prot_cmd; + size_t bytes; + size_t htonl_bytes; + char *buffer; + char *cmd_buffer; + enum protection_level prot_level = conn->data_prot; + bool iscmd = prot_level == prot_cmd; if(iscmd) { if(!strncmp(from, "PASS ", 5) || !strncmp(from, "ACCT ", 5)) - protlevel = prot_private; + prot_level = prot_private; else - protlevel = conn->command_prot; + prot_level = conn->command_prot; } - bytes = (conn->mech->encode)(conn->app_data, from, length, protlevel, - &buf, conn); + bytes = (conn->mech->encode)(conn->app_data, from, length, prot_level, + (void **)&buffer, conn); if(iscmd) { - char *cmdbuf; - - bytes = Curl_base64_encode(conn->data, (char *)buf, bytes, &cmdbuf); + bytes = Curl_base64_encode(conn->data, buffer, bytes, &cmd_buffer); if(bytes > 0) { - if(protlevel == prot_private) - block_write(fd, "ENC ", 4); + static const char *enc = "ENC "; + static const char *mic = "MIC "; + if(prot_level == prot_private) + socket_write(conn, fd, enc, 4); else - block_write(fd, "MIC ", 4); - block_write(fd, cmdbuf, bytes); - block_write(fd, "\r\n", 2); - Curl_infof(conn->data, "%s %s\n", - protlevel == prot_private ? "ENC" : "MIC", cmdbuf); - free(cmdbuf); + socket_write(conn, fd, mic, 4); + + socket_write(conn, fd, cmd_buffer, bytes); + socket_write(conn, fd, "\r\n", 2); + infof(conn->data, "Send: %s%s", prot_level == prot_private?enc:mic, + cmd_buffer); + free(cmd_buffer); } } else { - bytes = htonl(bytes); - block_write(fd, &bytes, sizeof(bytes)); - block_write(fd, buf, ntohl(bytes)); + htonl_bytes = htonl(bytes); + socket_write(conn, fd, &htonl_bytes, sizeof(htonl_bytes)); + socket_write(conn, fd, buffer, bytes); } - free(buf); - return length; + free(buffer); } -static ssize_t sec_write(struct connectdata *conn, int fd, - const char *buffer, int length) +static ssize_t sec_write(struct connectdata *conn, curl_socket_t fd, + const char *buffer, size_t length) { - int len = conn->buffer_size; + /* FIXME: Check for overflow */ + ssize_t len = conn->buffer_size; int tx = 0; len -= (conn->mech->overhead)(conn->app_data, conn->data_prot, len); if(len <= 0) len = length; - while(length){ - if(length < len) + while(length) { + if(len >= 0 || length < (size_t)len) len = length; - sec_send(conn, fd, buffer, len); + sec_send_internal(conn, fd, buffer, len); length -= len; buffer += len; tx += len; @@ -271,205 +281,239 @@ static ssize_t sec_write(struct connectdata *conn, int fd, return tx; } -int -Curl_sec_fflush_fd(struct connectdata *conn, int fd) +/* FIXME: fd should be a curl_socket_t */ +int Curl_sec_fflush_fd(struct connectdata *conn, int fd) { - if(conn->data_prot != prot_clear) { - if(conn->out_buffer.index > 0){ - sec_write(conn, fd, conn->out_buffer.data, conn->out_buffer.index); - conn->out_buffer.index = 0; - } - sec_send(conn, fd, NULL, 0); - } + if(conn->data_prot == prot_clear) + return 0; + + /* Force a flush by trying to send no data */ + sec_send_internal(conn, fd, NULL, 0); return 0; } -static ssize_t _sec_send(struct connectdata *conn, int num, - const void *buffer, size_t length, CURLcode *err) +/* Matches Curl_send signature */ +static ssize_t sec_send(struct connectdata *conn, int sockindex, + const void *buffer, size_t len, CURLcode *err) { - curl_socket_t fd = conn->sock[num]; + curl_socket_t fd = conn->sock[sockindex]; *err = CURLE_OK; - return sec_write(conn, fd, buffer, length); + return sec_write(conn, fd, buffer, len); } -int -Curl_sec_read_msg(struct connectdata *conn, char *s, int level) +/* FIXME: |level| should not be an int but a struct protection_level */ +int Curl_sec_read_msg(struct connectdata *conn, char *buffer, int level) { - int len; - unsigned char *buf; - int code; - - len = Curl_base64_decode(s + 4, &buf); /* XXX */ - if(len > 0) - len = (conn->mech->decode)(conn->app_data, buf, len, level, conn); - else + /* decoded_len should be size_t or ssize_t but conn->mech->decode returns an + int */ + int decoded_len; + char *buf; + int ret_code; + + /* FIXME: Why do we need to base64-decode? */ + decoded_len = Curl_base64_decode(buffer + 4, (unsigned char **)&buf); + if(decoded_len <= 0) { + free(buf); return -1; - - if(len < 0) { + } + decoded_len = (conn->mech->decode)(conn->app_data, buf, decoded_len, + level, conn); + /* FIXME: Why don't we have the same check as above? */ + if(decoded_len < 0) { free(buf); return -1; } if(conn->data->set.verbose) { - buf[len] = '\n'; - Curl_debug(conn->data, CURLINFO_HEADER_IN, (char *)buf, len + 1, conn); + buf[decoded_len] = '\n'; + Curl_debug(conn->data, CURLINFO_HEADER_IN, buf, decoded_len + 1, conn); } - - buf[len] = '\0'; - + buf[decoded_len] = '\0'; + /* FIXME: Can decoded_len be lower than 3? */ if(buf[3] == '-') - code = 0; + ret_code = 0; else - sscanf((char *)buf, "%d", &code); - if(buf[len-1] == '\n') - buf[len-1] = '\0'; - strcpy(s, (char *)buf); + /* Check for error? */ + sscanf(buf, "%d", &ret_code); + + if(buf[decoded_len - 1] == '\n') + buf[decoded_len - 1] = '\0'; + /* Is buffer length always greater to decoded_len? */ + strcpy(buffer, buf); free(buf); - return code; + return ret_code; } -enum protection_level -Curl_set_command_prot(struct connectdata *conn, enum protection_level level) +/* Send an FTP command defined by |message| and the optional arguments. The + function returns the ftp_code. If an error occurs, -1 is returned. */ +static int ftp_send_command(struct connectdata *conn, const char *message, ...) { - enum protection_level old = conn->command_prot; - conn->command_prot = level; - return old; + int ftp_code; + ssize_t nread; + va_list args; + + va_start(args, message); + if(Curl_ftpsendf(conn, message, args) != CURLE_OK) { + ftp_code = -1; + } + else { + if(Curl_GetFTPResponse(&nread, conn, &ftp_code) != CURLE_OK) + ftp_code = -1; + } + + (void)nread; /* Unused */ + va_end(args); + return ftp_code; } -static int -sec_prot_internal(struct connectdata *conn, int level) +enum protection_level Curl_set_command_prot(struct connectdata *conn, + enum protection_level level) { - char *p; - unsigned int s = 1048576; - ssize_t nread; + enum protection_level old_prot = conn->command_prot; + conn->command_prot = level; + return old_prot; +} - if(!conn->sec_complete){ - infof(conn->data, "No security data exchange has taken place.\n"); - return -1; +/* FIXME: We should return an error code here. */ +void Curl_sec_set_protection_level(struct connectdata *conn) +{ + int code; + char* pbsz; + static unsigned int buffer_size = 1 << 20; /* 1048576 */ + enum protection_level level = conn->request_data_prot; + + if(!conn->sec_complete) { + infof(conn->data, "Trying to change the protection level after the" + "completion of the data exchange."); + return; } - if(level){ - int code; - if(Curl_ftpsendf(conn, "PBSZ %u", s)) - return -1; + /* Bail out if we try to set up the same level */ + if(conn->data_prot == level) + return; - if(Curl_GetFTPResponse(&nread, conn, &code)) - return -1; + if(level) { + code = ftp_send_command(conn, "PSBZ %u", buffer_size); - if(code/100 != 2){ - failf(conn->data, "Failed to set protection buffer size."); - return -1; + if(code < 0) + return; + + if(code/100 != 2) { + failf(conn->data, "Could not set the protection level."); + return; } - conn->buffer_size = s; + conn->buffer_size = buffer_size; - p = strstr(conn->data->state.buffer, "PBSZ="); - if(p) - sscanf(p, "PBSZ=%u", &s); - if(s < conn->buffer_size) - conn->buffer_size = s; + pbsz = strstr(conn->data->state.buffer, "PBSZ="); + if(pbsz) { + sscanf(pbsz, "PBSZ=%u", &buffer_size); + if (buffer_size < conn->buffer_size) + conn->buffer_size = buffer_size; + } } - if(Curl_ftpsendf(conn, "PROT %c", level["CSEP"])) - return -1; + /* Now try to negiociate the protection level */ + code = ftp_send_command(conn, "PROT %c", level_to_char(level)); - if(Curl_GetFTPResponse(&nread, conn, NULL)) - return -1; + if(code < 0) + return; - if(conn->data->state.buffer[0] != '2'){ - failf(conn->data, "Failed to set protection level."); - return -1; + if(code/100 != 2) { + failf(conn->data, "Could not set the protection level."); + return; } - - conn->data_prot = (enum protection_level)level; + conn->data_prot = level; if(level == prot_private) - conn->command_prot = (enum protection_level)level; - return 0; -} - -void -Curl_sec_set_protection_level(struct connectdata *conn) -{ - if(conn->sec_complete && conn->data_prot != conn->request_data_prot) - sec_prot_internal(conn, conn->request_data_prot); + conn->command_prot = level; } -int -Curl_sec_request_prot(struct connectdata *conn, const char *level) +int Curl_sec_request_prot(struct connectdata *conn, const char *level) { int l = name_to_level(level); - if(l == -1) + if(l < 0) return -1; + conn->request_data_prot = (enum protection_level)l; return 0; } -int -Curl_sec_login(struct connectdata *conn) +static CURLcode choose_mech(struct connectdata *conn) { int ret; - const struct Curl_sec_client_mech * const *m; - ssize_t nread; - struct SessionHandle *data=conn->data; - int ftpcode; - - for(m = mechs; *m && (*m)->name; m++) { - void *tmp; - - tmp = realloc(conn->app_data, (*m)->size); - if(tmp == NULL) { - failf (data, "realloc %u failed", (*m)->size); - return -1; - } - conn->app_data = tmp; - - if((*m)->init && (*(*m)->init)(conn->app_data) != 0) { - infof(data, "Skipping %s...\n", (*m)->name); + struct SessionHandle *data = conn->data; + const struct Curl_sec_client_mech * const *mech; + void *tmp_allocation; + const char *mech_name; + + for(mech = mechs; (*mech); ++mech) { + mech_name = (*mech)->name; + /* We have no mechanism with a NULL name but keep this check */ + DEBUGASSERT(mech_name != NULL); + if(mech_name == NULL) { + infof(data, "Skipping mechanism with empty name (%p)", mech); continue; } - infof(data, "Trying %s...\n", (*m)->name); + tmp_allocation = realloc(conn->app_data, (*mech)->size); + if(tmp_allocation == NULL) { + infof(data, "Failed realloc of size %u", (*mech)->size); + mech = NULL; + return CURLE_OUT_OF_MEMORY; + } + conn->app_data = tmp_allocation; - if(Curl_ftpsendf(conn, "AUTH %s", (*m)->name)) - return -1; + if((*mech)->init) { + ret = (*mech)->init(conn); + if(ret != 0) { + infof(data, "Failed initialization for %s", mech_name); + continue; + } + } - if(Curl_GetFTPResponse(&nread, conn, &ftpcode)) - return -1; + infof(data, "Trying mechanism %s...", mech_name); + ret = ftp_send_command(conn, "AUTH %s", mech_name); + if(ret < 0) + /* FIXME: This error is too generic but it is OK for now. */ + return CURLE_COULDNT_CONNECT; - if(conn->data->state.buffer[0] != '3'){ - switch(ftpcode) { + if(ret/100 != 3) { + switch(ret) { case 504: - infof(data, - "%s is not supported by the server.\n", (*m)->name); + infof(data, "Mechanism %s is not supported by the server (server " + "returned ftp code: 504).", mech_name); break; case 534: - infof(data, "%s rejected as security mechanism.\n", (*m)->name); + infof(data, "Mechanism %s was rejected by the server (server returned " + "ftp code: 534).", mech_name); break; default: - if(conn->data->state.buffer[0] == '5') { - infof(data, "The server doesn't support the FTP " - "security extensions.\n"); - return -1; + if(ret/100 == 5) { + infof(data, "The server does not support the security extensions."); + return CURLE_USE_SSL_FAILED; } break; } continue; } - ret = (*(*m)->auth)(conn->app_data, conn); + /* Authenticate */ + ret = ((*mech)->auth)(conn->app_data, conn); if(ret == AUTH_CONTINUE) continue; - else if(ret != AUTH_OK){ - /* mechanism is supposed to output error string */ + else if(ret != AUTH_OK) { + /* Mechanism has dumped the error to stderr, don't error here. */ return -1; } - conn->mech = *m; + DEBUGASSERT(ret == AUTH_OK); + + conn->mech = *mech; conn->sec_complete = 1; - if (conn->data_prot != prot_clear) { - conn->recv[FIRSTSOCKET] = sec_read; - conn->send[FIRSTSOCKET] = _sec_send; - conn->recv[SECONDARYSOCKET] = sec_read; - conn->send[SECONDARYSOCKET] = _sec_send; + if(conn->data_prot != prot_clear) { + conn->recv[FIRSTSOCKET] = sec_recv; + conn->send[FIRSTSOCKET] = sec_send; + conn->recv[SECONDARYSOCKET] = sec_recv; + conn->send[SECONDARYSOCKET] = sec_send; } conn->command_prot = prot_safe; /* Set the requested protection level */ @@ -477,24 +521,30 @@ Curl_sec_login(struct connectdata *conn) Curl_sec_set_protection_level(conn); break; } + return CURLE_OK; +} - return *m == NULL; +int Curl_sec_login(struct connectdata *conn) +{ + CURLcode ret = choose_mech(conn); + return ret == CURLE_OK; } -void -Curl_sec_end(struct connectdata *conn) +void Curl_sec_end(struct connectdata *conn) { if(conn->mech != NULL) { if(conn->mech->end) (conn->mech->end)(conn->app_data); + /* FIXME: Why do we zero'd it before free'ing it? */ memset(conn->app_data, 0, conn->mech->size); free(conn->app_data); conn->app_data = NULL; } conn->sec_complete = 0; conn->data_prot = (enum protection_level)0; - conn->mech=NULL; + conn->mech = NULL; } -#endif /* HAVE_KRB4 */ +#endif /* HAVE_KRB4 || HAVE_GSSAPI */ + #endif /* CURL_DISABLE_FTP */ diff --git a/lib/urldata.h b/lib/urldata.h index 9369dd8..4248c5c 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -199,6 +199,7 @@ struct krb4buffer { size_t index; int eof_flag; }; +/* FIXME: Capitalize this enums as it would make the code more readable */ enum protection_level { prot_clear, prot_safe, @@ -806,7 +807,7 @@ struct connectdata { enum protection_level data_prot; enum protection_level request_data_prot; size_t buffer_size; - struct krb4buffer in_buffer, out_buffer; + struct krb4buffer in_buffer; void *app_data; const struct Curl_sec_client_mech *mech; struct sockaddr_in local_addr; -- 1.7.0.4