| [1705] | 1 | commit 310c056b2f39c534701b1ee1d1ec4755d4192f70 | 
|---|
 | 2 | Author: Noriko Hosoi <nhosoi@redhat.com> | 
|---|
 | 3 | Date:   Mon Oct 11 16:39:54 2010 -0700 | 
|---|
 | 4 |  | 
|---|
 | 5 |     Bug 637852 - sasl_io_start_packet: failed - read only 3 bytes | 
|---|
 | 6 |     of sasl packet length on connection 4 | 
|---|
 | 7 |      | 
|---|
 | 8 |     https://bugzilla.redhat.com/show_bug.cgi?id=637852 | 
|---|
 | 9 |      | 
|---|
 | 10 |     Description: A SASL packet is made from the 4 byte length and | 
|---|
 | 11 |     the length size of payload.  When the first 4 bytes were not | 
|---|
 | 12 |     successfully received by one PR_Recv call, sasl_io_start_packet | 
|---|
 | 13 |     in sasl_io.c considered an error occurred and set PR_IO_ERROR, | 
|---|
 | 14 |     which terminates the SASL IO session. | 
|---|
 | 15 |     To give clients a chance to send the rest of the length in the | 
|---|
 | 16 |     next packet, this patch sets PR_WOULD_BLOCK_ERROR to the nspr | 
|---|
 | 17 |     error code and EWOULDBLOCK/EAGAIN to errno and once the succeeding | 
|---|
 | 18 |     packet comes in, it appends it to the previous incomplete length | 
|---|
 | 19 |     data and continues the SASL IO. | 
|---|
 | 20 |  | 
|---|
 | 21 | diff --git a/ldap/servers/slapd/sasl_io.c b/ldap/servers/slapd/sasl_io.c | 
|---|
 | 22 | index 4bf81cc..52d6506 100644 | 
|---|
 | 23 | --- a/ldap/servers/slapd/sasl_io.c | 
|---|
 | 24 | +++ b/ldap/servers/slapd/sasl_io.c | 
|---|
 | 25 | @@ -210,11 +210,13 @@ sasl_io_start_packet(PRFileDesc *fd, PRIntn flags, PRIntervalTime timeout, PRInt | 
|---|
 | 26 |      size_t saslio_limit; | 
|---|
 | 27 |      sasl_io_private *sp = sasl_get_io_private(fd); | 
|---|
 | 28 |      Connection *c = sp->conn; | 
|---|
 | 29 | +    PRInt32 amount = sizeof(buffer); | 
|---|
 | 30 |   | 
|---|
 | 31 |      *err = 0; | 
|---|
 | 32 |      debug_print_layers(fd); | 
|---|
 | 33 | +    amount -= sp->encrypted_buffer_offset; | 
|---|
 | 34 |      /* first we need the length bytes */ | 
|---|
 | 35 | -    ret = PR_Recv(fd->lower, buffer, sizeof(buffer), flags, timeout); | 
|---|
 | 36 | +    ret = PR_Recv(fd->lower, buffer, amount, flags, timeout); | 
|---|
 | 37 |      LDAPDebug( LDAP_DEBUG_CONNS, | 
|---|
 | 38 |                 "read sasl packet length returned %d on connection %" NSPRIu64 "\n", ret, c->c_connid, 0 ); | 
|---|
 | 39 |      if (ret <= 0) { | 
|---|
 | 40 | @@ -229,49 +231,57 @@ sasl_io_start_packet(PRFileDesc *fd, PRIntn flags, PRIntervalTime timeout, PRInt | 
|---|
 | 41 |          return ret; | 
|---|
 | 42 |      } | 
|---|
 | 43 |      /* | 
|---|
 | 44 | -     * NOTE: A better way to do this would be to read the bytes and add them to  | 
|---|
 | 45 | -     * sp->encrypted_buffer - if offset < 4, tell caller we didn't read enough | 
|---|
 | 46 | -     * bytes yet - if offset >= 4, decode the length and proceed.  However, it | 
|---|
 | 47 | -     * is highly unlikely that a request to read 4 bytes will return < 4 bytes, | 
|---|
 | 48 | -     * perhaps only in error conditions, in which case the ret < 0 case above | 
|---|
 | 49 | -     * will run | 
|---|
 | 50 | +     * Read the bytes and add them to sp->encrypted_buffer  | 
|---|
 | 51 | +     * - if offset < 4, tell caller we didn't read enough bytes yet  | 
|---|
 | 52 | +     * - if offset >= 4, decode the length and proceed. | 
|---|
 | 53 |       */ | 
|---|
 | 54 |      if (ret < sizeof(buffer)) { | 
|---|
 | 55 | -        LDAPDebug( LDAP_DEBUG_ANY, | 
|---|
 | 56 | -                   "sasl_io_start_packet: failed - read only %d bytes of sasl packet length on connection %" NSPRIu64 "\n", ret, c->c_connid, 0 ); | 
|---|
 | 57 | -           PR_SetError(PR_IO_ERROR, 0); | 
|---|
 | 58 | -           return -1;         | 
|---|
 | 59 | +        memcpy(sp->encrypted_buffer + sp->encrypted_buffer_offset, buffer, ret); | 
|---|
 | 60 | +        sp->encrypted_buffer_offset += ret; | 
|---|
 | 61 | +        if (sp->encrypted_buffer_offset < sizeof(buffer)) { | 
|---|
 | 62 | +            LDAPDebug2Args( LDAP_DEBUG_CONNS, | 
|---|
 | 63 | +                   "sasl_io_start_packet: read only %d bytes of sasl packet " | 
|---|
 | 64 | +                   "length on connection %" NSPRIu64 "\n", ret, c->c_connid ); | 
|---|
 | 65 | +#if defined(EWOULDBLOCK) | 
|---|
 | 66 | +            errno = EWOULDBLOCK; | 
|---|
 | 67 | +#elif defined(EAGAIN) | 
|---|
 | 68 | +            errno = EAGAIN; | 
|---|
 | 69 | +#endif | 
|---|
 | 70 | +            PR_SetError(PR_WOULD_BLOCK_ERROR, errno); | 
|---|
 | 71 | +            return PR_FAILURE; | 
|---|
 | 72 | +        } | 
|---|
 | 73 | +    } else { | 
|---|
 | 74 | +        memcpy(sp->encrypted_buffer, buffer, sizeof(buffer)); | 
|---|
 | 75 | +        sp->encrypted_buffer_offset = sizeof(buffer); | 
|---|
 | 76 |      } | 
|---|
 | 77 | -    if (ret == sizeof(buffer)) { | 
|---|
| [1706] | 78 | -        /* Decode the length (could use ntohl here ??) */ | 
|---|
 | 79 | -        packet_length = buffer[0] << 24 | buffer[1] << 16 | buffer[2] << 8 | buffer[3]; | 
|---|
| [1705] | 80 | -        /* add length itself (for Cyrus SASL library) */ | 
|---|
 | 81 | -        packet_length += 4; | 
|---|
 | 82 | - | 
|---|
 | 83 | -        LDAPDebug( LDAP_DEBUG_CONNS, | 
|---|
 | 84 | -                   "read sasl packet length %ld on connection %" NSPRIu64 "\n", packet_length, c->c_connid, 0 ); | 
|---|
 | 85 | +    /* At this point, sp->encrypted_buffer_offset == sizeof(buffer) */ | 
|---|
 | 86 | +    /* Decode the length */ | 
|---|
 | 87 | +    packet_length = ntohl(*(uint32_t *)sp->encrypted_buffer); | 
|---|
 | 88 | +    /* add length itself (for Cyrus SASL library) */ | 
|---|
 | 89 | +    packet_length += sizeof(uint32_t); | 
|---|
 | 90 |   | 
|---|
 | 91 | -        /* Check if the packet length is larger than our max allowed.  A | 
|---|
 | 92 | -         * setting of -1 means that we allow any size SASL IO packet. */ | 
|---|
 | 93 | -        saslio_limit = config_get_maxsasliosize(); | 
|---|
 | 94 | -        if(((long)saslio_limit != -1) && (packet_length > saslio_limit)) { | 
|---|
 | 95 | -            LDAPDebug( LDAP_DEBUG_ANY, | 
|---|
 | 96 | +    LDAPDebug2Args( LDAP_DEBUG_CONNS, | 
|---|
 | 97 | +                    "read sasl packet length %ld on connection %" NSPRIu64 "\n",  | 
|---|
 | 98 | +                    packet_length, c->c_connid ); | 
|---|
 | 99 | + | 
|---|
 | 100 | +    /* Check if the packet length is larger than our max allowed.  A | 
|---|
 | 101 | +     * setting of -1 means that we allow any size SASL IO packet. */ | 
|---|
 | 102 | +    saslio_limit = config_get_maxsasliosize(); | 
|---|
 | 103 | +    if(((long)saslio_limit != -1) && (packet_length > saslio_limit)) { | 
|---|
 | 104 | +        LDAPDebug2Args( LDAP_DEBUG_ANY, | 
|---|
 | 105 |                  "SASL encrypted packet length exceeds maximum allowed limit (length=%ld, limit=%ld)." | 
|---|
 | 106 |                  "  Change the nsslapd-maxsasliosize attribute in cn=config to increase limit.\n", | 
|---|
 | 107 | -                 packet_length, config_get_maxsasliosize(), 0); | 
|---|
 | 108 | -            PR_SetError(PR_BUFFER_OVERFLOW_ERROR, 0); | 
|---|
 | 109 | -            *err = PR_BUFFER_OVERFLOW_ERROR; | 
|---|
 | 110 | -            return -1; | 
|---|
 | 111 | -        } | 
|---|
 | 112 | - | 
|---|
 | 113 | -        sasl_io_resize_encrypted_buffer(sp, packet_length); | 
|---|
 | 114 | -        /* Cyrus SASL implementation expects to have the length at the first  | 
|---|
 | 115 | -           4 bytes */ | 
|---|
 | 116 | -        memcpy(sp->encrypted_buffer, buffer, 4); | 
|---|
 | 117 | -        sp->encrypted_buffer_count = packet_length; | 
|---|
 | 118 | -        sp->encrypted_buffer_offset = 4; | 
|---|
 | 119 | +                 packet_length, config_get_maxsasliosize() ); | 
|---|
 | 120 | +        PR_SetError(PR_BUFFER_OVERFLOW_ERROR, 0); | 
|---|
 | 121 | +        *err = PR_BUFFER_OVERFLOW_ERROR; | 
|---|
 | 122 | +        return -1; | 
|---|
 | 123 |      } | 
|---|
 | 124 |   | 
|---|
 | 125 | +    sasl_io_resize_encrypted_buffer(sp, packet_length); | 
|---|
 | 126 | +    /* Cyrus SASL implementation expects to have the length at the first  | 
|---|
 | 127 | +       4 bytes */ | 
|---|
 | 128 | +    sp->encrypted_buffer_count = packet_length; | 
|---|
 | 129 | + | 
|---|
 | 130 |      return 1; | 
|---|
 | 131 |  } | 
|---|
 | 132 |   | 
|---|
 | 133 | @@ -345,7 +355,12 @@ sasl_io_recv(PRFileDesc *fd, void *buf, PRInt32 len, PRIntn flags, | 
|---|
 | 134 |          if (!sasl_io_finished_packet(sp)) { | 
|---|
 | 135 |              LDAPDebug( LDAP_DEBUG_CONNS, | 
|---|
 | 136 |                         "sasl_io_recv for connection %" NSPRIu64 " - not finished reading packet yet\n", c->c_connid, 0, 0 ); | 
|---|
 | 137 | -            PR_SetError(PR_WOULD_BLOCK_ERROR, 0); | 
|---|
 | 138 | +#if defined(EWOULDBLOCK) | 
|---|
 | 139 | +            errno = EWOULDBLOCK; | 
|---|
 | 140 | +#elif defined(EAGAIN) | 
|---|
 | 141 | +            errno = EAGAIN; | 
|---|
 | 142 | +#endif | 
|---|
 | 143 | +            PR_SetError(PR_WOULD_BLOCK_ERROR, errno); | 
|---|
 | 144 |              return PR_FAILURE; | 
|---|
 | 145 |          } | 
|---|
 | 146 |          /* We have the full encrypted buffer now - decrypt it */ | 
|---|
 | 147 | @@ -356,6 +371,9 @@ sasl_io_recv(PRFileDesc *fd, void *buf, PRInt32 len, PRIntn flags, | 
|---|
 | 148 |                         "sasl_io_recv finished reading packet for connection %" NSPRIu64 "\n", c->c_connid ); | 
|---|
 | 149 |              /* Now decode it */ | 
|---|
 | 150 |              ret = sasl_decode(c->c_sasl_conn,sp->encrypted_buffer,sp->encrypted_buffer_count,&output_buffer,&output_length); | 
|---|
 | 151 | +            /* even if decode fails, need re-initialize the encrypted_buffer */ | 
|---|
 | 152 | +            sp->encrypted_buffer_offset = 0; | 
|---|
 | 153 | +            sp->encrypted_buffer_count = 0; | 
|---|
 | 154 |              if (SASL_OK == ret) { | 
|---|
 | 155 |                  LDAPDebug2Args( LDAP_DEBUG_CONNS, | 
|---|
 | 156 |                             "sasl_io_recv decoded packet length %d for connection %" NSPRIu64 "\n", output_length, c->c_connid ); | 
|---|
 | 157 | @@ -364,8 +382,6 @@ sasl_io_recv(PRFileDesc *fd, void *buf, PRInt32 len, PRIntn flags, | 
|---|
 | 158 |                      memcpy(sp->decrypted_buffer,output_buffer,output_length); | 
|---|
 | 159 |                      sp->decrypted_buffer_count = output_length; | 
|---|
 | 160 |                      sp->decrypted_buffer_offset = 0; | 
|---|
 | 161 | -                    sp->encrypted_buffer_offset = 0; | 
|---|
 | 162 | -                    sp->encrypted_buffer_count = 0; | 
|---|
 | 163 |                      bytes_in_buffer = output_length; | 
|---|
 | 164 |                  } | 
|---|
 | 165 |              } else { | 
|---|
 | 166 | @@ -461,7 +477,12 @@ sasl_io_send(PRFileDesc *fd, const void *buf, PRInt32 amount, | 
|---|
 | 167 |                              (sp->send_size - sp->send_offset) ); | 
|---|
 | 168 |              sp->send_offset += ret; | 
|---|
 | 169 |              ret = PR_FAILURE; | 
|---|
 | 170 | -            PR_SetError(PR_WOULD_BLOCK_ERROR, 0); | 
|---|
 | 171 | +#if defined(EWOULDBLOCK) | 
|---|
 | 172 | +            errno = EWOULDBLOCK; | 
|---|
 | 173 | +#elif defined(EAGAIN) | 
|---|
 | 174 | +            errno = EAGAIN; | 
|---|
 | 175 | +#endif | 
|---|
 | 176 | +            PR_SetError(PR_WOULD_BLOCK_ERROR, errno); | 
|---|
 | 177 |          } | 
|---|
 | 178 |          /* else - ret is error - caller will handle */ | 
|---|
 | 179 |      } else { | 
|---|