Hi everyone, > My understanding of a rewrite was that it needed to be done in a > block. If we don't go this way, I would rather split the huge patch > into different components so that they can get better reviews. It will > also be easier to spot regressions that may arise. Please find > attached the first split. I will do the rest over the week-end.
Please find attached the remaining patches attached. That makes 14 patches in total! The patches have been split now per function to better underline the changes. Comments appreciated! Regards, Julien
From 82e89ddbe62656d6e61659386935591589034cc6 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sat, 11 Sep 2010 12:20:04 -0700 Subject: [PATCH 06/14] security.c: factored the logic from Curl_sec_login into a dedicated method that better reflect its intent. Introduced a helper method ftp_send_command that synchronously send an FTP query. --- lib/security.c | 115 +++++++++++++++++++++++++++++++++++++------------------ 1 files changed, 77 insertions(+), 38 deletions(-) diff --git a/lib/security.c b/lib/security.c index 3a3f0e0..d4bb0b3 100644 --- a/lib/security.c +++ b/lib/security.c @@ -101,6 +101,27 @@ static const struct Curl_sec_client_mech * const mechs[] = { NULL }; +/* 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, ...) +{ + 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; +} /* 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. */ @@ -416,66 +437,76 @@ Curl_sec_request_prot(struct connectdata *conn, const char *level) 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) { + failf(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. Skipping it.", 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; @@ -490,9 +521,17 @@ Curl_sec_login(struct connectdata *conn) break; } - return *m == NULL; + return mech != NULL ? CURLE_OK : CURLE_FAILED_INIT; } +int +Curl_sec_login(struct connectdata *conn) +{ + CURLcode code = choose_mech(conn); + return code == CURLE_OK; +} + + void Curl_sec_end(struct connectdata *conn) { -- 1.7.0.4
From dfb1ca99b3b12ba5164b8ffbb4cfe8139fbb75b7 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 15:41:44 -0700 Subject: [PATCH 07/14] security.c: Curl_sec_set_protection_level tweaking - Removed sec_prot_internal as it is now inlined in the function (this removed a redundant check). - Changed the prototype to return an error code. - Updated the method to use the new ftp_send_command function. - Added a level_to_char helper method to avoid relying on the compiler's bound checks. This default to the maximum security we have in case of a wrong input. --- lib/krb4.h | 2 +- lib/security.c | 93 ++++++++++++++++++++++++++++++++++--------------------- 2 files changed, 58 insertions(+), 37 deletions(-) diff --git a/lib/krb4.h b/lib/krb4.h index 2642793..29a2578 100644 --- a/lib/krb4.h +++ b/lib/krb4.h @@ -61,7 +61,7 @@ void Curl_sec_end (struct connectdata *); int Curl_sec_login (struct connectdata *); void Curl_sec_prot (int, char **); int Curl_sec_request_prot (struct connectdata *conn, const char *level); -void Curl_sec_set_protection_level(struct connectdata *conn); +int Curl_sec_set_protection_level(struct connectdata *conn); void Curl_sec_status (void); enum protection_level Curl_set_command_prot(struct connectdata *, diff --git a/lib/security.c b/lib/security.c index d4bb0b3..ca74ec5 100644 --- a/lib/security.c +++ b/lib/security.c @@ -91,6 +91,29 @@ name_to_level(const char *name) return (enum protection_level)-1; } +/* Convert a protocol |level| to its char representation. + We take an int to catch programming mistakes. */ +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); + /* Default to the most secure alternative. */ + return 'P'; +} + static const struct Curl_sec_client_mech * const mechs[] = { #if defined(HAVE_GSSAPI) &Curl_krb5_client_mech, @@ -369,64 +392,62 @@ Curl_set_command_prot(struct connectdata *conn, enum protection_level level) return old; } -static int -sec_prot_internal(struct connectdata *conn, int level) +/* FIXME: The error code returned here is never checked. */ +int Curl_sec_set_protection_level(struct connectdata *conn) { - char *p; - unsigned int s = 1048576; - ssize_t nread; + 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, "No security data exchange has taken place.\n"); + if(!conn->sec_complete) { + infof(conn->data, "Trying to change the protection level after the" + "completion of the data exchange."); return -1; } - 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 0; - if(Curl_GetFTPResponse(&nread, conn, &code)) + if(level) { + code = ftp_send_command(conn, "PSBZ %u", buffer_size); + if(code < 0) return -1; - if(code/100 != 2){ - failf(conn->data, "Failed to set protection buffer size."); + if(code/100 != 2) { + failf(conn->data, "Failed to set the protection's buffer size."); return -1; } - conn->buffer_size = s; - - p = strstr(conn->data->state.buffer, "PBSZ="); - if(p) - sscanf(p, "PBSZ=%u", &s); - if(s < conn->buffer_size) - conn->buffer_size = s; + conn->buffer_size = buffer_size; + + pbsz = strstr(conn->data->state.buffer, "PBSZ="); + if(pbsz) { + /* FIXME: Checks for errors in sscanf? */ + 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)) + if(code < 0) return -1; - if(conn->data->state.buffer[0] != '2'){ - failf(conn->data, "Failed to set protection level."); + if(code/100 != 2) { + failf(conn->data, "Failed to set the protection level."); return -1; } - conn->data_prot = (enum protection_level)level; + conn->data_prot = level; if(level == prot_private) - conn->command_prot = (enum protection_level)level; - return 0; -} + conn->command_prot = level; -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); + return 0; } - int Curl_sec_request_prot(struct connectdata *conn, const char *level) { -- 1.7.0.4
From a31dc392c392758080a560492fbc8dee303e1320 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 16:08:52 -0700 Subject: [PATCH 08/14] security.c: Curl_sec_read_msg tweaks - Renamed the variables name to better match their intend. - Unified the |decoded_len| checks. - Added some FIXMEs to flag some improvement that did not go in this change. --- lib/security.c | 52 ++++++++++++++++++++++++++++++---------------------- 1 files changed, 30 insertions(+), 22 deletions(-) diff --git a/lib/security.c b/lib/security.c index ca74ec5..617a625 100644 --- a/lib/security.c +++ b/lib/security.c @@ -348,40 +348,48 @@ static ssize_t _sec_send(struct connectdata *conn, int num, return sec_write(conn, fd, buffer, length); } -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; + + 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); + 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'; + DEBUGASSERT(decoded_len > 3); if(buf[3] == '-') - code = 0; - else - sscanf((char *)buf, "%d", &code); - if(buf[len-1] == '\n') - buf[len-1] = '\0'; - strcpy(s, (char *)buf); + ret_code = 0; + else { + /* Check for error? */ + sscanf(buf, "%d", &ret_code); + } + + if(buf[decoded_len - 1] == '\n') + buf[decoded_len - 1] = '\0'; + /* FIXME: Is |buffer| length always greater than |decoded_len|? */ + strcpy(buffer, buf); free(buf); - return code; + return ret_code; } enum protection_level -- 1.7.0.4
From e19dcf691b8bbfa067bb3d76fc3c0c77f96611ae Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 16:25:05 -0700 Subject: [PATCH 09/14] security.c: sec_send tweaks - Renamed it to do_sec_send as it is the function doing the actual transfer. - Do not return any values as no one was checking it and it never reported a failure (added a FIXME about checking for errors). - Renamed the variables to make their use more specific. - Removed some casts (int -> curl_socket_t, ...) - Avoid doing the htnl <-> nthl twice by caching the 2 results. --- lib/security.c | 59 ++++++++++++++++++++++++++++++------------------------- 1 files changed, 32 insertions(+), 27 deletions(-) diff --git a/lib/security.c b/lib/security.c index 617a625..699be47 100644 --- a/lib/security.c +++ b/lib/security.c @@ -270,45 +270,50 @@ static ssize_t sec_read(struct connectdata *conn, int num, return rx; } -static int -sec_send(struct connectdata *conn, int fd, const char *from, int length) +/* Send |length| bytes from |from| to the |fd| socket taking care of encoding + and negociating with the server. |from| can be NULL. */ +/* FIXME: We don't check for errors nor report any! */ +static void do_sec_send(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) - socket_write(conn, 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 - socket_write(conn, fd, "MIC ", 4); - socket_write(conn, fd, cmdbuf, bytes); + socket_write(conn, fd, mic, 4); + + socket_write(conn, fd, cmd_buffer, bytes); socket_write(conn, fd, "\r\n", 2); - Curl_infof(conn->data, "%s %s\n", - protlevel == prot_private ? "ENC" : "MIC", cmdbuf); - free(cmdbuf); + infof(conn->data, "Send: %s%s", prot_level == prot_private?enc:mic, + cmd_buffer); + free(cmd_buffer); } } else { - bytes = htonl(bytes); - socket_write(conn, fd, &bytes, sizeof(bytes)); - socket_write(conn, 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, @@ -323,7 +328,7 @@ static ssize_t sec_write(struct connectdata *conn, int fd, while(length){ if(length < len) len = length; - sec_send(conn, fd, buffer, len); + do_sec_send(conn, fd, buffer, len); length -= len; buffer += len; tx += len; @@ -335,7 +340,7 @@ int Curl_sec_fflush_fd(struct connectdata *conn, int fd) { if(conn->data_prot != prot_clear) { - sec_send(conn, fd, NULL, 0); + do_sec_send(conn, fd, NULL, 0); } return 0; } -- 1.7.0.4
From 486b98e472d873a0bd3d151fd69fd1adfc868bf4 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 16:32:41 -0700 Subject: [PATCH 10/14] security.c: Curl_sec_fflush_fd tweaks - Use an early return as it makes the code more readable. - Added a FIXME about a conversion. --- lib/security.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/security.c b/lib/security.c index 699be47..d0eccdf 100644 --- a/lib/security.c +++ b/lib/security.c @@ -336,12 +336,14 @@ 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) { - do_sec_send(conn, fd, NULL, 0); - } + if(conn->data_prot == prot_clear) + return 0; + + /* Force a flush by trying to send no data */ + do_sec_send(conn, fd, NULL, 0); return 0; } -- 1.7.0.4
From 5d4bdd64646ee01231491780029b13cf17f3d18e Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 16:38:38 -0700 Subject: [PATCH 11/14] security.c: sec_read tweaks - Renamed the function to sec_recv. - Renamed the parameters and variable to match the rest of the code. --- lib/security.c | 40 +++++++++++++++++++++------------------- 1 files changed, 21 insertions(+), 19 deletions(-) diff --git a/lib/security.c b/lib/security.c index d0eccdf..cc2fbf1 100644 --- a/lib/security.c +++ b/lib/security.c @@ -235,12 +235,13 @@ buffer_read(struct krb4buffer *buf, const char *data, size_t len) return len; } -static ssize_t sec_read(struct connectdata *conn, int num, - char *buffer, size_t length, CURLcode *err) +/* Matches Curl_recv 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; @@ -249,25 +250,26 @@ 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) { + 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; } /* Send |length| bytes from |from| to the |fd| socket taking care of encoding @@ -545,9 +547,9 @@ static CURLcode choose_mech(struct connectdata *conn) conn->mech = *mech; conn->sec_complete = 1; if (conn->data_prot != prot_clear) { - conn->recv[FIRSTSOCKET] = sec_read; + conn->recv[FIRSTSOCKET] = sec_recv; conn->send[FIRSTSOCKET] = _sec_send; - conn->recv[SECONDARYSOCKET] = sec_read; + conn->recv[SECONDARYSOCKET] = sec_recv; conn->send[SECONDARYSOCKET] = _sec_send; } conn->command_prot = prot_safe; -- 1.7.0.4
From cdc1a719f0d30925b0949a9f4f56050f8eae5ea1 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 16:41:45 -0700 Subject: [PATCH 12/14] security.c: _sec_send tweaks - Renamed the method to sec_send now that we renamed sec_send to do_sec_send. - Some more variable renaming. --- lib/security.c | 13 +++++++------ 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/security.c b/lib/security.c index cc2fbf1..ac3c689 100644 --- a/lib/security.c +++ b/lib/security.c @@ -349,12 +349,13 @@ int Curl_sec_fflush_fd(struct connectdata *conn, int fd) 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); } /* FIXME: |level| should not be an int but a struct protection_level */ @@ -548,9 +549,9 @@ static CURLcode choose_mech(struct connectdata *conn) conn->sec_complete = 1; if (conn->data_prot != prot_clear) { conn->recv[FIRSTSOCKET] = sec_recv; - conn->send[FIRSTSOCKET] = _sec_send; + conn->send[FIRSTSOCKET] = sec_send; conn->recv[SECONDARYSOCKET] = sec_recv; - conn->send[SECONDARYSOCKET] = _sec_send; + conn->send[SECONDARYSOCKET] = sec_send; } conn->command_prot = prot_safe; /* Set the requested protection level */ -- 1.7.0.4
From 92ef53b61d4fb49feae509504d21d805cb62c98b Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 16:46:09 -0700 Subject: [PATCH 13/14] security.c: sec_write tweaks - |fd| is now a curl_socket_t and |len| a size_t to avoid conversions. - Added 2 FIXMEs about the 2 unsigned -> signed conversions. - Included 2 minor changes to Curl_sec_end. --- lib/security.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/security.c b/lib/security.c index ac3c689..6e1797c 100644 --- a/lib/security.c +++ b/lib/security.c @@ -318,18 +318,21 @@ static void do_sec_send(struct connectdata *conn, curl_socket_t fd, 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) { + /* FIXME: Check for overflow. */ len = length; + } do_sec_send(conn, fd, buffer, len); length -= len; buffer += len; @@ -577,13 +580,14 @@ 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 || HAVE_GSSAPI */ -- 1.7.0.4
From 794f2caa46c0e66709c67caf50d1f4e82ed3a5f4 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix <[email protected]> Date: Sun, 12 Sep 2010 17:22:04 -0700 Subject: [PATCH 14/14] security.c: Update the #include statements after the rewrite. --- lib/security.c | 11 ++++------- 1 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/security.c b/lib/security.c index 6e1797c..2eeaa31 100644 --- a/lib/security.c +++ b/lib/security.c @@ -46,10 +46,7 @@ #ifndef CURL_DISABLE_FTP #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) -#define _MPRINTF_REPLACE /* we want curl-functions instead of native ones */ -#include <curl/mprintf.h> - -#include <stdlib.h> +#include <stdarg.h> #include <string.h> #ifdef HAVE_NETDB_H @@ -61,11 +58,11 @@ #endif #include "urldata.h" -#include "krb4.h" #include "curl_base64.h" -#include "sendf.h" -#include "ftp.h" #include "curl_memory.h" +#include "krb4.h" +#include "ftp.h" +#include "sendf.h" #include "rawstr.h" /* The last #include file should be: */ -- 1.7.0.4
------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
