openssl: 3.2-alpha2 test_bio_addr fails on NonStop

The 61-test_bio_addr fails on NonStop x86 with reseeding not being detected. We are using the hardware randomizer on x86. The hardware randomizer has worked since 3.0, but this looks like it might be a new situation.

Assistance requested to localize what is or is not happening.

61-test_bio_addr.t ...................... 
# The results of this test will end up in test-runs/test_bio_addr
1..1
    # Subtest: ../../test/bio_addr_test
    1..1
        # Subtest: test_bio_addr_copy_dup
        1..6
        ok 1 - iteration 1
        ok 2 - iteration 2
        ok 3 - iteration 3
        ok 4 - iteration 4
        # ERROR: (bool) 'bio_addr_is_eq(src, dst) == true' failed @ /home/jenkinsbuild/.jenkins/workspace/OpenSSL-3.2_Monitor/test/bio_addr_test.c:145
        # false
        # OPENSSL_TEST_RAND_SEED=1695973622
        not ok 5 - iteration 5
        # ERROR: (bool) 'bio_addr_is_eq(src, dst) == true' failed @ /home/jenkinsbuild/.jenkins/workspace/OpenSSL-3.2_Monitor/test/bio_addr_test.c:145
        # false
        # OPENSSL_TEST_RAND_SEED=1695973622
        not ok 6 - iteration 6
    # OPENSSL_TEST_RAND_SEED=1695973622
    not ok 1 - test_bio_addr_copy_dup
../../util/wrap.pl ../../test/bio_addr_test => 255
not ok 1 - running bio_addr_test

#   Failed test 'running bio_addr_test'
#   at test/recipes/61-test_bio_addr.t line 20.
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)

Configuration:

Command line (with current working directory = .):

    perl ./Configure nonstop-nsx_64 --with-rand-seed=rdcpu enable-fips

Perl information:

    perl
    5.30.3 for NSV-D-nonstop_kernel

Enabled features:

    acvp-tests
    apps
    argon2
    aria
    async
    autoalginit
    autoerrinit
    autoload-config
    bf
    blake2
    bulk
    cached-fetch
    camellia
    capieng
    cast
    chacha
    cmac
    cmp
    cms
    comp
    ct
    deprecated
    des
    dgram
    dh
    docs
    dsa
    dso
    dtls
    dynamic-engine
    ec
    ec2m
    ecdh
    ecdsa
    ecx
    egd
    engine
    err
    filenames
    fips
    fips-securitychecks
    gost
    http
    idea
    legacy
    loadereng
    md4
    mdc2
    module
    multiblock
    nextprotoneg
    ocb
    ocsp
    padlockeng
    pic
    pinshared
    poly1305
    posix-io
    psk
    quic
    rc2
    rc4
    rdrand
    rfc3779
    rmd160
    scrypt
    secure-memory
    seed
    shared
    siphash
    siv
    sm2
    sm2-precomp
    sm3
    sm4
    sock
    srp
    srtp
    sse2
    ssl
    ssl-trace
    static-engine
    stdio
    tests
    tls
    ts
    ui-console
    whirlpool
    tls1
    tls1-method
    tls1_1
    tls1_1-method
    tls1_2
    tls1_2-method
    tls1_3
    dtls1
    dtls1-method
    dtls1_2
    dtls1_2-method

Disabled features:

    afalgeng            [not-linux]      OPENSSL_NO_AFALGENG
    asan                [default]        OPENSSL_NO_ASAN
    asm                 [no asm_arch]    OPENSSL_NO_ASM
    brotli              [default]        OPENSSL_NO_BROTLI
    brotli-dynamic      [default]        OPENSSL_NO_BROTLI_DYNAMIC
    buildtest-c++       [default]
    winstore            [not-windows]    OPENSSL_NO_WINSTORE
    crypto-mdebug       [default]        OPENSSL_NO_CRYPTO_MDEBUG
    default-thread-pool [cascade]        OPENSSL_NO_DEFAULT_THREAD_POOL
    devcryptoeng        [default]        OPENSSL_NO_DEVCRYPTOENG
    ec_nistp_64_gcc_128 [default]        OPENSSL_NO_EC_NISTP_64_GCC_128
    external-tests      [default]        OPENSSL_NO_EXTERNAL_TESTS
    fuzz-afl            [default]        OPENSSL_NO_FUZZ_AFL
    fuzz-libfuzzer      [default]        OPENSSL_NO_FUZZ_LIBFUZZER
    ktls                [default]        OPENSSL_NO_KTLS
    makedepend          [unavailable]
    md2                 [default]        OPENSSL_NO_MD2 (skip crypto/md2)
    msan                [default]        OPENSSL_NO_MSAN
    rc5                 [default]        OPENSSL_NO_RC5 (skip crypto/rc5)
    sctp                [default]        OPENSSL_NO_SCTP
    tfo                 [default]        OPENSSL_NO_TFO
    thread-pool         [cascade]        OPENSSL_NO_THREAD_POOL
    threads             [config]
    trace               [default]        OPENSSL_NO_TRACE
    ubsan               [default]        OPENSSL_NO_UBSAN
    unit-test           [default]        OPENSSL_NO_UNIT_TEST
    uplink              [no uplink_arch] OPENSSL_NO_UPLINK
    weak-ssl-ciphers    [default]        OPENSSL_NO_WEAK_SSL_CIPHERS
    zlib                [default]        OPENSSL_NO_ZLIB
    zlib-dynamic        [default]        OPENSSL_NO_ZLIB_DYNAMIC
    zstd                [default]        OPENSSL_NO_ZSTD
    zstd-dynamic        [default]        OPENSSL_NO_ZSTD_DYNAMIC
    ssl3                [default]        OPENSSL_NO_SSL3
    ssl3-method         [default]        OPENSSL_NO_SSL3_METHOD

Config target attributes:

    AR => "ar",
    ARFLAGS => "qc",
    CC => "c99",
    HASHBANGPERL => "/usr/bin/env perl",
    RANLIB => "ranlib",
    RC => "windres",
    bn_ops => "SIXTY_FOUR_BIT",
    build_file => "Makefile",
    build_scheme => [ "unified", "unix" ],
    cflags => "-g -O2 -Wextensions -Wnowarn=203,220,272,734,770,1506 -Wbuild_neutral_library -Wverbose -Wtarget=tns/x -Wsystype=oss -Wlp64 -WIEEE_float",
    cppflags => "",
    defines => [ "OPENSSL_BUILDING_OPENSSL", "OPENSSL_VPROC=\$(OPENSSL_VPROC)", "_XOPEN_SOURCE", "_XOPEN_SOURCE_EXTENDED=1", "_TANDEM_SOURCE", "__NSK_OPTIONAL_TYPES__", "B_ENDIAN" ],
    disable => [ "threads" ],
    dso_scheme => "DLFCN",
    enable => [ "egd" ],
    ex_libs => "-lrld",
    includes => [  ],
    lflags => "-Wxld=\"-set systype oss\" -Wxld=\"-set data_model lp64\" -Wxld=\"-set floattype ieee_float\"",
    lib_cflags => "",
    lib_cppflags => "",
    lib_defines => [  ],
    module_cflags => "",
    module_cxxflags => undef,
    module_ldflags => "-Wshared",
    multibin => "64",
    multilib => "64",
    perl => "/usr/bin/perl",
    perl_platform => "Unix",
    shared_argfileflag => "-Wxld_obey=",
    shared_cflag => "",
    shared_defflag => "-Wxld_obey=",
    shared_defines => [  ],
    shared_extension => ".so",
    shared_ldflag => "-Wshared",
    shared_rcflag => "",
    shared_target => "nonstop-shared",
    sys_id => "TANDEM",
    thread_defines => [  ],
    thread_scheme => "(unknown)",
    unistd => "<unistd.h>",

Recorded environment:

    AR =
    ARFLAGS =
    AS =
    ASFLAGS =
    BUILDFILE =
    CC =
    CFLAGS =
    CPP =
    CPPDEFINES =
    CPPFLAGS =
    CPPINCLUDES =
    CROSS_COMPILE =
    CXX =
    CXXFLAGS =
    HASHBANGPERL =
    LD =
    LDFLAGS =
    LDLIBS =
    MT =
    MTFLAGS =
    OPENSSL_LOCAL_CONFIG_DIR =
    PERL =
    RANLIB =
    RC =
    RCFLAGS =
    RM =
    WINDRES =
    __CNF_CFLAGS =
    __CNF_CPPDEFINES =
    __CNF_CPPFLAGS =
    __CNF_CPPINCLUDES =
    __CNF_CXXFLAGS =
    __CNF_LDFLAGS =
    __CNF_LDLIBS =

Makevars:

    AR              = ar
    ARFLAGS         = qc
    ASFLAGS         =
    CC              = c99
    CFLAGS          =
    CPPDEFINES      =
    CPPFLAGS        =
    CPPINCLUDES     =
    CXXFLAGS        =
    HASHBANGPERL    = /usr/bin/env perl
    LDFLAGS         =
    LDLIBS          =
    PERL            = perl
    RANLIB          = ranlib
    RC              = windres
    RCFLAGS         =

NOTE: These variables only represent the configuration view.  The build file
template may have processed these variables further, please have a look at the
build file for more exact data:
    Makefile

build file:

    Makefile

build file templates:

    Configurations/common0.tmpl
    Configurations/unix-Makefile.tmpl

About this issue

Original URL
State: closed
Created 9 months ago
Comments: 38 (38 by maintainers)

Commits related to this issue

Fix the BIO_addr test The BIO_addr test is failing on non-stop. The length of the data is larger than the size we have allocated for it. We dynamically allocate instead. Fixes #22218 — committed to mattcaswell/openssl by mattcaswell 9 months ago
limit length of AF_UNIX sockets to the max socket size If an AF_UNIX socket sets a sun_addr that uses all 108 bytes of the sun_addr field (i.e. doesn't include a null terminator), then strlen will re... — committed to nhorman/openssl by nhorman 9 months ago
limit length of AF_UNIX sockets to the max socket size If an AF_UNIX socket sets a sun_addr that uses all 108 bytes of the sun_addr field (i.e. doesn't include a null terminator), then strlen will re... — committed to nhorman/openssl by nhorman 9 months ago
limit length of AF_UNIX sockets to the max socket size If an AF_UNIX socket sets a sun_addr that uses all 108 bytes of the sun_addr field (i.e. doesn't include a null terminator), then strlen will re... — committed to nhorman/openssl by nhorman 9 months ago
Accomodate NonStop definition of sockaddr_storage Currently, on NonStop, sockaddr_storage appears to be far too small to accomodate struct sockaddr_un (the former being about 198 bytes, the latter 10... — committed to nhorman/openssl by nhorman 9 months ago
test/bio_addr_test.c: don't rely on sockaddr_storage It's a fairly recent POSIX addition, and therefore not available on some older platforms. Furthermore, it's been found unreliable on other platfo... — committed to levitte/openssl by levitte 9 months ago
Fix the BIO_addr test The BIO_addr test is failing on non-stop. The length of the data is larger than the size we have allocated for it. We dynamically allocate instead. Fixes #22218 Reviewed-by: P... — committed to absole/openssl by mattcaswell 9 months ago

Most upvoted comments

Thanks. Please can you try out the fix in #22294?

Thanks @mattcaswell

61-test_bio_addr.t ..
# The results of this test will end up in test-runs/test_bio_addr
1..1
    # Subtest: ../../test/bio_addr_test
    1..1
        # Subtest: test_bio_addr_copy_dup
        1..6
        ok 1 - iteration 1
        ok 2 - iteration 2
        ok 3 - iteration 3
        ok 4 - iteration 4
        ok 5 - iteration 5
        ok 6 - iteration 6
    ok 1 - test_bio_addr_copy_dup
../../util/wrap.pl ../../test/bio_addr_test => 0
ok 1 - running bio_addr_test
ok
All tests successful.
Files=1, Tests=1,  0 wallclock secs ( 0.01 usr  0.00 sys +  0.41 cusr  0.00 csys =  0.42 CPU)
Result: PASS
make[1]: Leaving directory '/home/randall/openssl-3.2'

rsbeckerca on Oct 5, 2023

There it is, thats the problem!

In nonstop sockaddr_un (from your comment here https://github.com/openssl/openssl/issues/22218#issuecomment-1751826845)

Is defined to be 1024 + 16 (for sa_family_t) = 1040 bytes, but sockaddr_storage on the same platform is only defined to be 16+6 + 64 + 112 = 198 bytes

in bio_addr_test, we test for equality of two cloned BIO_ADDR structs, which we fetch by declaring two stockaddr_storage structs (adata and bdata) on the stack. Given that those are two automatic allocations that are adjacent, when we call BIO_ADDR_rawaddress(a, &data, &alen) in that function, we find that a->s_un.sun_path is 1024 bytes, and completely overwrite the sockaddr_storage adata struct, well into part of bdata, and beyond. Then we call BIO_ADDR_rawaddress (a, &bdata, &blen) which does it again

So our adata and bdata values become completely corrupt, leading to a bad result in memcmp down below in bio_addr_is_eq

According to posix: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/socket.h.html

The <sys/socket.h> header shall define the sockaddr_storage structure. This structure shall be:

Large enough to accommodate all supported protocol-specific address structures

Aligned at an appropriate boundary so that pointers to it can be cast as pointers to protocol-specific address structures and used to access the fields of those structures without alignment problems

The definition of sockaddr_storage on nonstop violates that first provision, as it is not large enough to hold the currently defined sockaddr_un struct

I think the correct fix here is to modify the definition of sockaddr_storage such that: _SS_PAD1SIZE is at least 1024 bytes and remove __ss_pad2

I know that kind of stinks, as modifying the OS is more difficult, but that clearly doesn’t seem right, and should cause any number of other problems where sockaddr_storage is used in conjunction with AF_INET sockets on nonstop platforms.

An alternate interim solution might be to change the definition of adata and bdata to be a uniion of sockaddr_in, sockaddr_in6, and sockaddr_un, and cast them as appropriate for each address family. That would give us the storage size required, but the correct fix really is to modify the definition of sockaddr_storage, as any usage of it on that platform has the potential to break in this same manner.

nhorman on Oct 8, 2023