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

The private key may be leaked to the attacker #33

Open
jmp0x7c00 opened this issue Mar 9, 2022 · 5 comments
Open

The private key may be leaked to the attacker #33

jmp0x7c00 opened this issue Mar 9, 2022 · 5 comments

Comments

@jmp0x7c00
Copy link

jmp0x7c00 commented Mar 9, 2022

Hello, sir

I found there maybe a security issue here and need your confirm.

related source code:

EVP_PKEY *SSL_get_privatekey(SSL *s) {
   if (global_eid == 0) {
   	initialize_library();
   }

   log_enter_ecall(__func__);
	sgx_status_t ret = SGX_ERROR_UNEXPECTED;
	ret = ecall_SSL_get_privatekey(global_eid, &my_evp_pkey, s); 
	if (ret != SGX_SUCCESS) {
		print_error_message(ret, __func__);
		return NULL;
	}
	log_exit_ecall(__func__);

	return &my_evp_pkey;
}

In ecall_SSL_get_privatekey, the private key is copied to the memory area pointed to by pkey, but since pkey is user_check, and points to untrusted memory outside the enclave, so an attacker can monitor its content to obtain the private key.

 /* Fix this function so that it takes an optional type parameter */
+void
+ecall_SSL_get_privatekey(EVP_PKEY* pkey, SSL *s) {
+#ifdef COMPILE_WITH_INTEL_SGX
+	const SSL* out_s = s;
+
+	hashmap* m = get_ssl_hardening();
+	SSL* in_s = (SSL*) hashmapGet(m, (unsigned long)out_s);
+
+	EVP_PKEY* enclave_pkey = SSL_get_privatekey(in_s);
+	memcpy(pkey, enclave_pkey, sizeof(*pkey)); //   An attacker can spy on the buffer pointed to by pkey
+#else
+	printf("Cannot call %s without SGX!!!\n", __func__);
+#endif
+}
+
@plaublin
Copy link
Collaborator

plaublin commented Mar 10, 2022

Hi. I guess the big question is why would the untrusted code call SSL_get_privatekey() in the first place. As a research prototype we needed this call to be able to easily inspect and modify protected data for running our various experiments.

@jmp0x7c00
Copy link
Author

jmp0x7c00 commented Mar 10, 2022

Hi,dear sir

Thanks for your quick reply.

So if we use TaLos in production,there might be a security issue if the attacker try to obtain the private key by directly invoking SSL_get_privatekey()

so can we just directly remove this ECALL before running TaLos in production?

@plaublin
Copy link
Collaborator

Yes, you can remove it. You should probably also remove the printf ecall. It's useful for debugging, but it could potentially expose sensitive information to the untrusted world.

@f0rki
Copy link

f0rki commented May 19, 2022

so can we just directly remove this ECALL before running TaLos in production?

@jmp0x7c00 I would advise you not to use this project in production. A couple of years ago we have developed 3 different proof-of-concept exploits that target the talos enclave from a malicious host process. I believe the issues we exploited were not fixed. Due to the prevalence of user_checked pointers there are probably even more issues. See issue #18 for a discussion on this.

@plaublin Don't you think it would be good to put a notice somewhere that this code is not fit for production?

@hxuhack
Copy link

hxuhack commented Jan 30, 2023

Similar to this issue reported by @jmp0x7c00, ecall_SSL_CTX_use_PrivateKey() copies the private key to the untrusted space through the call chain: ecall_SSL_CTX_use_PrivateKey()->SSL_CTX_use_PrivateKey()->ssl_set_pkey().

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

4 participants