Fix SSL_peek and SSL_pending.
This commit is contained in:
parent
57d2f21782
commit
a0aae68cf6
7 changed files with 65 additions and 61 deletions
18
CHANGES
18
CHANGES
|
@ -144,11 +144,19 @@
|
|||
512 bits], about 30% for larger ones [1024 or 2048 bits].)
|
||||
[Bodo Moeller]
|
||||
|
||||
*) Disable ssl2_peek and ssl3_peek (i.e., both implementations
|
||||
of SSL_peek) because they both are completely broken.
|
||||
For fixing this, the internal read functions now have an additional
|
||||
'peek' parameter, but the actual peek functionality has not
|
||||
yet been implemented.
|
||||
*) Fix ssl3_pending: If the record in s->s3->rrec is not of type
|
||||
SSL3_RT_APPLICATION_DATA, return 0.
|
||||
[Bodo Moeller]
|
||||
|
||||
*) Fix SSL_peek:
|
||||
Both ssl2_peek and ssl3_peek, which were totally broken in earlier
|
||||
releases, have been re-implemented by renaming the previous
|
||||
implementations of ssl2_read and ssl3_read to ssl2_read_internal
|
||||
and ssl3_read_internal, respectively, and adding 'peek' parameters
|
||||
to them. The new ssl[23]_{read,peek} functions are calls to
|
||||
ssl[23]_read_internal with the 'peek' flag set appropriately.
|
||||
A 'peek' parameter has also been added to ssl3_read_bytes, which
|
||||
does the actual work for ssl3_read_internal.
|
||||
[Bodo Moeller]
|
||||
|
||||
*) New function BN_kronecker.
|
||||
|
|
|
@ -14,8 +14,8 @@ SSL_get_error - obtain result code for TLS/SSL I/O operation
|
|||
|
||||
SSL_get_error() returns a result code (suitable for the C "switch"
|
||||
statement) for a preceding call to SSL_connect(), SSL_accept(),
|
||||
SSL_read(), or SSL_write() on B<ssl>. The value returned by that
|
||||
TLS/SSL I/O function must be passed to SSL_get_error() in parameter
|
||||
SSL_read(), SSL_peek(), or SSL_write() on B<ssl>. The value returned by
|
||||
that TLS/SSL I/O function must be passed to SSL_get_error() in parameter
|
||||
B<ret>.
|
||||
|
||||
In addition to B<ssl> and B<ret>, SSL_get_error() inspects the
|
||||
|
@ -64,10 +64,10 @@ TLS/SSL I/O function should be retried.
|
|||
|
||||
Caveat: Any TLS/SSL I/O function can lead to either of
|
||||
B<SSL_ERROR_WANT_READ> and B<SSL_ERROR_WANT_WRITE>. In particular,
|
||||
SSL_read() may want to write data and SSL_write() may want to read
|
||||
data. This is mainly because TLS/SSL handshakes may occur at any time
|
||||
during the protocol (initiated by either the client or the server);
|
||||
SSL_read() and SSL_write() will handle any pending handshakes.
|
||||
SSL_read() or SSL_peek() may want to write data and SSL_write() may want
|
||||
to read data. This is mainly because TLS/SSL handshakes may occur at any
|
||||
time during the protocol (initiated by either the client or the server);
|
||||
SSL_read(), SSL_peek(), and SSL_write() will handle any pending handshakes.
|
||||
|
||||
=item SSL_ERROR_WANT_X509_LOOKUP
|
||||
|
||||
|
|
|
@ -33,8 +33,8 @@ I<read_ahead> flag is set, additional protocol bytes may have been
|
|||
read containing more TLS/SSL records; these are ignored by
|
||||
SSL_pending().
|
||||
|
||||
SSL_pending() does not check if the record type of pending data is
|
||||
application data.
|
||||
Up to OpenSSL 0.9.6, SSL_pending() does not check if the record type
|
||||
of pending data is application data.
|
||||
|
||||
=head1 SEE ALSO
|
||||
|
||||
|
|
|
@ -260,9 +260,6 @@ SSL_CIPHER *ssl2_get_cipher(unsigned int u)
|
|||
|
||||
int ssl2_pending(SSL *s)
|
||||
{
|
||||
/* Unlike ssl2_pending, this one probably works (if read-ahead
|
||||
* is disabled), but it should be examined
|
||||
* XXX */
|
||||
return(s->s2->ract_data_length);
|
||||
}
|
||||
|
||||
|
|
58
ssl/s2_pkt.c
58
ssl/s2_pkt.c
|
@ -138,7 +138,7 @@ static int ssl2_read_internal(SSL *s, void *buf, int len, int peek)
|
|||
return -1;
|
||||
}
|
||||
|
||||
ssl2_read_again:
|
||||
ssl2_read_again:
|
||||
if (SSL_in_init(s) && !s->in_handshake)
|
||||
{
|
||||
n=s->handshake_func(s);
|
||||
|
@ -162,13 +162,22 @@ ssl2_read_again:
|
|||
n=len;
|
||||
|
||||
memcpy(buf,s->s2->ract_data,(unsigned int)n);
|
||||
s->s2->ract_data_length-=n;
|
||||
s->s2->ract_data+=n;
|
||||
if (s->s2->ract_data_length == 0)
|
||||
s->rstate=SSL_ST_READ_HEADER;
|
||||
if (!peek)
|
||||
{
|
||||
s->s2->ract_data_length-=n;
|
||||
s->s2->ract_data+=n;
|
||||
if (s->s2->ract_data_length == 0)
|
||||
s->rstate=SSL_ST_READ_HEADER;
|
||||
}
|
||||
|
||||
return(n);
|
||||
}
|
||||
|
||||
/* s->s2->ract_data_length == 0
|
||||
*
|
||||
* Fill the buffer, then goto ssl2_read_again.
|
||||
*/
|
||||
|
||||
if (s->rstate == SSL_ST_READ_HEADER)
|
||||
{
|
||||
if (s->first_packet)
|
||||
|
@ -266,33 +275,24 @@ ssl2_read_again:
|
|||
INC32(s->s2->read_sequence); /* expect next number */
|
||||
/* s->s2->ract_data is now available for processing */
|
||||
|
||||
#if 1
|
||||
/* How should we react when a packet containing 0
|
||||
* bytes is received? (Note that SSLeay/OpenSSL itself
|
||||
* never sends such packets; see ssl2_write.)
|
||||
* Returning 0 would be interpreted by the caller as
|
||||
* indicating EOF, so it's not a good idea.
|
||||
* Instead, we just continue reading. Note that using
|
||||
* select() for blocking sockets *never* guarantees
|
||||
/* Possibly the packet that we just read had 0 actual data bytes.
|
||||
* (SSLeay/OpenSSL itself never sends such packets; see ssl2_write.)
|
||||
* In this case, returning 0 would be interpreted by the caller
|
||||
* as indicating EOF, so it's not a good idea. Instead, we just
|
||||
* continue reading; thus ssl2_read_internal may have to process
|
||||
* multiple packets before it can return.
|
||||
*
|
||||
* [Note that using select() for blocking sockets *never* guarantees
|
||||
* that the next SSL_read will not block -- the available
|
||||
* data may contain incomplete packets, and except for SSL 2
|
||||
* renegotiation can confuse things even more. */
|
||||
* data may contain incomplete packets, and except for SSL 2,
|
||||
* renegotiation can confuse things even more.] */
|
||||
|
||||
goto ssl2_read_again; /* This should really be
|
||||
* "return ssl2_read(s,buf,len)",
|
||||
* but that would allow for
|
||||
* denial-of-service attacks if a
|
||||
* C compiler is used that does not
|
||||
* recognize end-recursion. */
|
||||
#else
|
||||
/* If a 0 byte packet was sent, return 0, otherwise
|
||||
* we play havoc with people using select with
|
||||
* blocking sockets. Let them handle a packet at a time,
|
||||
* they should really be using non-blocking sockets. */
|
||||
if (s->s2->ract_data_length == 0)
|
||||
return(0);
|
||||
return(ssl2_read(s,buf,len));
|
||||
#endif
|
||||
* "return ssl2_read(s,buf,len)",
|
||||
* but that would allow for
|
||||
* denial-of-service attacks if a
|
||||
* C compiler is used that does not
|
||||
* recognize end-recursion. */
|
||||
}
|
||||
else
|
||||
{
|
||||
|
|
|
@ -782,8 +782,7 @@ SSL_CIPHER *ssl3_get_cipher(unsigned int u)
|
|||
|
||||
int ssl3_pending(SSL *s)
|
||||
{
|
||||
/* The problem is that it may not be the correct record type */
|
||||
return(s->s3->rrec.length); /* FIXME */
|
||||
return (s->s3->rrec.type == SSL3_RT_APPLICATION_DATA) ? s->s3->rrec.length : 0;
|
||||
}
|
||||
|
||||
int ssl3_new(SSL *s)
|
||||
|
|
28
ssl/s3_pkt.c
28
ssl/s3_pkt.c
|
@ -711,17 +711,12 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
|
|||
SSL3_RECORD *rr;
|
||||
void (*cb)()=NULL;
|
||||
|
||||
if (peek)
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_READ_BYTES, SSL_R_FIXME); /* proper implementation not yet completed */
|
||||
return -1;
|
||||
}
|
||||
|
||||
if (s->s3->rbuf.buf == NULL) /* Not initialized yet */
|
||||
if (!ssl3_setup_buffers(s))
|
||||
return(-1);
|
||||
|
||||
if ((type != SSL3_RT_APPLICATION_DATA) && (type != SSL3_RT_HANDSHAKE) && type)
|
||||
if ((type && (type != SSL3_RT_APPLICATION_DATA) && (type != SSL3_RT_HANDSHAKE) && type) ||
|
||||
(peek && (type != SSL3_RT_APPLICATION_DATA)))
|
||||
{
|
||||
SSLerr(SSL_F_SSL3_READ_BYTES, SSL_R_INTERNAL_ERROR);
|
||||
return -1;
|
||||
|
@ -734,6 +729,7 @@ int ssl3_read_bytes(SSL *s, int type, unsigned char *buf, int len, int peek)
|
|||
unsigned char *dst = buf;
|
||||
unsigned int k;
|
||||
|
||||
/* peek == 0 */
|
||||
n = 0;
|
||||
while ((len > 0) && (s->s3->handshake_fragment_len > 0))
|
||||
{
|
||||
|
@ -769,7 +765,7 @@ start:
|
|||
* s->s3->rrec.length, - number of bytes. */
|
||||
rr = &(s->s3->rrec);
|
||||
|
||||
/* get new packet */
|
||||
/* get new packet if necessary */
|
||||
if ((rr->length == 0) || (s->rstate == SSL_ST_READ_BODY))
|
||||
{
|
||||
ret=ssl3_get_record(s);
|
||||
|
@ -787,7 +783,8 @@ start:
|
|||
goto err;
|
||||
}
|
||||
|
||||
/* If the other end has shutdown, throw anything we read away */
|
||||
/* If the other end has shut down, throw anything we read away
|
||||
* (even in 'peek' mode) */
|
||||
if (s->shutdown & SSL_RECEIVED_SHUTDOWN)
|
||||
{
|
||||
rr->length=0;
|
||||
|
@ -816,12 +813,15 @@ start:
|
|||
n = (unsigned int)len;
|
||||
|
||||
memcpy(buf,&(rr->data[rr->off]),n);
|
||||
rr->length-=n;
|
||||
rr->off+=n;
|
||||
if (rr->length == 0)
|
||||
if (!peek)
|
||||
{
|
||||
s->rstate=SSL_ST_READ_HEADER;
|
||||
rr->off=0;
|
||||
rr->length-=n;
|
||||
rr->off+=n;
|
||||
if (rr->length == 0)
|
||||
{
|
||||
s->rstate=SSL_ST_READ_HEADER;
|
||||
rr->off=0;
|
||||
}
|
||||
}
|
||||
return(n);
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue