libressl / portable

LibreSSL Portable itself. This includes the build scaffold and compatibility layer that builds portable LibreSSL from the OpenBSD source code. Pull requests or patches sent to tech@openbsd.org are welcome.
https://www.libressl.org
1.36k stars 267 forks source link

*_it variables (ASN1_ITEM for builtin types) not accessible on Windows #350

Open rhenium opened 7 years ago

rhenium commented 7 years ago

(This issue was originally reported at https://bugs.ruby-lang.org/issues/13902)

When an application is compiled with cl.exe, it cannot access *_it variables properly (OCSP_BASICRESP_it in the example code below).

I can reproduce this with both libressl-2.2.9-windows.zip and libressl-2.5.5-windows.zip downloaded from https://ftp.openbsd.org/pub/OpenBSD/LibreSSL/.

test.c:

#include <stdio.h>
#include <stdlib.h>
#include <openssl/ocsp.h>
#include <openssl/asn1t.h>

int main(int argc, char **argv)
{
    OCSP_BASICRESP *bs;
    size_t i;
    int ret;

    puts("dump of OCSP_BASICRESP_it:");
    for (i = 0; i < sizeof(OCSP_BASICRESP_it); i++)
        printf("%02X", ((unsigned char *)&OCSP_BASICRESP_it)[i]);
    puts("");

    bs = OCSP_BASICRESP_new();
    if (!bs) {
        puts("bad alloc");
        return 1;
    }

    /*
     * in crypto/ocsp/ocsp_asn.c:
     *
     * int
     * i2d_OCSP_BASICRESP(OCSP_BASICRESP *a, unsigned char **out)
     * {
     *  return ASN1_item_i2d((ASN1_VALUE *)a, out, &OCSP_BASICRESP_it);
     * }
     */
    puts("trying i2d_OCSP_BASICRESP():");
    ret = i2d_OCSP_BASICRESP(bs, NULL);
    if (ret < 0)
        puts("ok i2d_(,NULL) error");
    else
        puts("ok");

    puts("trying ASN1_item_i2d():");
    ret = ASN1_item_i2d((ASN1_VALUE *)bs, NULL,
                ASN1_ITEM_rptr(OCSP_BASICRESP));
    if (ret < 0)
        puts("ok ASN1_item_i2d(,NULL,) error");
    else
        puts("ok");
}
PS C:\mswin64\src\test> cl /Zi /I\mswin64\libressl-2.5.5-windows\include test.c C:\mswin64\libressl-2.5.5-windows\x64\libcrypto-41.lib
Microsoft (R) C/C++ Optimizing Compiler Version 19.11.25507.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test.c
Warning, overriding WinCrypt defines
Microsoft (R) Incremental Linker Version 14.11.25507.1
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:test.exe
/debug
test.obj
C:\mswin64\libressl-2.5.5-windows\x64\libcrypto-41.lib
PS C:\mswin64\src\test> .\test.exe
dump of OCSP_BASICRESP_it:
FF25BC5E070040534883EC20B901000000E858CBFFFFE86CADFFFF8BC8E871ABFFFFE85EB4FFFF488BD8E802B8FFFFB9
trying i2d_OCSP_BASICRESP():
ok
trying ASN1_item_i2d():
[ segfault in ASN1_item_ex_i2d() ]

I didn't dig further, but it looks like ASN1_item_i2d() is crashing because OCSP_BASICRESP_it points to garbage.

kinichiro commented 7 years ago

Hi, Can you try to edit openssl/ocsp.h like below, before building test.c ?

//extern const ASN1_ITEM OCSP_BASICRESP_it;
_declspec(dllimport) ASN1_ITEM OCSP_BASICRESP_it;

Also, you need to replace the include files order of ocsp.h and asn1t.h.

With Visual Studio 2017, I could see this resolves the issue. Does this work well in your environment, too ?

rhenium commented 7 years ago

Thank you for looking into this. I'm also using VS 2017, and editing the header as suggested by you fixed the issue for me.

> .\test
dump of OCSP_BASICRESP_it:
0100000010000000E0E75D5D0000000004000000000000000000000000000000200000000000000040E75D5D00000000
trying i2d_OCSP_BASICRESP():
ok
trying ASN1_item_i2d():
ok
kinichiro commented 7 years ago

Thanks for your confirmation. This seems Windows specific issue, and this doesn't affect to other platforms. I had already tried to see test.c runs fine on Fedora. To solve this issue, it is required to replace extern const to _declspec(dllimport) only for Windows client program. Now, I have no good idea to adjust this yet.

rhenium commented 7 years ago

For what it's worth, OpenSSL has OPENSSL_EXTERN macro that expands to __declspec(dllimport) as necessary.

https://github.com/openssl/openssl/blob/9be34ee5c8576539a929d5b396ad071aed525f43/include/openssl/e_os2.h#L149-L174

It looks like it's been removed in LibreSSL very soon after the fork.

https://github.com/libressl-portable/openbsd/commit/e2b579c6444448cfb7d95988539fb88caa48c2e5#diff-5300baa69671290cb1254537e4699dc2

busterb commented 6 years ago

I'm thinking an addition on the affected headers like this:

#ifdef _WIN32
#ifndef LIBRESSL_INTERNAL
  #define LIBRESSL_IMPORT    __declspec(dllimport)
#else
  #define LIBRESSL_IMPORT
#endif
#else
#define LIBRESSL_IMPORT
#endif

Then a patch that does something like this to the *_it constants:

extern LIBRESSL_IMPORT const ASN1_ITEM OCSP_RESPID_it;

Is this still an issue if you build the dll's with msc to begin with (I suspect it is)? We stopped providing the pre-built DLLs due to other ABI incompatibilities between Visual Studio and MinGW64-produced binaries.