veracrypt / VeraCrypt

Disk encryption with strong security based on TrueCrypt
https://www.veracrypt.fr
Other
6.89k stars 947 forks source link

RDRAND not detected on AMD Ryzen 3900X #530

Closed jbaker6953 closed 5 years ago

jbaker6953 commented 5 years ago

Running Veracrypt 1.24 on Ryzen 3900X has "Use CPU hardware random number generator..." option grayed out.

idrassi commented 5 years ago

This is strange because the code that checks if CPU supported RDRAND instruction is standard and it works on all CPUs.

idrassi commented 5 years ago

As a quick test, can you please use an application that displays CPU features in order to see if they detected RDRAND? For example, you can install FileZilla and use it’s About dialog which displays at the button CPU features (a screenshot will be helpful in this case).

jbaker6953 commented 5 years ago

I know it supports RDRAND because I modified Intel's reference DRNG library to run on it. Intel's reference implementation checks for an Intel processor as a part of checking whether the processor is RDRAND-capable. When I get home I will load something that detects RDRAND, though.

ChubbyAnt commented 5 years ago

Running Veracrypt 1.24 on Ryzen 3900X has "Use CPU hardware random number generator..." option grayed out.

I can confirm this also greyed out with an AMD Ryzen 3700u.

jbaker6953 commented 5 years ago

FileZilla: https://i.imgur.com/l1NeJFd.png

CPU-Z: https://i.imgur.com/eZLqa1C.png

jbaker6953 commented 5 years ago

I don't get it.

g_hasRDRAND = (cpuid1[2] & (1 << 30)) != 0

should work. AMD developer docs say rdrand feature flag is at ECX[30]. Is

else if (IsAMD(cpuid) || IsHygon(cpuid))

failing?

idrassi commented 5 years ago

This is indeed a strange situation. In order to understand more, I have written a small command line tool that displays various CPU information in order to check if your CPU is returning the expected value as with other AMD CPUs. You get a binary of this tool at https://github.com/idrassi/cpu_info/releases/download/cpu_info_1.0/cpu_info-1.0-bin.zip. It is signed using the same code signing certificates as VeraCrypt. The source code of this tool is at https://github.com/idrassi/cpu_info.

Can you please share the output of this tool?

Thank you.

jbaker6953 commented 5 years ago

CPU Information - By Mounir IDRASSI (mounir_at_idrix.fr)

Manufacturer ID: AuthenticAMD Processor Brand: AMD Ryzen 9 3900X 12-Core Processor

Features: ABM ADX AES AVX AVX2 BMI1 BMI2 CLFSH CMPXCHG16B CX8 F16C FMA FSGSBASE FXSR LAHF MMX MMXEXT MONITOR MOVBE MSR OSXSAVE PCLMULQDQ POPCNT RDRAND RDSEED SEP SHA SSE SSE2 SSE3 SSE4.1 SSE4.2 SSE4a SSSE3 XSAVE

Press a key to exit..

It appears the issue is somewhere other than CPU detection.

idrassi commented 5 years ago

Something is indeed not right...to go further, I have written another command line tool but this time it uses the exact same code as in VeraCrypt for CPU features détection and RDRAND detection. It displays many low level values and it will help understand why RDRAND is not detected. I checked all possible execution paths and the UI relies solely on RDRAND detection so the issue is clearly clearly coming from there.

You can get the binary form the following link: vc_cpu_detect-bin.zip

As usual, the binaires are signed using same code signing certificates as VeraCrypt.

Can you please share again the output of this tool and also confirm that you have the same output between 32-bit version and 64-bit version?

Thank you.

jbaker6953 commented 5 years ago

I'm working backwards from the code that enables/disables the rdrand option. If I understand it correctly, the only way for the option to be disabled is for IsCpuRngSupported() to be false:

VeraCrypt-master/src/Mount/Mount.c


if (IsCpuRngSupported())
    {
        CheckDlgButton (hwndDlg, IDC_ENABLE_CPU_RNG, (driverConfig & VC_DRIVER_CONFIG_ENABLE_CPU_RNG) ? BST_CHECKED : BST_UNCHECKED);
    }
    else
    {
        CheckDlgButton (hwndDlg, IDC_ENABLE_CPU_RNG,  BST_UNCHECKED);
        EnableWindow (GetDlgItem (hwndDlg, IDC_ENABLE_CPU_RNG), FALSE);
    }

It looks like IsCpuRngSupported comes from VeraCrypt-master/src/Common/Crypto.c:

#if !defined (TC_WINDOWS_BOOT) && !defined (_UEFI)

static BOOL CpuRngDisabled = TRUE;
static BOOL RamEncryptionEnabled = FALSE;

BOOL IsCpuRngSupported ()
{
    if (HasRDSEED() || HasRDRAND())
        return TRUE;
    else
        return FALSE;
}

void EnableCpuRng (BOOL enable)
{
    CpuRngDisabled = !enable;
}

BOOL IsCpuRngEnabled ()
{
    return !CpuRngDisabled;
}

My suspicions lie in Crypto.c. I just don't know what could be happening on AMD that isn't happening on Intel. It may be some other difference in machines that isn't the CPU, like UEFI or Windows 10 vs 7 for example.

jbaker6953 commented 5 years ago

I am at work, so I can only offer the output from an Intel i7 Windows 7 machine as a reference until I get home after 00:00 CET:

32-bit
-------------------------------------------------------------------------------
CpuId(0) : highest basic function understood is 0x0000000D

CpuId(0) = 0x0000000D, 0x756E6547, 0x6C65746E, 0x49656E69
CpuId(1) = 0x000306C3, 0x04100800, 0x7FFAFBFF, 0xBFEBFBFF

CpuId(0) = 0x0000000D, 0x756E6547, 0x6C65746E, 0x49656E69

CPU brand = Intel

Features = SSE2 ISSE MMX SSE41 SSE42 AVX AVX2 BMI2 SSSE3 AESNI CLMUL RDRAND

IsCpuRngSupported = TRUE

And...

64-bit
-------------------------------------------------------------------------------
CpuId(0) = 0x0000000D, 0x756E6547, 0x6C65746E, 0x49656E69
CpuId(1) = 0x000306C3, 0x01100800, 0x7FFAFBFF, 0xBFEBFBFF

CpuId(0) = 0x0000000D, 0x756E6547, 0x6C65746E, 0x49656E69

CPU brand = Intel

Features = SSE2 ISSE MMX SSE41 SSE42 AVX AVX2 BMI2 SSSE3 AESNI CLMUL RDRAND

IsCpuRngSupported = TRUE
idrassi commented 5 years ago

Thank you for the test and the analysis. I arrived to the same conclusion as you and that’s why this tool outputs the value of IsCpuRngSupported.

Anyway, waiting for your feedback once you can run the tool on the Ryzen.

ChubbyAnt commented 5 years ago

@idrassi @jbaker6953

Output from cpu_info-1.0-bin.zip on an AMD Ryzen 3700u:

>cpu_info-x64
CPU Information - By Mounir IDRASSI (mounir@idrix.fr)
----------------------------------------------------------------

Manufacturer ID: AuthenticAMD
Processor Brand: AMD Ryzen 7 3700U with Radeon Vega Mobile Gfx

Features: ABM ADX AES AVX AVX2 BMI1 BMI2 CLFSH CMPXCHG16B CX8 F16C FMA FSGSBASE FXSR LAHF MMX MMXEXT MONITOR MOVBE MSR OSXSAVE PCLMULQDQ POPCNT RDRAND RDSEED SEP SHA SSE SSE2 SSE3 SSE4.1 SSE4.2 SSE4a SSSE3 XSAVE

Output from vc_cpu_detect-bin.zip on an AMD Ryzen 3700u:

32 bit:

>vc_cpu_detect
CpuId(0) : highest basic function understood is 0x0000000D

CpuId(0) = 0x0000000D, 0x68747541, 0x444D4163, 0x69746E65
CpuId(1) = 0x00810F81, 0x02080800, 0x7ED8320B, 0x178BFBFF

CpuId(0) = 0x0000000D, 0x68747541, 0x444D4163, 0x69746E65

CPU brand = Unknown

Features = SSE2 ISSE MMX SSE41 SSE42 AVX SSSE3 AESNI CLMUL

IsCpuRngSupported = FALSE

64 bit:

>vc_cpu_detect-x64
CpuId(0) = 0x0000000D, 0x68747541, 0x444D4163, 0x69746E65
CpuId(1) = 0x00810F81, 0x06080800, 0x7ED8320B, 0x178BFBFF

CpuId(0) = 0x0000000D, 0x68747541, 0x444D4163, 0x69746E65

CPU brand = Unknown

Features = SSE2 ISSE MMX SSE41 SSE42 AVX SSSE3 AESNI CLMUL

IsCpuRngSupported = FALSE

Thanks for looking into this!

idrassi commented 5 years ago

@ChubbyAnt : Thank you for sharing the output. I was able to find the cause of this problem: it was a bug in the function IsAMD from cpu.c where the expected values for ECX and EDX were mistakenly exchanged.

This bug has always existed but it was never noticed because it affected only the detection of AVX2 which was never used for AMD CPU because of its latency. But with the introduction of RDRAND, this bug has manifested itself.

I have pushed a commit to fix this: https://github.com/veracrypt/VeraCrypt/commit/3b5d4771a0af5b4fc89ec77f43826b9ae2544949

It looks like I will have to publish a Hotfix2 for 1.24 version...sorry for this.

jbaker6953 commented 5 years ago

It's not a big deal preventing anybody from doing work or compromising security. Don't feel compelled to release a hotfix over this minor issue.

ChubbyAnt commented 5 years ago

Have you seen this about the AMD Ryzen RDRAND bug:

https://arstechnica.com/gadgets/2019/10/how-a-months-old-amd-microcode-bug-destroyed-my-weekend/

jbaker6953 commented 5 years ago

The "easy" workaround is to call rdrand a second time if it returns 0xFFFFFFFF and use another entropy source if the second call also returns 0xFFFFFFFF.

I'm a novice, but I'd start by trying something like (pseudo-code):

int RDRAND_getBytes(unsigned char* buf, size_t bufLen)
{
    if (!buf || !HasRDRAND())
        return 0;

    if (bufLen)
        MASM_RDRAND_GenerateBlock(buf, bufLen);

        if (buf == 0xFFFFFFFF)
        {
            MASM_RDRAND_GenerateBlock(buf, bufLen);

            if (buf == 0xFFFFFFFF)
                return 0;
        }

    return 1;
}
idrassi commented 5 years ago

@jbaker6953 and @ChubbyAnt : Thank you both for sharing this information that I was not aware of. So this VeraCrypt bug is not bad after all! I have modified my previous tool to include calls to RDRAND and do tests on its output.

Since I don’t have a Ryzen CPU at hand, is it possible for you to run it and share its output? It would be helpful if one of you has the buggy Ryzen CPU so I can see how it behaves.

Here is the new binary: vc_cpu_detect-1.1-bin.zip

Thank you again for your help.

ChubbyAnt commented 5 years ago

@idrassi @jbaker6953

The Ryzen 3700u is not affected by the RDRAND bug.

Output from vc_cpu_detect-1.1-bin.zip on an AMD Ryzen 3700u:

32 Bit:

>vc_cpu_detect
CpuId(0) = 0x0000000D, 0x68747541, 0x444D4163, 0x69746E65
CpuId(1) = 0x00810F81, 0x02080800, 0x7ED8320B, 0x178BFBFF

CPU brand = AMD

Features = SSE2 ISSE MMX SSE41 SSE42 AVX AVX2 BMI2 SSSE3 AESNI CLMUL RDRAND RDSEED

IsCpuRngSupported = TRUE

testing RDRAND:
  - Generating 32 bytes...done!
  - Bytes = 0B EA B9 F6 ED 23 D1 C5 54 87 0F 2F 84 6B B7 51 76 99 57 6A CD B9 63 FB BE 0C 8A 46 1D 47 51 BB

testing RDSEED:
  - Generating 32 bytes...done!
  - Bytes = 52 ED 2D 5B 69 AD 26 A3 87 BA FA E4 EF 12 A1 03 A1 15 1E BC 3C 46 CF D5 B5 05 EF 21 BB 27 26 4F

64 Bit:

>vc_cpu_detect-x64
CpuId(0) = 0x0000000D, 0x68747541, 0x444D4163, 0x69746E65
CpuId(1) = 0x00810F81, 0x05080800, 0x7ED8320B, 0x178BFBFF

CPU brand = AMD

Features = SSE2 ISSE MMX SSE41 SSE42 AVX AVX2 BMI2 SSSE3 AESNI CLMUL RDRAND RDSEED

IsCpuRngSupported = TRUE

testing RDRAND:
  - Generating 32 bytes...done!
  - Bytes = E4 2D 49 4B 5B B0 B4 53 D5 9F 58 F1 A2 9A 8D 65 F3 8E E3 CC 26 91 27 8C 1B 8C 85 B6 46 37 31 8B

testing RDSEED:
  - Generating 32 bytes...done!
  - Bytes = A8 8A 7A 93 AA 0B 2C 19 A6 CE 47 7B A2 AC B3 06 1E D3 FE 71 12 B7 1C 6F C1 C4 D1 1A DE EC 91 A3

Looking good, except still untested on a RDRAND crippled AMD Ryzen.

jbaker6953 commented 5 years ago

I think that bug was patched by a microcode update. The only affected users will be those with an outdated bios. It's probably good to check for the existence of the bug because even in the rare edge case that person is going to have a random seed that is not random at all.

As far as checking output, I think in the interest of tidiness that if rdrand returns 0xffffffff and the subsequent check does not return 0xffffffff the original 0xfffffffff should be used to avoid excluding 0xffffffff from the universe of possible random 32-bit values.

ChubbyAnt commented 5 years ago

I think that bug was patched by a microcode update. The only affected users will be those with an outdated bios. It's probably good to check for the existence of the bug because even in the rare edge case that person is going to have a random seed that is not random at all.

I think the issue is that many board manufacturers will never update the BIOS to include the fix because that's just how it is, and only a fraction of users ever update their BIOS, ever.

As far as checking output, I think in the interest of tidiness that if rdrand returns 0xffffffff and the subsequent check does not return 0xffffffff the original 0xfffffffff should be used to avoid excluding 0xffffffff from the universe of possible random 32-bit values.

This appears to be the correct way to check for and control the bug.

idrassi commented 5 years ago

@ChubbyAnt : Thank you for the test results and the feedback. @jbaker6953 : your proposal is good but it is enough to do it at startup during initialization : when RDRAND is detected, we generate 4 32-bit words and if they are all set to 0xFFFFFFFF then we consider RDRAND to be broken and we disable it for the rest of execution. This should be enough to catch the buggy Ryzen while allowing random values of 0xFFFFFFFF to appear. In this context, it will be helpful if my test program can be run in such buggy Ryzen CPU in order to confirm the behavior.

jbaker6953 commented 5 years ago

Unfortunately for testing I have too recent of a BIOS.

CpuId(0) = 0x00000010, 0x68747541, 0x444D4163, 0x69746E65
CpuId(1) = 0x00870F10, 0x08180800, 0x7ED8320B, 0x178BFBFF

CPU brand = AMD

Features = SSE2 ISSE MMX SSE41 SSE42 AVX AVX2 BMI2 SSSE3 AESNI CLMUL RDRAND RDSEED

IsCpuRngSupported = TRUE

testing RDRAND:
  - Generating 32 bytes...done!
  - Bytes = 3B 3D 35 25 EB 1E D7 34 5D 73 90 85 7B 03 75 5A A9 8F E9 10 FE 0A 6D FD 19 F8 F8 4C 51 9A ED 08

testing RDSEED:
  - Generating 32 bytes...done!
  - Bytes = 54 F8 CA 57 BD ED 16 D4 1C 2B A3 28 2E 82 28 7D B2 21 18 A4 09 E6 75 1C C0 38 18 2D 35 6C FF CA
idrassi commented 5 years ago

Thank you @jbaker6953 for the test and thank you all for your help in fixing this issue.

I have pushed a modification to disable RDRAND and RDSEED support if 4 consecutive 0xFFFFFFFF values are retuned: https://github.com/veracrypt/VeraCrypt/commit/5ecff99edc9c342987ef59156c2358a4c24ce9b5 and https://github.com/veracrypt/VeraCrypt/commit/3565cb1afe4f917422853e42d7dbe05526ab750b