Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insecure RNG Based on Spoofed CPUID #111

Closed
zx2c4 opened this issue Jun 1, 2017 · 2 comments
Closed

Insecure RNG Based on Spoofed CPUID #111

zx2c4 opened this issue Jun 1, 2017 · 2 comments

Comments

@zx2c4
Copy link

zx2c4 commented Jun 1, 2017

In this file, as of writing, you appear to determine whether or not rdrand is supported like this:

static int rdrand_cpuid()
{
    int info[4] = {-1, -1, -1, -1};

    /* Are we on an Intel processor? */

    __cpuid(info, 0);

    if (memcmp(&info[1], "Genu", 4) != 0 ||
        memcmp(&info[3], "ineI", 4) != 0 ||
        memcmp(&info[2], "ntel", 4) != 0 ) {
            return 0;
    }

   /* Do we have RDRAND? */

    __cpuid(info, /*feature bits*/1);

    int ecx = info[2];
    if ((ecx & RDRAND_MASK) == RDRAND_MASK)
         return 1;
    else
         return 0;
}

...

g_is_rdrand_supported = rdrand_cpuid();

However, CPUID is not allowed to be trusted inside SGX, per this documentation:

Because calls to the CPUID instruction must take place in untrusted memory, the results of CPUID cannot be trusted! This warning applies whether you run CPUID yourself or rely on the SGX functions to do it for you. The Intel SGX SDK offers this advice: “Code should verify the results and perform a threat evaluation to determine the impact on trusted code if the results were spoofed.”

It goes onto suggest that in a secure RNG design,

RDRAND is detected, but a negative result is spoofed. This will result in an error at runtime, causing the program to exit gracefully since a required feature is not detected.

However, the code in question does not raise a runtime error, but rather quite fantastically makes this mistake:

    if(!g_is_rdrand_supported){
        uint32_t i;
        for(i=0;i<(uint32_t)size;++i){
            buf[i]=(uint8_t)rand();
        }
    }else{
        int rd_ret =rdrand_get_bytes((uint32_t)size, buf);
        if(rd_ret != RDRAND_SUCCESS){
            rd_ret = rdrand_get_bytes((uint32_t)size, buf);
            if(rd_ret != RDRAND_SUCCESS){
                return SGX_ERROR_UNEXPECTED;
            }
        }
    }
    return SGX_SUCCESS;

It appears to fallback to stdlib's rand() if the untrusted cpuid call says rdrand is not supported, which means that sgx_read_rand will fail to deliver good randomness.

It looks like other Intel code, such as the recent work porting OpenSSL to SGX, relies on that function returning cryptographically secure random numbers:
https://github.com/01org/intel-sgx-ssl/blob/master/sgx/libsgx_tsgxssl/trand.cpp#L51
https://github.com/01org/intel-sgx-ssl/blob/master/openssl_source/rand_unix.c#L44
https://github.com/01org/intel-sgx-ssl/blob/master/openssl_source/md_rand.c#L61

@haitaohuang
Copy link
Contributor

Thanks for the feedback. I don't think we actually use that implementation for trusted side(inside enclave). The trusted implementation is at https://github.com/01org/linux-sgx/blob/master/sdk/trts/trts.cpp#L257.

I think the implementation you referred to is only compiled into untrusted side of AESM and/or some simulation libs

@zx2c4
Copy link
Author

zx2c4 commented Jun 1, 2017

Hi @haitaohuang - That's good to hear. In that case, I'll close the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants