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