Commit 8d59d694 authored by Julien Chaffraix's avatar Julien Chaffraix
Browse files

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.
parent 465865c3

lib/ftp.c

+2 −0
Original line number Original line Diff line number Diff line
@@ -3784,11 +3784,13 @@ CURLcode Curl_ftpsendf(struct connectdata *conn, 




  for(;;) {

  for(;;) {

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

    DEBUGASSERT(prot_cmd > prot_none && prot_cmd < prot_last);

    conn->data_prot = prot_cmd;

    conn->data_prot = prot_cmd;

#endif

#endif

    res = Curl_write(conn, conn->sock[FIRSTSOCKET], sptr, write_len,

    res = Curl_write(conn, conn->sock[FIRSTSOCKET], sptr, write_len,

                     &bytes_written);

                     &bytes_written);

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

    DEBUGASSERT(data_sec > prot_none && data_sec < prot_last);

    conn->data_prot = data_sec;

    conn->data_prot = data_sec;

#endif

#endif

lib/krb4.c

+2 −1
Original line number Original line Diff line number Diff line
@@ -319,6 +319,7 @@ static enum protection_level 
krb4_set_command_prot(struct connectdata *conn, enum protection_level level)

krb4_set_command_prot(struct connectdata *conn, enum protection_level level)

{

{

  enum protection_level old = conn->command_prot;

  enum protection_level old = conn->command_prot;

  DEBUGASSERT(level > prot_none && level < prot_last);

  conn->command_prot = level;

  conn->command_prot = level;

  return old;

  return old;

}

}
@@ -333,7 +334,7 @@ CURLcode Curl_krb_kauth(struct connectdata *conn) 
  char passwd[100];

  char passwd[100];

  size_t tmp;

  size_t tmp;

  ssize_t nread;

  ssize_t nread;

  int save;

  enum protection_level save;

  CURLcode result;

  CURLcode result;

  unsigned char *ptr;

  unsigned char *ptr;

lib/krb4.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -47,7 +47,7 @@ extern struct Curl_sec_client_mech Curl_krb5_client_mech; 
#endif

#endif





CURLcode Curl_krb_kauth(struct connectdata *conn);

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 *);

void Curl_sec_end (struct connectdata *);

CURLcode Curl_sec_login (struct connectdata *);

CURLcode Curl_sec_login (struct connectdata *);

int Curl_sec_request_prot (struct connectdata *conn, const char *level);

int Curl_sec_request_prot (struct connectdata *conn, const char *level);

lib/pingpong.c

+4 −2
Original line number Original line Diff line number Diff line
@@ -217,11 +217,13 @@ CURLcode Curl_pp_vsendf(struct pingpong *pp, 
#endif /* CURL_DOES_CONVERSIONS */

#endif /* CURL_DOES_CONVERSIONS */





#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

  DEBUGASSERT(prot_cmd > prot_none && prot_cmd < prot_last);

  conn->data_prot = prot_cmd;

  conn->data_prot = prot_cmd;

#endif

#endif

  res = Curl_write(conn, conn->sock[FIRSTSOCKET], sptr, write_len,

  res = Curl_write(conn, conn->sock[FIRSTSOCKET], sptr, write_len,

                   &bytes_written);

                   &bytes_written);

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

  DEBUGASSERT(data_sec > prot_none && data_sec < prot_last);

  conn->data_prot = data_sec;

  conn->data_prot = data_sec;

#endif

#endif
@@ -331,13 +333,13 @@ CURLcode Curl_pp_readresp(curl_socket_t sockfd, 
      int res;

      int res;

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

      enum protection_level prot = conn->data_prot;

      enum protection_level prot = conn->data_prot;



      conn->data_prot = prot_clear;

      conn->data_prot = 0;

#endif

#endif

      DEBUGASSERT((ptr+BUFSIZE-pp->nread_resp) <= (buf+BUFSIZE+1));

      DEBUGASSERT((ptr+BUFSIZE-pp->nread_resp) <= (buf+BUFSIZE+1));

      res = Curl_read(conn, sockfd, ptr, BUFSIZE-pp->nread_resp,

      res = Curl_read(conn, sockfd, ptr, BUFSIZE-pp->nread_resp,

                      &gotbytes);

                      &gotbytes);

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

#if defined(HAVE_KRB4) || defined(HAVE_GSSAPI)

      DEBUGASSERT(prot  > prot_none && prot < prot_last);

      conn->data_prot = prot;

      conn->data_prot = prot;

#endif

#endif

      if(res == CURLE_AGAIN)

      if(res == CURLE_AGAIN)

lib/security.c

+14 −7
Original line number Original line Diff line number Diff line
@@ -85,7 +85,7 @@ name_to_level(const char *name) 
  for(i = 0; i < (int)sizeof(level_names)/(int)sizeof(level_names[0]); i++)

  for(i = 0; i < (int)sizeof(level_names)/(int)sizeof(level_names[0]); i++)

    if(checkprefix(name, level_names[i].name))

    if(checkprefix(name, level_names[i].name))

      return level_names[i].level;

      return level_names[i].level;

  return (enum protection_level)-1;

  return prot_none;

}

}





/* Convert a protocol |level| to its char representation.

/* 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;

  enum protection_level prot_level = conn->data_prot;

  bool iscmd = prot_level == prot_cmd;

  bool iscmd = prot_level == prot_cmd;





  DEBUGASSERT(prot_level > prot_none && prot_level < prot_last);



  if(iscmd) {

  if(iscmd) {

    if(!strncmp(from, "PASS ", 5) || !strncmp(from, "ACCT ", 5))

    if(!strncmp(from, "PASS ", 5) || !strncmp(from, "ACCT ", 5))

      prot_level = prot_private;

      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);

  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 Curl_sec_read_msg(struct connectdata *conn, char *buffer, int level)

                      enum protection_level level)

{

{

  /* decoded_len should be size_t or ssize_t but conn->mech->decode returns an

  /* decoded_len should be size_t or ssize_t but conn->mech->decode returns an

     int */

     int */
@@ -364,6 +366,8 @@ int Curl_sec_read_msg(struct connectdata *conn, char *buffer, int level) 
  char *buf;

  char *buf;

  int ret_code;

  int ret_code;





  DEBUGASSERT(level > prot_none && level < prot_last);



  decoded_len = Curl_base64_decode(buffer + 4, (unsigned char **)&buf);

  decoded_len = Curl_base64_decode(buffer + 4, (unsigned char **)&buf);

  if(decoded_len <= 0) {

  if(decoded_len <= 0) {

    free(buf);

    free(buf);
@@ -407,6 +411,8 @@ static int sec_set_protection_level(struct connectdata *conn) 
  static unsigned int buffer_size = 1 << 20; /* 1048576 */

  static unsigned int buffer_size = 1 << 20; /* 1048576 */

  enum protection_level level = conn->request_data_prot;

  enum protection_level level = conn->request_data_prot;





  DEBUGASSERT(level > prot_none && level < prot_last);



  if(!conn->sec_complete) {

  if(!conn->sec_complete) {

    infof(conn->data, "Trying to change the protection level after the"

    infof(conn->data, "Trying to change the protection level after the"

                      "completion of the data exchange.\n");

                      "completion of the data exchange.\n");
@@ -458,10 +464,11 @@ static int sec_set_protection_level(struct connectdata *conn) 
int

int

Curl_sec_request_prot(struct connectdata *conn, const char *level)

Curl_sec_request_prot(struct connectdata *conn, const char *level)

{

{

  int l = name_to_level(level);

  enum protection_level l = name_to_level(level);

  if(l == -1)

  if(l == prot_none)

    return -1;

    return -1;

  conn->request_data_prot = (enum protection_level)l;

  DEBUGASSERT(l > prot_none && l < prot_last);

  conn->request_data_prot = l;

  return 0;

  return 0;

}

}
@@ -575,7 +582,7 @@ Curl_sec_end(struct connectdata *conn) 
    conn->in_buffer.eof_flag = 0;

    conn->in_buffer.eof_flag = 0;

  }

  }

  conn->sec_complete = 0;

  conn->sec_complete = 0;

  conn->data_prot = (enum protection_level)0;

  conn->data_prot = prot_clear;

  conn->mech = NULL;

  conn->mech = NULL;

}

}