riscv-non-isa / riscv-external-debug-security

The RISC-V External Debug Security Specification
https://jira.riscv.org/browse/RVG-136
Creative Commons Attribution 4.0 International
15 stars 2 forks source link

Behaviour on an illegal write value attempt (M-mode) to DCSR.PRV (debug access privilege) #31

Closed sudheendraks closed 4 months ago

sudheendraks commented 4 months ago
gokhankaplayan commented 4 months ago

I agree that reporting security fault error can be made optional. I am not aware any use case of reporting security fault error. For example, RoT takes actions depending on security fault error for malicious attacks. Is there an use case for reporting security fault error? @joxie I know that reporting security fault error requirement will bring extra design complexity especially for Execution Based implementations (See RISC-V Debug Spec Appendix-A). Therefore, making the reporting as optional will be beneficial for RISC-V.

joxie commented 4 months ago

Agreed on the extra complexity here, sure we can make it optional to allow as many implementations to adopt ext. debug. sec spec as possible. This is not a must have feature, raise alarm on illegal privilege escalation attempt is more of a best practice from security perspective because this looks like an attack event, explicit cmderr forces debugger to log the potential attack event for further analysis.

Maybe we need a new (optional) extension on error response for misprograms vs WARL.

gokhankaplayan commented 4 months ago

Although security fault error is pain for some implementations, it might be a case for other security fault errors for other implementations. So I proposed to make all error reporting optional including security fault error (cmderr 6), bus security fault error (sberror 6), and allsecfault/anysecfault in dmstatus. I think we do not need extra extension for optional features. E.g. Debug Extensions (Sdext, Sdtrig, DM non-ISA) has many optional features,

AoteJin commented 4 months ago

It is okay to make secfault (cmderr 6) optional for misconfiguring the PRV/V in dcsr, since the failure could be detected by reading after writing. However, it is not feasible for System Bus Access, because we need an error to indicate that the value in sbdata* is invalid. Regarding reset/resethaltreq/keepalive, the error indicator in allsecfault/anysecfault simplify the procedure to determine whether the operation succeed. Otherwise the user can only check the result of operation indirectly. E.g. the user has to check the success of resethaltreq by issuing a reset first.

gokhankaplayan commented 4 months ago

@AoteJin thank you for your explanation. It makes sense to me. So, the consensus is to make only secfault (cmderr 6) as optional.