Re: git: 838b6caababb - main - openssl: use getrandom(2) instead of probing for getentropy(2)
Date: Wed, 17 Jul 2024 13:52:12 UTC
On 7/16/24 01:12, Kyle Evans wrote:
> The branch main has been updated by kevans:
>
> URL: https://cgit.FreeBSD.org/src/commit/?id=838b6caababbaaab65659d40a41c2dd46b3a5fd2
>
> commit 838b6caababbaaab65659d40a41c2dd46b3a5fd2
> Author: Kyle Evans <kevans@FreeBSD.org>
> AuthorDate: 2024-07-16 05:12:27 +0000
> Commit: Kyle Evans <kevans@FreeBSD.org>
> CommitDate: 2024-07-16 05:12:27 +0000
>
> openssl: use getrandom(2) instead of probing for getentropy(2)
>
> The probing for getentropy(2) relies on re-declaring getentropy(2)
> as weak and checking the address, but this is incompatible with
> the _FORTIFY_SOURCE symbol renaming scheme. It's always present on
> all supported FreeBSD versions now so we could cut it down to
> unconditional use, but there's another segment for getrandom(2)
> already that's cleaner to just add us to.
>
> We should upstream this.
>
> Reviewed by: kib (earlier version), markj
> Sponsored by: Klara, Inc.
> Sponsored by: Stormshield
> Differential Revision: https://reviews.freebsd.org/D45976
Curiously, while this builds fine on GCC 13, it fails for me on GCC 14 (which
I've just made a port for and started testing):
/usr/local/bin/x86_64-unknown-freebsd14.1-gcc14 --sysroot=/usr/obj/gcc14/usr/home/john/work/freebsd/main/amd64.amd64/tmp -B/usr/local/x86_64-unknown-freebsd14.1/bin/ -O2 -pipe -fno-common -I/usr/home/john/work/freebsd/main/crypto/openssl -I/usr/home/john/work/freebsd/main/crypto/openssl/include -I/usr/home/john/work/freebsd/main/crypto/openssl/providers/common/include -I/usr/home/john/work/freebsd/main/crypto/openssl/providers/implementations/include -DL_ENDIAN -DOPENSSL_CPUID_OBJ -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_MONT5 -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DKECCAK1600_ASM -DRC4_ASM -DMD5_ASM -DAES_ASM -DBSAES_ASM -DVPAES_ASM -DWHIRLPOOL_ASM -DGHASH_ASM -DCMLL_ASM -DECP_NISTZ256_ASM -DX25519_ASM -DPADLOCK_ASM -DPOLY1305_ASM -DOPENSSLDIR="\"/etc/ssl\"" -DENGINESDIR="\"/usr/lib/engines-3\"" -DMODULESDIR="\"/usr/lib/ossl-modules\"" -DNDEBUG -I/usr/home/john/work/freebsd/main/crypto/openssl/crypto -I/usr/home/john/work/freebsd/main/crypto/openssl/crypto/ec/curve448 -I/usr/home/john/work/freebsd/main/crypto/openssl/crypto/ec/curve448/arch_32 -I/usr/home/john/work/freebsd/main/crypto/openssl/crypto/modes -I/usr/obj/gcc14/usr/home/john/work/freebsd/main/amd64.amd64/secure/lib/libcrypto -g -MD -MF.depend.rand_unix.o -MTrand_unix.o -std=gnu99 -Wno-format-zero-length -fstack-protector-strong -Wno-pointer-sign -Wdate-time -Wno-error=address -Wno-error=array-bounds -Wno-error=attributes -Wno-error=bool-compare -Wno-error=cast-align -Wno-error=clobbered -Wno-error=deprecated-declarations -Wno-error=enum-compare -Wno-error=extra -Wno-error=logical-not-parentheses -Wno-error=strict-aliasing -Wno-error=uninitialized -Wno-error=unused-function -Wno-error=unused-value -Wno-error=empty-body -Wno-error=maybe-uninitialized -Wno-error=nonnull-compare -Wno-error=shift-negative-value -Wno-error=tautological-compare -Wno-error=unused-const-variable -Wno-error=bool-operation -Wno-error=deprecated -Wno-error=expansion-to-defined -Wno-error=format-overflow -Wno-error=format-truncation -Wno-error=implicit-fallthrough -Wno-error=int-in-bool-context -Wno-error=memset-elt-size -Wno-error=noexcept-type -Wno-error=nonnull -Wno-error=pointer-compare -Wno-error=stringop-overflow -Wno-error=aggressive-loop-optimizations -Wno-error=cast-function-type -Wno-error=catch-value -Wno-error=multistatement-macros -Wno-error=restrict -Wno-error=sizeof-pointer-memaccess -Wno-error=stringop-truncation -Wno-return-type -Wno-address-of-packed-member -c /usr/home/john/work/freebsd/main/crypto/openssl/providers/implementations/rands/seeding/rand_unix.c -o rand_unix.o
/usr/home/john/work/freebsd/main/crypto/openssl/providers/implementations/rands/seeding/rand_unix.c: In function 'syscall_random':
/usr/home/john/work/freebsd/main/crypto/openssl/providers/implementations/rands/seeding/rand_unix.c:399:12: error: implicit declaration of function 'getrandom'; did you mean 'srandom'? [-Wimplicit-function-declaration]
399 | return getrandom(buf, buflen, 0);
| ^~~~~~~~~
| srandom
*** Error code 1
Stop.
And in fact, getrandom() is declared in <sys/random.h> which isn't included
in rand_unix.c except for DragonFly:
#ifdef __linux
# include <sys/syscall.h>
# ifdef DEVRANDOM_WAIT
# include <sys/shm.h>
# include <sys/utsname.h>
# endif
#endif
#if (defined(__FreeBSD__) || defined(__NetBSD__)) && !defined(OPENSSL_SYS_UEFI)
# include <sys/types.h>
# include <sys/sysctl.h>
# include <sys/param.h>
#endif
#if defined(__OpenBSD__)
# include <sys/param.h>
#endif
#if defined(__DragonFly__)
# include <sys/param.h>
# include <sys/random.h>
#endif
I think we should fix this instead to move the FreeBSD down to the Dragonfly case?
That would better match what you did in the code changes below I think.
You also left sysctl_random() defined (but unused) on FreeBSD.
> ---
> .../openssl/providers/implementations/rands/seeding/rand_unix.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/crypto/openssl/providers/implementations/rands/seeding/rand_unix.c b/crypto/openssl/providers/implementations/rands/seeding/rand_unix.c
> index 750afca58ed7..eadacedbe40c 100644
> --- a/crypto/openssl/providers/implementations/rands/seeding/rand_unix.c
> +++ b/crypto/openssl/providers/implementations/rands/seeding/rand_unix.c
> @@ -356,7 +356,7 @@ static ssize_t syscall_random(void *buf, size_t buflen)
> * Note: Sometimes getentropy() can be provided but not implemented
> * internally. So we need to check errno for ENOSYS
> */
> -# if !defined(__DragonFly__) && !defined(__NetBSD__)
> +# if !defined(__DragonFly__) && !defined(__NetBSD__) && !defined(__FreeBSD__)
> # if defined(__GNUC__) && __GNUC__>=2 && defined(__ELF__) && !defined(__hpux)
> extern int getentropy(void *buffer, size_t length) __attribute__((weak));
>
> @@ -393,11 +393,12 @@ static ssize_t syscall_random(void *buf, size_t buflen)
> /* Linux supports this since version 3.17 */
> # if defined(__linux) && defined(__NR_getrandom)
> return syscall(__NR_getrandom, buf, buflen, 0);
> -# elif (defined(__FreeBSD__) || defined(__NetBSD__)) && defined(KERN_ARND)
> - return sysctl_random(buf, buflen);
> # elif (defined(__DragonFly__) && __DragonFly_version >= 500700) \
> - || (defined(__NetBSD__) && __NetBSD_Version >= 1000000000)
> + || (defined(__NetBSD__) && __NetBSD_Version >= 1000000000) \
> + || defined(__FreeBSD__)
> return getrandom(buf, buflen, 0);
> +# elif defined(__NetBSD__) && defined(KERN_ARND)
> + return sysctl_random(buf, buflen);
> # else
> errno = ENOSYS;
> return -1;
--
John Baldwin