Fix/ handle long pvc names

instead of putting the original PVC name into a label, we write it to an
annotation
also, PVC name are no longer truncated at 63 chars, as they can be up to 253
long (DNS name)
finally, instead the migration's pod name is prefixed with migrate- and
truncated to a max total length of 63 chars (as pod name should typically
not be longer than 63 chars)

closes #155 #171

also, I noticed that the baseAnnotation=sourcePvcName isn't actually used when swapping off the PVCs after migration.
please let me know if I missed the usage for this annotation, but I think we can change it from a label to an annotation without trouble.
also, here's a log of a migration for a PVC with a 253 chars long name:

pvmigrate_log.txt
./pvmigrate -source-sc nfs -dest-sc nfs -skip-preflight-validation -namespace=cjmnuss
Running pvmigrate build:
version=(devel)
sha=
time=

Found 1 StorageClass:
nfs

Migrating data from nfs to nfs
PV pvc-641fbe2b-3e2a-49ba-a58b-06090f6e1c3b does not match source SC nfs, not migrating

Found 1 matching PVCs to migrate across 1 namespaces:
namespace: pvc:                                                                                pv:                                      size:
cjmnuss    super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate pvc-2e452efe-9733-4f56-8861-9fecc9f9038d 1Gi
cjmnuss    super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong pvc-ead5ef1d-5b07-4152-8e63-b3469f8ab042 1Gi

Creating new PVCs to migrate data to using the nfs StorageClass
created new PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglononglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate-pvcmigrate with size 1Gi in cjmnuss
found existing PVC with name super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate, not creating new one

Found 1 matching pods to migrate across 1 namespaces:
namespace: pod:
cjmnuss    test-deploy-attached-to-super-long-pvc-with-a-name-63-charv547h

Scaling down StatefulSets and Deployments with matching PVCs
scaling Deployment test-deploy-attached-to-super-long-pvc-with-a-name-63-char-long-659898f79f from 1 to 0 in cjmnuss

Waiting for pods with mounted PVCs to be cleaned up
Found pod test-deploy-attached-to-super-long-pvc-with-a-name-63-charv547h in cjmnuss mounting to-be-migrated PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong, waiting
All pods removed successfully

Copying data from nfs PVCs to nfs PVCs
Copying data from super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate (pvc-2e452efe-9733-4f56-8861-9fecc9f9038d) to super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglononglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate-pvcmigrate in cjmnuss
Determining the node to migrate PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate on
Creating pvc migrator pod on node
waiting for pod migrate--super-long-pvc-253-chaglonglonglonglonglong-pvcmigrate to start in cjmnuss
migrating PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate:
.done!
finished migrating PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate
Copying data from super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong (pvc-ead5ef1d-5b07-4152-8e63-b3469f8ab042) to super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate in cjmnuss
Determining the node to migrate PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong on
Creating pvc migrator pod on node abcd.domain.ch
waiting for pod migrate--super-long-pvc-253-chalonglonglonglonglonglonglonglong to start in cjmnuss
migrating PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong:
..done!
finished migrating PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong

Swapping PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate in cjmnuss to the new StorageClass
Marking original PV pvc-2e452efe-9733-4f56-8861-9fecc9f9038d as to-be-retained
Marking migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb as to-be-retained
Deleting original PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate in cjmnuss to free up the name
Deleting migrated-to PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglononglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate-pvcmigrate in cjmnuss to release the PV
Waiting for original PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate in cjmnuss to finish deleting
Waiting for migrated-to PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglononglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate-pvcmigrate in cjmnuss to finish deleting
Removing claimref from original PV pvc-2e452efe-9733-4f56-8861-9fecc9f9038d
Removing claimref from migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb
Creating new PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate with migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb
Resetting migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb reclaim policy
Got conflict updating PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb, waiting 5s to retry
Deleting original, now redundant, PV pvc-2e452efe-9733-4f56-8861-9fecc9f9038d
Successfully migrated PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate in cjmnuss from PV pvc-2e452efe-9733-4f56-8861-9fecc9f9038d to pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb

Swapping PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong in cjmnuss to the new StorageClass
Marking original PV pvc-ead5ef1d-5b07-4152-8e63-b3469f8ab042 as to-be-retained
Marking migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb as to-be-retained
Deleting original PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong in cjmnuss to free up the name
Deleting migrated-to PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate in cjmnuss to release the PV
Waiting for original PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong in cjmnuss to finish deleting
Waiting for migrated-to PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonnglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong-pvcmigrate in cjmnuss to finish deleting
Removing claimref from original PV pvc-ead5ef1d-5b07-4152-8e63-b3469f8ab042
Removing claimref from migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb
Creating new PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong with migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb
Resetting migrated-to PV pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb reclaim policy
Deleting original, now redundant, PV pvc-ead5ef1d-5b07-4152-8e63-b3469f8ab042
Successfully migrated PVC super-long-pvc-253-chars-longlonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglonglong in cjmnuss from PV pvc-ead5ef1d-5b07-4152-8e63-b3469f8ab042 to pvc-3e2be7e1-fa9b-4d32-90f5-5a553a5104bb

Scaling back StatefulSets and Deployments with matching PVCs
scaling Deployment test-deploy-attached-to-super-long-pvc-with-a-name-63-char-long from 0 to 1 in cjmnuss

Success!

I'm sorry for leaving this open for so long! I'm taking a look now

Thank you for the PR @clementnuss !


rand_win.c: avoid explicit linking to bcrypt.dll, use dynamic loading to check if BCryptGenRandom is available

The change allows delay loading of bcrypt.dll (see https://docs.microsoft.com/en-us/cpp/build/reference/delayload-delay-load-import). Fixes #8915

If you want to support windows version that doesn't have bcrypt, you have the at least those options:

Actually build on that windows version
Pass -D_WIN32_WINNT=0x0501 to config
Avoid the call to BCryptGenRandom when it's not available and delay load bcrypt.dll
Use LoadLibrary and GetProcAddress

This change seems to do half of the 3rd option: it's using LoadLibrary to see if the call is supported, and then avoids calling the function. It doesn't do they delay loading part.

This change seems to do half of the 3rd option: it's using LoadLibrary to see if the call is supported, and then avoids calling the function. It doesn't do they delay loading part.

I agree, my proposed change doesn't fully solve the problem. Delay loading for bcrypt.dll would need to be specified as a linker option of any 3rd party app that links to openssl statically and obviously cannot be part of openssl lib itself (openssl cmd line tool or dll build could use delay-loading for bcrypt and would work on xp then with this change). Unfortunately, this cannot be done as a pragma linker option directly in source code, it used to be possible, but doesn't work anymore.
If delay loading was specified, you don't need to do LoadLibrary to check if bcrypt.dll exists, GetModulehandle works correctly. It's a bit confusing from your comment if you suggested or not to use LoadLibrary instead of GetModulehandle check.
In my case I prefer not to cripple builds by targeting xp (by setting 0x0501 winnt version), but at the same time I prefer to make that these builds work on xp if that's a minor change. In case of openssl I only get that problem with BCryptGenRandom. LoadLibrary/GetProcAddress would be a better general solution here.

I have no idea what the best option is. But at least for the
shared builds, which I think is what people should use, this
doesn't solve it. The only way I'm sure it's actually solved in
all cases it to load the library at runtime, not using the delayed
load. But I don't actually really like doing that.
I'm also not sure we should put such an effort in still supporting XP.

I'm also not sure we should put such an effort in still supporting XP.

In my case we have millions of users on XP and because of user base growth rate this number is increasing. XP has outdated ssl support, I can replace winssl with openssl, but after linking with openssl my app wouldn't even start.

I updated the change with LoadLibrary/GetProcAddress. This is the proper solution overall

For this to be accepted, you need to submit a signed CLA. See https://www.openssl.org/policies/cla.html

For this to be accepted, you need to submit a signed CLA. See https://www.openssl.org/policies/cla.html

I did it a few days ago.

I did it a few days ago.

Ah, right, I can see the email...  the OMC is currently busy in a face-to-face meeting, that's the reason it hasn't been registered yet.

Close / open to kick the cla checking bot

There, CLA checker happy

Code updated to use RUN_ONCE to ensure trade safety

@pps83 I'm seeing build failures under mingw cross compiled from Linux: I think the original ifdef check for _MSC_VER was guarding against non-MSVC compilers targetting Windows

@pps83 I'm seeing build failures under mingw cross compiled from Linux: I think the original ifdef check for _MSC_VER was guarding against non-MSVC compilers targetting Windows

That _MSC_VER in the existing code does not make sense IMO. Bcrypt API in newer windows versions obviously has nothing to do with what compiler is used to compile the code. The only reason the check is there is because that code relies on  #  pragma comment(lib, "bcrypt.lib"), on top of that non-ms compilers perhaps don't even have proper windows SDK and don't have bcrypt support.
Anyways, it would be more helpful if you could post relevant logs for the errors you are getting, so I would be able to update the change.

I you're right that I'm really referring to the toolchain rather than the compiler, and the mingw cross compiler I'm using doesn't bundle or have access to the relevant Windows headers.
Running ./Configure no-shared no-gost mingw -g with your patch (7fbd969) on Linux:

Fedora 14
Toolchain from packages:
mingw32-runtime-3.15.2-5.fc13.noarch
mingw32-dlfcn-0-0.7.r11.fc13.noarch
mingw32-gcc-c++-4.5.0-1.fc14.x86_64
mingw32-filesystem-64-2.fc14.noarch
mingw32-cpp-4.5.0-1.fc14.x86_64
mingw32-w32api-3.14-1.fc14.noarch
mingw32-gcc-4.5.0-1.fc14.x86_64
mingw32-dlfcn-static-0-0.7.r11.fc13.noarch
mingw32-binutils-2.20.1-2.fc14.x86_64
mingw32-pthreads-2.8.0-10.fc13.noarch

Compile error:
i686-pc-mingw32-gcc  -I. -Icrypto/include -Iinclude -m32 -Wall -O3 -fomit-frame-pointer -g -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_BN_ASM_PART_WORDS -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DRC4_ASM -DMD5_ASM -DRMD160_ASM -DAES_ASM -DVPAES_ASM -DWHIRLPOOL_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DPOLY1305_ASM -DOPENSSLDIR="\"/usr/local/ssl\"" -DENGINESDIR="\"/usr/local/lib/engines-1_1\"" -DUNICODE -D_UNICODE -DWIN32_LEAN_AND_MEAN -D_MT -DNDEBUG  -MMD -MF crypto/rand/rand_win.d.tmp -MT crypto/rand/rand_win.o -c -o crypto/rand/rand_win.o crypto/rand/rand_win.c
crypto/rand/rand_win.c:37:25: error: expected declaration specifiers or '...' before '*' token
crypto/rand/rand_win.c:37:62: error: expected ')' before 'PUCHAR'
crypto/rand/rand_win.c:39:24: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'bcrypt_gen_random'
crypto/rand/rand_win.c: In function 'do_bcrypt_init':
crypto/rand/rand_win.c:45:9: error: 'bcrypt_gen_random' undeclared (first use in this function)
crypto/rand/rand_win.c:45:9: note: each undeclared identifier is reported only once for each function it appears in
crypto/rand/rand_win.c:45:30: error: 'BCRYPTGENRANDOM' undeclared (first use in this function)
crypto/rand/rand_win.c:45:46: error: expected ';' before 'GetProcAddress'
crypto/rand/rand_win.c: In function 'rand_pool_acquire_entropy':
crypto/rand/rand_win.c:75:13: warning: implicit declaration of function 'bcrypt_gen_random'
crypto/rand/rand_win.c: In function 'do_bcrypt_init':
crypto/rand/rand_win.c:48:1: warning: control reaches end of non-void function

Our build environment aborts all platforms on the first failure, so I'm unsure if the mingw-w64 toolchains have the same issue, but it looks like they contain a bcrypt.h, so I'm expecting that this is issue only manifests with the older mingw toolchain.

Running ./Configure no-shared no-gost mingw -g with your patch (7fbd969) on Linux:

I tried that same command with msys2 on windows and it compiles for me. Can you please try to fiddle a bit with
typedef NTSTATUS(WINAPI *BCRYPTGENRANDOM)(BCRYPT_ALG_HANDLE, PUCHAR, ULONG,
                                          ULONG);

to find out what part exactly fails?
for example, paste this piece right before the typedef and it should be able to tell us what exactly fails.
NTSTATUS testVar1;
BCRYPT_ALG_HANDLE testVar2;
PUCHAR testVar3;
ULONG testVar4;
WINAPI void testFunc();

Looks like NTSTATUS and BCRYPT_ALG_HANDLE are the problematic types:
crypto/rand/rand_win.c:37:10: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'testVar1'
crypto/rand/rand_win.c:38:19: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'testVar2'

msys2 is the mingw-w64 toolchain, which contains a bcrypt.h, so good to know that the problem I'm seeing is limited to the original mingw (msys1) toolchain.
I tried reproducing the issue with the msys (http://www.mingw.org/wiki/MSYS) on Windows, but the version of perl available in the package manager maxes out at 5.8 (5.10 required) and my other perl version is built to use Windows paths (Unix paths required). That stumbling block isn't an issue in my cross compiler environment, since Fedora14's native perl is new enough.

i686-pc-mingw32-gcc  -I. -Icrypto/include -Iinclude -m32 -Wall -O3 -fomit-frame-pointer -g -DL_ENDIAN -DOPENSSL_PIC -DOPENSSL_CPUID_OBJ -DOPENSSL_BN_ASM_PART_WORDS -DOPENSSL_IA32_SSE2 -DOPENSSL_BN_ASM_MONT -DOPENSSL_BN_ASM_GF2m -DSHA1_ASM -DSHA256_ASM -DSHA512_ASM -DRC4_ASM -DMD5_ASM -DRMD160_ASM -DAES_ASM -DVPAES_ASM -DWHIRLPOOL_ASM -DGHASH_ASM -DECP_NISTZ256_ASM -DPOLY1305_ASM -DOPENSSLDIR="\"/usr/local/ssl\"" -DENGINESDIR="\"/usr/local/lib/engines-1_1\"" -DUNICODE -D_UNICODE -DWIN32_LEAN_AND_MEAN -D_MT -DNDEBUG  -MMD -MF crypto/rand/rand_win.d.tmp -MT crypto/rand/rand_win.o -c -o crypto/rand/rand_win.o crypto/rand/rand_win.c
crypto/rand/rand_win.c:23:21: fatal error: bcrypt.h: No such file or directory

Those typedefs reduced the error count, but now the compiler has gotten far enough to report that there's no bcrypt.h to be found.
The current version of this file is using the _MSC_VER to switch bcrypt usage based on the compiler, which works for msys1 but msys2 can do better. Could the Configure script check for bcrypt.h and set USE_BCRYPTGENRANDOM based on the existence? I don't see a clear way to implement such checks though.
I guess a disabling define could be added to the mingw configuration options, but it's used for both the older msys1 mingw32 and newer msys2 mingw-w64 (32-bit) configurations.

but now the compiler has gotten far enough to report that there's no bcrypt.h to be found.

PR updated, should be ok now.

Sorry for the delay: I can confirm that the last update builds across all my build environments.
Thanks for your help.

ping
do you accept PRs, any objections to the change?
Many projects by the way stopped working on XP because of that crypt change even though they barely use any openssl

I have no real objections to the change, it looks beneficial.  However, you will have to rebase, 'cause there's currently a conflict with other changes in crypto/rand/rand_win.c...

However, you will have to rebase, 'cause there's currently a conflict with other changes in crypto/rand/rand_win.c...

in progress.
The change that affected it Fix build with VS2008 is strange. It disables bcrypt on vs2008. These things aren't related. bcrypt ins't specific to VS version, more over bcrypt existed for two years before vs2008 came out.

@levitte I updated the change to use USE_BCRYPTGENRANDOM when enabled FIPS_MODE because RUN_ONCE cannot be used in that case.

Automated Ping: This issue is in a state where it requires action by @openssl/committers but the last update was 91 days ago

This issue is in a state where it requires action by @openssl/committers but the last update was 121 days ago

This issue is in a state where it requires action by @openssl/committers but the last update was 152 days ago

Instead of the bogus FIPS_MODE the code should simply be made a compile time option. Also the current implementation does not look right as it does not seem to me to fallback to the CryptoAPI call if the bcrypt.dll cannot be loaded. You basically lose proper RNG seeding if the bcrypt.dll cannot be loaded. Or am I wrong here?

Really, I want Mathias to comment on this, he designed that.
@mspncp wake up please.

@t8m I agree.  This fix is wrong.  The fallback should be to CryptGenRandom() not to dynamically loading the BCrypt library.  There should be no conditional on FIPS.
I'm putting a hold on it.
@mspncp your thoughts are welcome.

@mspncp your thoughts are welcome.

I'm not sleeping Bernd, I'm just overwhelmed with my regular work. But I'll try to give it a thought today.

I wonder how you guys manage to sleep at all.
Do you use some drugs for that?

The BCryptGenRandom discussion is a very old one which goes back until #1142. I reread a lot of issues and pull requests and here's my opinion. First of all, a general remark: Runtime checks and delay loading are not the right way for OpenSSL to go IMHO. It should be a compile time decision which operating systems are supported. If people prefer to do it otherwise, they are free to patch their OpenSSL library, but I wouldn't consider it a feature we should officially support.
Windows XP
Given the first remark, the statement in issue #8915 is incorrect: our way of defining USE_BCRYPTGENRANDOM does not break Windows XP support: whoever wants to use OpenSSL on Windows XP, just needs to compile with_WIN32_WINNT set to 0x0501, then USE_BCRYPTGENRANDOM won't be defined
https://github.com/openssl/openssl/blob/036ee3706352970a15300b5cd4bf0b2cb370e12a/crypto/rand/rand_win.c#L22-L25
and the old XP compatible code will be compiled (it's in the else part):
https://github.com/openssl/openssl/blob/036ee3706352970a15300b5cd4bf0b2cb370e12a/crypto/rand/rand_win.c#L79-L117
Anyway, we shouldn't care too much Windows XP support: whoever still uses this hopelessly outdated Windows version might as well use an old openssl version which still works with it. Compared to all the Windows XP security issues, the OpenSSL security issues are neglible. (https://github.com/openssl/openssl/issues/8644#issuecomment-480109330)
Windows Vista
The main problem (pointed out by @matbech, see issue #8644) is that in #8639 I lowered the version requirements for USE_BCRYPTGENRANDOM from Windows 7 to Windows Vista. This was not entirely correct, because according to MSDN, Windows Vista does have BCryptGetRandom but it does not support being called with the BCRYPT_USE_SYSTEM_PREFERRED_RNG flag.
At least for Windows Vista SP2, it seems that runtime support has been added: my experiments showed that the call succeeds on Windows Vista SP2. So it seems that there only remains a compile time issue: some SDKs dont define the BCRYPT_USE_SYSTEM_PREFERRED_RNG flag.
A quick-and-not-so-dirty fix would be to just define the constant if it's missing:
# ifdef USE_BCRYPTGENRANDOM
 #  include <bcrypt.h>
+#  ifndef BCRYPT_USE_SYSTEM_PREFERRED_RNG
+#    define BCRYPT_USE_SYSTEM_PREFERRED_RNG 2
+#  endif
...
# endif

If the runtime doesn't support the flag, the call should fail, no?
The alternative, more conservative fix would be to refrain from using BCRYPT_USE_SYSTEM_PREFERRED_RNG if it's not defined and provide a non-null hAlgorithm handle to BCryptGenRandom() instead, which needs to be obtained using BCryptOpenAlgorithmProvider(). But I don't know if it's worth implementing that.

Runtime checks and delay loading are not the right way for OpenSSL to go IMHO.

In other words: -1 from me for adding runtime checks

@mspncp What if I am creating an application that is supposed to work on XP but I want to use BCryptGenRandom if it is available? Is that something we do not want to support? If so, then we should close this PR and the related issue as WONTFIX.

Well, it's a strategical decision whether we still want to support Windows XP, which is EOL since 2014. Personally I think it's not worth the effort, but the OMC might think differently about it.
I would be interested which old Windows Versions @pps83 was talking about. Was it really Windows XP, or Windows Vista and higher?

Well, it's a strategical decision whether we still want to support Windows XP, which is EOL since 2014. Personally I think it's not worth the effort, but the OMC might think differently about it.
I would be interested which old Windows Versions @pps83 was talking about. Was it really Windows XP, or Windows Vista and higher?

In general, I see no value to use bcrypt. Old CryptGenRandom most likely calls under the hood the same code anyways. The problem with current approach is that it makes openssl explicitly linking to bcrypt and if any product uses openssl, they will not run on XP even if they think that their code is basic and should work everywhere.
In my case I wanted to replace outdated system TLS that comes with outdated windows with OpenSSL, and ended up blocking millions of kinds in 3rd world countries on old PCs from playing. They were patiently waiting for the fix and I fixed it for them. That's the use case I had.

+1 for accepting this patch

Runtime checks and delay loading are not the right way for OpenSSL to go IMHO.

In other words: -1 from me for adding runtime checks

I note that the general stance with respect to our assembly-code modules for crypto implementations is the other way around: to do the runtime detection and make use of the "most optimized" implementation that we can.

Ok. If there is such a great desire for this feature, I won’t block it. But I‘m not going to approve it.

The comparison to the assembler is a misleading one, as it costs only one instruction and doesn't change anything that's loaded, just a code-path.

@mspncp

Ok. If there is such a great desire for this feature, I won’t block it. But I‘m not going to approve it.

Oh, interesting, as you remember, you promised me to approve when I approve, right?
What it I approve this one?

This block is mine, although @mspncp also added a -1.
I'm still not seeing a compelling reason for the dynamic loading.  Falling back to CryptGenRandom would still be my preference.

I support handling this at runtime - with fallback - as this does impact real-world users. You might choose not to run WinXP - but many people do - and the changes aren't all that challenging and lead to a better result.
@paulidale your hold isn't about the concept - it is about the actual patch itself - correct?

Essentially.  Falling back and dynamically loading the BCrypt code seems wrong.
Falling back to the old API works for more platforms.

What?
@paulidale could you explain how you want the autor to solve his problem?
You are not an expert for that O/S, neither am I.  But @pps83 is an expert.

I'd prefer to see a fallback to the old legacy calls.
I'd accept that in addition to dynamically loading the BCrypt DLL but, to be honest, I don't see a point in dynamically loading it if it isn't available -- just fall back to CryptGenRandom.  I believe they both call the same underlying RNG.

@pps83 I hope you know what you have to do now. (admittedly I don't)
But I think this issue is important enough for you additional time on this matter.
In any case, please do two PR's one against 1.1.1 and one against
the "master" branch.  Especially 1.1.1 deserves more care than usually,
since 1.1.1 is already very different than the 3.0 version.

I don't think that 1.1.1 and master have diverged with respect to the RNG seed material.
Doing both separately seems overkill at this point -- do it once on master and we'll worry about the back port later.

Apparently we disagree:
I have a dim memory that the RNG code is being moved to a provider.
There was a recursion issue due to the loader needed random data
to start loading.

And, as far as I remember, it often happens that the "back-port"
is just a cherry-pick, which has no CI testing at all, and nobody
has time enough to look in detail if the cherry-picked code looks
reasonable.  Was this issue addressed ?
And if yes, how was it addressed ?

The RNG code is being moved: #11682
The problem you are remembering was fixed quite a while back.  It was more about locking that recursion.

The cherrypick script needs to build the tree.  That will sort out most issues.

The RNG code is being moved: #11682

Oh, admittedly I thought that #11682 was already merged or will soon be merged.
What are our plans when that will happen, as it will most likely give a non-trivial
merge-conflict to this PR, I think the author should know that schedule, since
his PR need to be approved / merged before that happens.

The cherrypick script needs to build the tree. That will sort out most issues.

But not the O/S with W in the name - issue, unless someone

The Windows GenRandom dilemma
I did some further investigations and experiments. Windows has several API functions to generate cryptographically secure random numbers, but it seems that none of them can be used for all Windows versions. In particular, none of the RNG functions below works from Windows XP all the way up to Windows 10 UWP apps.
CryptGenRandom
CryptGenRandom is available on Windows XP, but apparently not available for Windows UWP apps according to @matbech. This is the reason why he introduced BCryptGenRandom in #1142 in the first place. (If this is true, then this pull request breaks UWP apps again, because it unconditionally links to CryptGenRandom.)
BCryptGenRandom
BCryptGenRandom is not available on Windows XP, but for all higher Windows version, including UWP.
RtlGenRandom
The experiments below indicate that on Windows XP and Windows 10 CryptGenRandom() simpy calls RtlGenRandom(). In fact, the OSTIF Security Audit of the OpenSSL 1.1.1. RNG recommends to bypass CryptGenRandom and use RtlGenRandom() directly (see issue #7945). This has the advantage of sparing the overhead of loading a crypto provider. Many applications use it, in particular Chromium/BoringSSL. But it is not clear to me whether it would work for UWP apps. If linking to advapi32.dll is disallowed entirely for UWP apps, then this should hold for RtlGenRandom (advapi32!SystemFunction036) too, right?
@paulidale has stated repeatedly (e.g. here and here) that CryptGenRandom and BCryptGenRandom are identical under the hood. The experiments below indicate that they seem to be similar but not quite identical. @davidben and @davidmakepeace also commented on #7945, maybe they have additional information to share?
Experiments
I built two 32bit binaries from the current OpenSSL_1_1_1-stable branch using the 'Visual Studio Command Prompt (2010)', one with /D_WIN32_WINNT=0x0501 and one without.
openssl-1.1.1-xp
perl Configure VC-WIN32 shared /D_WIN32_WINNT=0x0501 /subsystem:console,5.01

(The /subsystem:console,5.01 might not even be necessary for VS 2010, but I didn't double-check).
openssl-1.1.1
perl Configure VC-WIN32 shared

Windows XP
As @kroeckx and I stated, the first application (openssl-1.1.1-xp) works on Windows XP even without this pull request. It calls CryptGenRandom, which in turn calls RtlGenRandom (ADVAPI32!SystemFunction036):
0012e964 6800d69e ADVAPI32!SystemFunction036
0012e9d0 6800d7da rsaenh!CPDeriveKey+0x618
0012e9f4 77dcb431 rsaenh!CPGenRandom+0x33

0012ea38 005c3143 ADVAPI32!CryptGenRandom+0x3d

0012ea5c 005c29c1 libcrypto_1_1!rand_pool_acquire_entropy+0x53 [f:\msp\src\openssl-1.1.1\crypto\rand\rand_win.c @ 87]
0012ea78 005c0a4d libcrypto_1_1!rand_drbg_get_entropy+0x151 [f:\msp\src\openssl-1.1.1\crypto\rand\rand_lib.c @ 198]
0012eab4 005c1bf7 libcrypto_1_1!RAND_DRBG_instantiate+0xfd [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 339]
0012eacc 005c1c65 libcrypto_1_1!do_rand_drbg_init+0x87 [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 924]
0012ead0 005f6f21 libcrypto_1_1!do_rand_drbg_init_ossl_+0x5 [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 909]
0012eadc 005c1caf libcrypto_1_1!CRYPTO_THREAD_run_once+0x41 [f:\msp\src\openssl-1.1.1\crypto\threads_win.c @ 88]
0012eae8 005c1da5 libcrypto_1_1!RAND_DRBG_get0_public+0xf [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 1116]
0012eaec 004181a9 libcrypto_1_1!drbg_bytes+0x5 [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 968]
0012eb58 7c929f68 openssl!rand_main+0x1d9 [f:\msp\src\openssl-1.1.1\apps\rand.c @ 107]
...

while the second application (openssl-1.1.1) fails because it links to BCryptGenRandom and bcrypt.dll is not found.
Windows 10
So I ran both (32bit) executables on Windows 10. It shows that the function call sequences of CryptGenRandom and BCryptGenRandom are similar (using bcryptPrimitives!SymCryptRng*), but not identical. In particular, CryptGenRandom calls RtlGenRandom (a.k.a. SystemFunction036), but BCryptGenRandom does not. (Similar results can be found in this StackOverflow post)
CryptGenRandom
00 012fe3fc 774fe7bd bcryptPrimitives!SymCryptRngAesGenerate
01 012fe40c 774fe70a bcryptPrimitives!SymCryptRngAesFips140_2Instantiate+0x1f
02 012fe428 774fe29d bcryptPrimitives!AesRNGState_init+0x32
03 012fe494 774fdf44 bcryptPrimitives!AesRNGState_select+0xd4
04 012fe4bc 75042a31 bcryptPrimitives!ProcessPrng+0x34
05 012fe4cc 71025655 CRYPTBASE!SystemFunction036+0x11
06 012fe4e0 76fb3954 rsaenh!CPGenRandom+0x25

07 012fe4fc 55d93143 CRYPTSP!CryptGenRandom+0x34

08 012fe520 55d929c1 libcrypto_1_1!rand_pool_acquire_entropy+0x53 [f:\msp\src\openssl-1.1.1\crypto\rand\rand_win.c @ 87]
09 012fe53c 55d90a4d libcrypto_1_1!rand_drbg_get_entropy+0x151 [f:\msp\src\openssl-1.1.1\crypto\rand\rand_lib.c @ 198]
0a 012fe578 55d91bf7 libcrypto_1_1!RAND_DRBG_instantiate+0xfd [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 339]
0b 012fe590 55d91c65 libcrypto_1_1!do_rand_drbg_init+0x87 [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 924]
0c 012fe594 55dc6f21 libcrypto_1_1!do_rand_drbg_init_ossl_+0x5 [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 909]
0d 012fe5a0 55d91caf libcrypto_1_1!CRYPTO_THREAD_run_once+0x41 [f:\msp\src\openssl-1.1.1\crypto\threads_win.c @ 88]
0e 012fe5ac 55d91da5 libcrypto_1_1!RAND_DRBG_get0_public+0xf [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 1116]
0f 012fe5b0 007181a9 libcrypto_1_1!drbg_bytes+0x5 [f:\msp\src\openssl-1.1.1\crypto\rand\drbg_lib.c @ 968]
10 012fe5e4 778bb0cf openssl!rand_main+0x1d9 [f:\msp\src\openssl-1.1.1\apps\rand.c @ 107]
11 012fe6b0 778bae8e ntdll!RtlpAllocateHeapInternal+0x22f
12 012fe6d4 779693ca ntdll!RtlAllocateHeap+0x3e
13 012fe6dc 779693a1 ntdll!RtlDebugAllocateHeap+0x368
14 012fe740 778bbbad ntdll!RtlDebugAllocateHeap+0x33f
15 012fe894 012fe928 ntdll!RtlpAllocateHeap+0xed

BCryptGenRandom
00 0059e074 774fe0ef bcryptPrimitives!SymCryptRngAesGenerate
01 0059e0d8 774fe284 bcryptPrimitives!AesRNGState_generate+0x15f
02 0059e144 774fdebf bcryptPrimitives!AesRNGState_select+0xbb
03 0059e168 774fde7e bcryptPrimitives!GenRandomAes+0x24
04 0059e180 76167c72 bcryptPrimitives!MSCryptGenRandom+0x4e
05 0059e1a0 7616b3d8 bcrypt!BCryptGenRandom+0xb2
06 0059e1cc 76167c20 bcrypt!BCryptGenSystemPreferredRandom+0x4f

07 0059e1e4 55deedeb bcrypt!BCryptGenRandom+0x60

08 0059e204 55dee526 libcrypto_3!rand_pool_acquire_entropy+0x2b [C:\src\openssl\crypto\rand\rand_win.c @ 71]
09 0059e21c 55dec22d libcrypto_3!rand_drbg_get_entropy+0x226 [C:\src\openssl\crypto\rand\rand_lib.c @ 184]
0a 0059e258 55decbf8 libcrypto_3!RAND_DRBG_instantiate+0x15d [C:\src\openssl\crypto\rand\drbg_lib.c @ 626]
0b 0059e274 55deb69b libcrypto_3!drbg_setup+0xb8 [C:\src\openssl\crypto\rand\drbg_lib.c @ 1194]
0c 0059e290 55db4292 libcrypto_3!drbg_ossl_ctx_new+0x5b [C:\src\openssl\crypto\rand\drbg_lib.c @ 178]
0d 0059e29c 55db6644 libcrypto_3!openssl_ctx_generic_new+0x12 [C:\src\openssl\crypto\context.c @ 189]
0e 0059e2c8 55db442a libcrypto_3!CRYPTO_alloc_ex_data+0x84 [C:\src\openssl\crypto\ex_data.c @ 424]
0f 0059e2f4 55deb8c3 libcrypto_3!openssl_ctx_get_data+0x12a [C:\src\openssl\crypto\context.c @ 265]
10 (Inline) -------- libcrypto_3!drbg_get_global+0x10 [C:\src\openssl\crypto\rand\drbg_lib.c @ 254]
11 0059e310 55dedae2 libcrypto_3!OPENSSL_CTX_get0_public_drbg+0x13 [C:\src\openssl\crypto\rand\drbg_lib.c @ 1390]
12 (Inline) -------- libcrypto_3!RAND_bytes_ex+0x51 [C:\src\openssl\crypto\rand\rand_lib.c @ 890]
13 0059e31c 00b9b4d9 libcrypto_3!RAND_bytes+0x52 [C:\src\openssl\crypto\rand\rand_lib.c @ 899]
14 0059e36c 778bb0cf openssl!rand_main+0x1e9 [C:\src\openssl\apps\rand.c @ 119]
15 0059e438 778bae8e ntdll!RtlpAllocateHeapInternal+0x22f
16 0059e45c 779693ca ntdll!RtlAllocateHeap+0x3e
17 0059e464 779693a1 ntdll!RtlDebugAllocateHeap+0x368
18 0059e4c8 778bbbad ntdll!RtlDebugAllocateHeap+0x33f
19 0059e61c 00000000 ntdll!RtlpAllocateHeap+0xed

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

This PR is in a state where it requires action by @openssl/otc but the last update was 61 days ago

This PR is in a state where it requires action by @openssl/otc but the last update was 92 days ago

This PR is in a state where it requires action by @openssl/otc but the last update was 92 days ago

This PR actually requires action by the contributor, @pps83.

This PR is in a state where it requires action by @openssl/otc but the last update was 30 days ago

The author has not responded in the last four months. Closing this pull request due to inactivity.
@pps83 if you still intend to pursue this pull request, please reopen the ticket (if GitHub allows it) or drop a short notice here.


Potential Data Loss when Using Direct Ingestion

Ref 1: https://github.com/wavefrontHQ/wavefront-sdk-java/blob/master/src/main/java/com/wavefront/sdk/direct/ingestion/WavefrontDirectIngestionClient.java#L185
private void internalFlush(LinkedBlockingQueue<String> buffer, String format)
      throws IOException {
    ...
    ...
    try (InputStream is = batchToStream(batch)) {
      // Exception in the below line will not add the data back to buffer
      int statusCode = directService.report(format, is);
      if (400 <= statusCode && statusCode <= 599) {
        logger.log(Level.WARNING, "Error reporting points, respStatus=" + statusCode);
        try {
          buffer.addAll(batch);
        } catch (Exception ex) {
          // unlike offer(), addAll adds partially and throws an exception if buffer full
          logger.log(Level.WARNING, "Buffer full, dropping attempted points");
        }
      }
    } catch (IOException ex) {
      failures.incrementAndGet();
      throw ex;
    }
  }

If an exception is thrown by directService.report(), then we don't know if the data write failed or we failed to get the http return code from the URL Output stream.
Ref 2: https://github.com/wavefrontHQ/wavefront-sdk-java/blob/master/src/main/java/com/wavefront/sdk/direct/ingestion/DataIngesterService.java#L54
try (GZIPOutputStream gzipOS = new GZIPOutputStream(urlConn.getOutputStream())) {
        byte[] buffer = new byte[4096];
        while (stream.available() > 0) {
          gzipOS.write(buffer, 0, stream.read(buffer));
        }
        gzipOS.flush();
      }
      statusCode = urlConn.getResponseCode();
      readAndClose(urlConn.getInputStream());
    } catch (IOException ex) {
      if (urlConn != null) {
        statusCode = urlConn.getResponseCode();
        readAndClose(urlConn.getErrorStream());
      }
    }

I was testing the direct Ingestion and I noticed that intermittently a java.net.SocketTimeoutException: Read timed out was thrown with this stack:
java.net.SocketTimeoutException: Read timed out
	at com.wavefront.sdk.direct.ingestion.DataIngesterService.report(DataIngesterService.java:58)
	at com.wavefront.sdk.direct.ingestion.WavefrontDirectIngestionClient.internalFlush(WavefrontDirectIngestionClient.java:185)
	at com.wavefront.sdk.direct.ingestion.WavefrontDirectIngestionClient.flush(WavefrontDirectIngestionClient.java:171)
	at com.wavefront.sdk.direct.ingestion.WavefrontDirectIngestionClient.run(WavefrontDirectIngestionClient.java:163)

The exception here occurs when we try to get the responseCode. But the method DataIngesterService.report can also throw an exception when doing a gzipOS.write or gzipOS.flush and subsequently we might get an exception inside the catch block. In this case, we cascade the exception to WavefrontDirectIngestionClient.report() and we never add the data back to buffer for retrying.

@basilisk487 or @MikeMcMahon is this obsolete?

@panghy @MikeMcMahon Looks like this bug is still valid, ReportingService.send still can throw an IOException if it happens in the catch (IOException) block.

okay, assigned to you two then

the outermost exception handling simply drops the points; we can call to re-queue the points there so that any exception during the processing (regardless of source) will cause points to be resent later, since it would become impossible to know if the points were actually received if we cannot obtain the status code.

      } catch (IOException ex) {
        requeue(buffer, items, dropped, entityType, bufferFullMessageType);
        reportErrors.inc();
        for (int j = i + 1; j < batch.size(); j++) {
          dropped.inc(batch.get(j).size());
        }
        throw ex;
      }

vs. just dropping the points completely.

hrm actually this looks like we can drop an entire batch not just the points being sent, we should adjust the service code to return a -1 and safely attempt to close the stream if we detect an error.   We could overflow the buffer and begin dropping points that way, but that might be preferred here (since we'd eventually get those points vs. losing them completely when the network comes back)

fix is in master


Trace window (and the Subs window) no longer scrolls content

No matter how many items are in the list, it does not scroll.
A scrollbar is shown but it continues beyond the bottom of the page.
Using the element inspector, I see that the height of the window is basically the height of content itself, probably minus enough pixels to force the scrollbar to be displayed (that spans from top to bottom).
Addition: Same for the Subs window

Fixed thanks to @Gregg8 and merged into master.
Morale of the storey - use flexbox all the way down and don't use strange scrollable-content custom components for things that should be simple...