diff options
| author | Julien Chaffraix <julien.chaffraix@gmail.com> | 2010-11-13 12:01:33 -0800 | 
|---|---|---|
| committer | Julien Chaffraix <julien.chaffraix@gmail.com> | 2010-11-13 14:12:43 -0800 | 
| commit | 8d59d69449c2a86c478699a50d920541aa106201 (patch) | |
| tree | 15ef9751b2d92dbd511c95d22d70ca61bdeec674 | |
| parent | 465865c3cb316907ca1c1ea813cf426a2366dce4 (diff) | |
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.
| -rw-r--r-- | lib/ftp.c | 2 | ||||
| -rw-r--r-- | lib/krb4.c | 3 | ||||
| -rw-r--r-- | lib/krb4.h | 2 | ||||
| -rw-r--r-- | lib/pingpong.c | 6 | ||||
| -rw-r--r-- | lib/security.c | 21 | ||||
| -rw-r--r-- | lib/url.c | 4 | ||||
| -rw-r--r-- | lib/urldata.h | 4 | 
7 files changed, 30 insertions, 12 deletions
| @@ -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;  } @@ -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 | 
