[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 { |
---|