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.

diff --git a/lib/ftp.c b/lib/ftp.c
index 416e5ec..2258935 100644 (file)
--- 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 f519931..8c77931 100644 (file)
--- 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 537a1e1..b3b5ea7 100644 (file)
--- 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 bced110..01f8506 100644 (file)
--- 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 d22ff9a..88c6541 100644 (file)
--- 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 fb5122d..3f9bfa8 100644 (file)
--- 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 16f365a..06bbcda 100644 (file)
--- 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