BIO_do_accept Issue

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

BIO_do_accept Issue

OpenSSL - User mailing list
Run into an odd issue.

Consider the following program, based on the documentation[0], using
OpenSSL 1.1.1f

==BEGIN==

#include <stdio.h>
#include <openssl/bio.h>
#include <openssl/err.h>

int main(int argc, char** argv)
{
        BIO *abio;
        int res;

        abio = BIO_new_accept("4444");
        res = BIO_do_accept(abio);
        printf("First BIO_do_accept returned : %d\n", res);
        if(res <= 0) {
                printf("Should have errored here!\n");
                ERR_print_errors_fp(stderr);
                return 1;
        }
        if(res != 1) {
                printf("This is an error, just not correctly returned!\n");
                ERR_print_errors_fp(stderr);
        }

        res = BIO_do_accept(abio);
        printf("Second BIO_do_accept returned : %d\n", res);
        if(res <= 0) {
                printf("Now we get an error!\n");
                ERR_print_errors_fp(stderr);
                return 2;
        }

        return 0;
}

==END==

It compiles and runs fine, but if there is another app using the port,
the first call to BIO_do_accept returns odd values that don't match the
docs.

C:\openssl_test>main.exe
First BIO_do_accept returned : 356
This is an error, just not correctly returned!
OPENSSL_Uplink(78C93330,08): no OPENSSL_Applink

C:\openssl_test>main.exe
First BIO_do_accept returned : 356
This is an error, just not correctly returned!
OPENSSL_Uplink(79313330,08): no OPENSSL_Applink

C:\openssl_test>main.exe
First BIO_do_accept returned : 384
This is an error, just not correctly returned!
OPENSSL_Uplink(79313330,08): no OPENSSL_Applink

The docs say BIO_do_accept should return 0 or -1 on error. It seems a
simple fix is just to check the return == 1, but why the odd and
inconsistent return values?

Scott

[0] https://www.openssl.org/docs/man1.1.1/man3/BIO_do_accept.html
Reply | Threaded
Open this post in threaded view
|

Re: BIO_do_accept Issue

Viktor Dukhovni
On Wed, Apr 08, 2020 at 11:47:19AM +0100, Scott Morgan via openssl-users wrote:

> Run into an odd issue.
>
> Consider the following program, based on the documentation[0], using
> OpenSSL 1.1.1f
>
> abio = BIO_new_accept("4444");
> res = BIO_do_accept(abio);

It seems to me that since commit 417be660e1c BIO_do_accept() has
incomplete error handling, "ret" isn't assigned when bind() or listen()
fail:

    acpt_state(), crypto/bio//bss_acpt.c: line 241:

        case ACPT_S_LISTEN:
            {
                if (!BIO_listen(c->accept_sock,
                                BIO_ADDRINFO_address(c->addr_iter),
                                c->bind_mode)) {
                    BIO_closesocket(c->accept_sock);
                    goto exit_loop;
                }
            }

    ...
      exit_loop:
        if (bio != NULL)
            BIO_free(bio);
        else if (s >= 0)
            BIO_closesocket(s);
      end:
        return ret;

So the function returns the wrong value of ret, in your case the the
socket descriptor created in an earlier case in the loop.  The fix is
presumably to set ret to either -1 or 0, whichever is appropriate here.

--
    Viktor.
Reply | Threaded
Open this post in threaded view
|

Re: BIO_do_accept Issue

OpenSSL - User mailing list
On 08/04/2020 18:06, Viktor Dukhovni wrote:

> On Wed, Apr 08, 2020 at 11:47:19AM +0100, Scott Morgan via openssl-users wrote:
>
>> Run into an odd issue.
>>
>> Consider the following program, based on the documentation[0], using
>> OpenSSL 1.1.1f
>>
>> abio = BIO_new_accept("4444");
>> res = BIO_do_accept(abio);
>
> It seems to me that since commit 417be660e1c BIO_do_accept() has
> incomplete error handling, "ret" isn't assigned when bind() or listen()
> fail:
>
...<snip>...
>
> So the function returns the wrong value of ret, in your case the the
> socket descriptor created in an earlier case in the loop.  The fix is
> presumably to set ret to either -1 or 0, whichever is appropriate here.
>

That makes sense. Just checked github, and there is a ticket listing
that problem, #7717

Scott
Reply | Threaded
Open this post in threaded view
|

Re: BIO_do_accept Issue

Viktor Dukhovni
On Thu, Apr 09, 2020 at 11:42:11AM +0100, Scott Morgan via openssl-users wrote:

> > It seems to me that since commit 417be660e1c BIO_do_accept() has
> > incomplete error handling, "ret" isn't assigned when bind() or listen()
> > fail:
> >
> ...<snip>...
> >
> > So the function returns the wrong value of ret, in your case the the
> > socket descriptor created in an earlier case in the loop.  The fix is
> > presumably to set ret to either -1 or 0, whichever is appropriate here.
> >
>
> That makes sense. Just checked github, and there is a ticket listing
> that problem, #7717

Thanks for finding that, and posting the PR.  The fix will surely be
included when 1.1.1g is released.

--
    Viktor.