jovanbulck / sgx-step

A practical attack framework for precise enclave execution control
GNU General Public License v3.0
433 stars 82 forks source link

aep_cb_func not triggered and abnormal value of context RIP in the last round when running bench #82

Closed bainianbuhe closed 3 weeks ago

bainianbuhe commented 1 month ago

These are some results of running bench with NUM=100 STRLEN=1 make parse, I copied and pasted some codes from memcmp to handle SIGTRAP to make it not terminated by SIGTRAP, not sure if that's the right way. register_signal_handler(SIGSEGV); case SIGTRAP: /* ensure RFLAGS.TF is clear to disable debug single-stepping */ ucontext_t *uc = (ucontext_t *)ctx; uc->uc_mcontext.gregs[REG_EFL] &= ~0x100; 2024-07-24 19-09-47 的屏幕截图 Although the result is approximately right,(11+1)*100=1200, I notice that the value of RIP in ctx is not what I expected in the last SIGTRAP handling process(I think it should be the address of AEP, 0x5557c1c28551 in this case), and after the handling process, aep_cb_func is not triggered like the former SIGTRAPS By the way,even in the former SIGTRAPS,the context RIP is 1 greater than AEP, however, in the first and only SIGSEGV, the RIP is just equal to AEP,not sure if that's normal. 2024-07-24 19-15-24 的屏幕截图 2024-07-24 19-21-43 的屏幕截图 not sure why, thanks in advance

bainianbuhe commented 1 month ago

After checking the assembly code, I guess in the last SIGTRAP, the process has left the Enclave using enclu, so it's normal that the aep_cb_func is not triggered 2024-07-25 10-23-47 的屏幕截图

jovanbulck commented 4 weeks ago

Hi bainianbuhe,

Not sure what's exactly your setup here: are you trying to get app/bench working with SIGTRAP instead of timer interrupts? Please note that SIGTRAP support was added to app/memcp to ease rapid prototyping only, i.e., it uses x86 #DB traps which will not work on production enclaves. The SIGTRAP is only intended to more easily test your attack logic, which should then also work with "proper" timer-based single-stepping as a drop-in replacement.

Atm the app/bench is not configured to work with SIGTRAP, so not sure what exactly you added and what behavior you expect. It's true that the last step after enclu[EEXIT] will be out of the enclave, as you observed above.

Let me know if this can be closed?

bainianbuhe commented 4 weeks ago

Hi bainianbuhe,

Not sure what's exactly your setup here: are you trying to get app/bench working with SIGTRAP instead of timer interrupts? Please note that SIGTRAP support was added to app/memcp to ease rapid prototyping only, i.e., it uses x86 #DB traps which will not work on production enclaves. The SIGTRAP is only intended to more easily test your attack logic, which should then also work with "proper" timer-based single-stepping as a drop-in replacement.

Atm the app/bench is not configured to work with SIGTRAP, so not sure what exactly you added and what behavior you expect. It's true that the last step after enclu[EEXIT] will be out of the enclave, as you observed above.

Let me know if this can be closed?

Thank you for your reply! It seems I misunderstood how the bench works. Actually if I simply try the NUM=200 STRLEN=1 make parse command under the unmodified bench folder the program will be terminated by SIGTRAP, which leads me to think I should catch and handle the SIGTRAP signal. So what else should I do to make the bench run in the right way? Is SIGTRAP expected to appear in the unmodified bench program? 2024-08-10 13-21-38 的屏幕截图

jovanbulck commented 3 weeks ago

hm, normally app/bench should not raise SIGTRAP

I reviewed the code and it seems like you indeed ran into a bug where the aep_trampoline.S assembly code incorrectly reads out the sgx_step_do_trap variable and always enables the traps:

https://github.com/jovanbulck/sgx-step/blob/1856d6bbfe61c45b04ba20aeb41dd1dee29c9c6a/libsgxstep/aep_trampoline.S#L83-L86

of course misses a mov instruction to actually read out the memory location -- my bad! :facepalm:

Thank you for reporting the bug, I will fix this shortly on the master branch

jovanbulck commented 3 weeks ago

@bainianbuhe please pull the latest master with the commit above and let me know if this fixes it for you (on a clean /app/bench without modifications)? Feel free to re-open this issues if further discussion is needed!