On Wed, Feb 01, 2006 at 09:49:26AM +0100, Nils Larsch wrote:
> >This is chnaging the beahavior of static long conn_ctrl(BIO *b, int cm=

d, long num, void=20
> >*ptr) function.
> >I think it is a typo (just like some other epople using openssl).
> >Nils, could you please confirm me that, or explain me the reason why c=

hanging this?
>=20
> well "if (!data->state !=3D BIO_CONN_S_OK)" obviously doesn't make
> much sense as BIO_CONN_S_OK (=3D=3D 6) is always unequal to 0 or 1,
> but looking at the code again I think it really should be
> "if (data->state !=3D BIO_CONN_S_OK)" as conn_state() does nothing
> if the BIO is already in the BIO_CONN_S_OK state.



"fixing" this typo like that still breaks the state machine at connect
time.

let's say we add some debug around the if statement:

----snip----snip----snip----snip----snip----snip----snip----snip----

case BIO_C_DO_STATE_MACHINE:
/* use this one to start the connection */
fprintf(stderr, "data->state =3D %d\n", data->state);
if (!(data->state !=3D BIO_CONN_S_OK)) {
fprintf(stderr, "called conn_state()\n");
ret=3D(long)conn_state(b,data);
} else {
fprintf(stderr, "whoops! wrong turn!\n");
ret=3D1;
}
break;
=09
----snip----snip----snip----snip----snip----snip----snip----snip----


and use this trivial code for testing:

----snip----snip----snip----snip----snip----snip----snip----snip----

#include

main()
{
BIO *b =3D BIO_new(BIO_s_connect());
int fd, res;

BIO_set_conn_hostname(b, "127.0.0.1:4242");
=09
res =3D BIO_do_connect(b);
fprintf(stderr, "do_connect: %d\n", res);

fd =3D BIO_get_fd(b, &fd);
fprintf(stderr, "fd =3D %d\n", fd);

BIO_puts(b, "test\n");
=09
fd =3D BIO_get_fd(b, &fd);
fprintf(stderr, "fd =3D %d\n", fd);

return 0;
}

----snip----snip----snip----snip----snip----snip----snip----snip----

prior to the incriminated commit, we got this at execution
(assuming some server is listening on 127.0.0.1:4242):

----snip----snip----snip----snip----snip----snip----snip----snip----

$ ./test
data->state =3D 1
called conn_state()
do_connect: 1
fd =3D 3
fd =3D 3

----snip----snip----snip----snip----snip----snip----snip----snip----

everything looks okay.
notice the value of data->state before the BIO_do_connect() call,
which is NOT BIO_CONN_S_OK !

just in case, let's test without anyone listening on 127.0.0.1:4242.

----snip----snip----snip----snip----snip----snip----snip----snip----

$ ./test
data->state =3D 1
called conn_state()
do_connect: -1
fd =3D 3
fd =3D 3

----snip----snip----snip----snip----snip----snip----snip----snip----

BIO_do_connect() returned -1, as expected from a refused connection
to a closed port...

now let's look at what happens with the parentheses from the commit:

----snip----snip----snip----snip----snip----snip----snip----snip----

$ ./test
data->state =3D 1
whoops! wrong turn!
do_connect: 1
fd =3D -1
fd =3D 3

----snip----snip----snip----snip----snip----snip----snip----snip----

uh-oh! conn_state() is obviously not called (noticed the "use this
one to *start* the connection" just above the if in the code ?).

just to be sure, let's try again with a closed port on the other side:

----snip----snip----snip----snip----snip----snip----snip----snip----

$ ./test
data->state =3D 1
whoops! wrong turn!
do_connect: 1
fd =3D -1
fd =3D 3

----snip----snip----snip----snip----snip----snip----snip----snip----

same result, and no error reported. not exactly what I call consistent
behavior...

what's interesting is the fd value. before the BIO_puts() it's -1, which
is normal since *nothing* was done yet due to the "wrong turn".
after the BIO_puts() call, the connection is established, because of
this code in conn_write() (the same is also true if we do a read
instead):

----snip----snip----snip----snip----snip----snip----snip----snip----

if (data->state !=3D BIO_CONN_S_OK)
{
ret=3Dconn_state(b,data);
if (ret <=3D 0)
return(ret);
}

----snip----snip----snip----snip----snip----snip----snip----snip----

this effectively establishes the connection, but a bit too late.
I don't know if you're like that, but when I do a connect(2) call on
unix sockets, I like to have my socket right now, not just after the
first read() or write() :P


IMHO, the negation in your commit should be removed, and the if statement
(which looks like it's been some "miraculously-working" typo since a
few years from now) should just be like the ones in conn_read() and
conn_write().


Cheers,

--=20
Gabriel Fort=E9

__________________________________________________ ____________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majordomo@openssl.org