From 8d59d69449c2a86c478699a50d920541aa106201 Mon Sep 17 00:00:00 2001 From: Julien Chaffraix Date: Sat, 13 Nov 2010 12:01:33 -0800 Subject: [PATCH] security: tighten enum protection_level usage. While changing Curl_sec_read_msg to accept an enum protection_level instead of an int, I went ahead and fixed the usage of the associated fields. Some code was assuming that prot_clear == 0. Fixed those to use the proper value. Added assertions prior to any code that would set the protection level. --- lib/ftp.c | 2 ++ lib/krb4.c | 3 ++- lib/krb4.h | 2 +- lib/pingpong.c | 6 ++++-- lib/security.c | 21 ++++++++++++++------- lib/url.c | 4 ++++ lib/urldata.h | 4 +++- 7 files changed, 30 insertions(+), 12 deletions(-) diff --git a/lib/ftp.c b/lib/ftp.c index 416e5ecfa..22589355f 100644 --- a/lib/ftp.c +++ b/lib/ftp.c @@ -3784,11 +3784,13 @@ CURLcode Curl_ftpsendf(struct connectdata *conn, for(;;) { #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) + DEBUGASSERT(prot_cmd > prot_none && prot_cmd < prot_last); conn->data_prot = prot_cmd; #endif res = Curl_write(conn, conn->sock[FIRSTSOCKET], sptr, write_len, &bytes_written); #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) + DEBUGASSERT(data_sec > prot_none && data_sec < prot_last); conn->data_prot = data_sec; #endif diff --git a/lib/krb4.c b/lib/krb4.c index f5199312d..8c7793102 100644 --- a/lib/krb4.c +++ b/lib/krb4.c @@ -319,6 +319,7 @@ static enum protection_level krb4_set_command_prot(struct connectdata *conn, enum protection_level level) { enum protection_level old = conn->command_prot; + DEBUGASSERT(level > prot_none && level < prot_last); conn->command_prot = level; return old; } @@ -333,7 +334,7 @@ CURLcode Curl_krb_kauth(struct connectdata *conn) char passwd[100]; size_t tmp; ssize_t nread; - int save; + enum protection_level save; CURLcode result; unsigned char *ptr; diff --git a/lib/krb4.h b/lib/krb4.h index 537a1e140..b3b5ea763 100644 --- a/lib/krb4.h +++ b/lib/krb4.h @@ -47,7 +47,7 @@ extern struct Curl_sec_client_mech Curl_krb5_client_mech; #endif CURLcode Curl_krb_kauth(struct connectdata *conn); -int Curl_sec_read_msg (struct connectdata *conn, char *, int); +int Curl_sec_read_msg (struct connectdata *conn, char *, enum protection_level); void Curl_sec_end (struct connectdata *); CURLcode Curl_sec_login (struct connectdata *); int Curl_sec_request_prot (struct connectdata *conn, const char *level); diff --git a/lib/pingpong.c b/lib/pingpong.c index bced110ed..01f850677 100644 --- a/lib/pingpong.c +++ b/lib/pingpong.c @@ -217,11 +217,13 @@ CURLcode Curl_pp_vsendf(struct pingpong *pp, #endif /* CURL_DOES_CONVERSIONS */ #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) + DEBUGASSERT(prot_cmd > prot_none && prot_cmd < prot_last); conn->data_prot = prot_cmd; #endif res = Curl_write(conn, conn->sock[FIRSTSOCKET], sptr, write_len, &bytes_written); #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) + DEBUGASSERT(data_sec > prot_none && data_sec < prot_last); conn->data_prot = data_sec; #endif @@ -331,13 +333,13 @@ CURLcode Curl_pp_readresp(curl_socket_t sockfd, int res; #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) enum protection_level prot = conn->data_prot; - - conn->data_prot = 0; + conn->data_prot = prot_clear; #endif DEBUGASSERT((ptr+BUFSIZE-pp->nread_resp) <= (buf+BUFSIZE+1)); res = Curl_read(conn, sockfd, ptr, BUFSIZE-pp->nread_resp, &gotbytes); #if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) + DEBUGASSERT(prot > prot_none && prot < prot_last); conn->data_prot = prot; #endif if(res == CURLE_AGAIN) diff --git a/lib/security.c b/lib/security.c index d22ff9a32..88c6541d9 100644 --- a/lib/security.c +++ b/lib/security.c @@ -85,7 +85,7 @@ name_to_level(const char *name) for(i = 0; i < (int)sizeof(level_names)/(int)sizeof(level_names[0]); i++) if(checkprefix(name, level_names[i].name)) return level_names[i].level; - return (enum protection_level)-1; + return prot_none; } /* Convert a protocol |level| to its char representation. @@ -290,6 +290,8 @@ static void do_sec_send(struct connectdata *conn, curl_socket_t fd, enum protection_level prot_level = conn->data_prot; bool iscmd = prot_level == prot_cmd; + DEBUGASSERT(prot_level > prot_none && prot_level < prot_last); + if(iscmd) { if(!strncmp(from, "PASS ", 5) || !strncmp(from, "ACCT ", 5)) prot_level = prot_private; @@ -355,8 +357,8 @@ static ssize_t sec_send(struct connectdata *conn, int sockindex, return sec_write(conn, fd, buffer, len); } -/* 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 Curl_sec_read_msg(struct connectdata *conn, char *buffer, + enum protection_level level) { /* decoded_len should be size_t or ssize_t but conn->mech->decode returns an int */ @@ -364,6 +366,8 @@ int Curl_sec_read_msg(struct connectdata *conn, char *buffer, int level) char *buf; int ret_code; + DEBUGASSERT(level > prot_none && level < prot_last); + decoded_len = Curl_base64_decode(buffer + 4, (unsigned char **)&buf); if(decoded_len <= 0) { free(buf); @@ -407,6 +411,8 @@ static int sec_set_protection_level(struct connectdata *conn) static unsigned int buffer_size = 1 << 20; /* 1048576 */ enum protection_level level = conn->request_data_prot; + DEBUGASSERT(level > prot_none && level < prot_last); + if(!conn->sec_complete) { infof(conn->data, "Trying to change the protection level after the" "completion of the data exchange.\n"); @@ -458,10 +464,11 @@ static int sec_set_protection_level(struct connectdata *conn) int Curl_sec_request_prot(struct connectdata *conn, const char *level) { - int l = name_to_level(level); - if(l == -1) + enum protection_level l = name_to_level(level); + if(l == prot_none) return -1; - conn->request_data_prot = (enum protection_level)l; + DEBUGASSERT(l > prot_none && l < prot_last); + conn->request_data_prot = l; return 0; } @@ -575,7 +582,7 @@ Curl_sec_end(struct connectdata *conn) conn->in_buffer.eof_flag = 0; } conn->sec_complete = 0; - conn->data_prot = (enum protection_level)0; + conn->data_prot = prot_clear; conn->mech = NULL; } diff --git a/lib/url.c b/lib/url.c index fb5122d65..3f9bfa877 100644 --- a/lib/url.c +++ b/lib/url.c @@ -3541,6 +3541,10 @@ static struct connectdata *allocate_conn(struct SessionHandle *data) !conn->done_pipe) goto error; +#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI) + conn->data_prot = prot_clear; +#endif + return conn; error: Curl_llist_destroy(conn->send_pipe, NULL); diff --git a/lib/urldata.h b/lib/urldata.h index 16f365ae8..06bbcda86 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -200,11 +200,13 @@ struct krb4buffer { int eof_flag; }; enum protection_level { + prot_none, /* first in list */ prot_clear, prot_safe, prot_confidential, prot_private, - prot_cmd + prot_cmd, + prot_last /* last in list */ }; #endif