search

google / OpenSK

OpenSK is an open-source implementation for security keys written in Rust that supports both FIDO U2F and FIDO2 standards.
Apache License 2.0
2.98k stars 289 forks source link

Can't enable JTAG lockdown #681

Open gtf35 opened 5 months ago

gtf35 commented 5 months ago

Expected Behavior

configure.py --lock-device info: Device is now locked down!

Actual Behavior

configure.py --certificate=crypto_data/opensk_cert.pem --private-key=crypto_data/opensk.key --lock-device info: Certificate is valid. info: Programming OpenSK device info: Please touch the device to confirm... error: Failed to configure OpenSK (lockdown conditions not met or hardware error).

Steps to Reproduce the Problem

  1. setup.sh
  2. deploy.py --board=nrf52840_dongle_opensk --opensk --programmer=pyocd
  3. configure.py --certificate=crypto_data/opensk_cert.pem --private-key=crypto_data/opensk.key
  4. configure.py --certificate=crypto_data/opensk_cert.pem --private-key=crypto_data/opensk.key --lock-device

Specifications

kaczmarczyck commented 5 months ago

This issue is indeed not well documented. We heard that on the nrf52840 version we worked with, the lock down can be circumvented. Therefore, I made the command error instead of giving a false sense of security, see #620:

        #[cfg(not(feature = "std"))]
        {
            matches!(
                crp::set_protection(crp::ProtectionLevel::FullyLocked),
                Ok(())
                    | Err(TockError::Command(CommandError {
                        return_code: EALREADY,
                        ..
                    }))
            )
        }
        #[cfg(feature = "std")]
        {
            true
        }

became a simple false.

We could re-enable the old behavior so users with a new version of the hardware can benefit from it.

@jmichelp Opinions?

jmichelp commented 5 months ago

That would need more tweaking because to fix the bypass, Nordic made changes to the way the JTAG lockdown works. So we would need to add code to support the newer versions of the chip as well as issuing a warning for old revisions that the lockdown could be circumvented. And that also means we need to ensure we have such a revision of the chip to do some testing :)

gtf35 commented 5 months ago

I think you did the right thing.

If it cannot be truly locked, it is correct to report an error.

We could re-enable the old behavior so users with a new version of the hardware can benefit from it.

Sounds good, looking forward to it

kaczmarczyck commented 5 months ago

This is not going to happen soon. I'll keep this issue open, steps to fix this include: