rhboot / shim

UEFI shim loader
Other
816 stars 284 forks source link

Verification by vendor cert -- dead code? #616

Open polarina opened 9 months ago

polarina commented 9 months ago

https://github.com/rhboot/shim/blob/7ba7440c49d32f911fb9e1c213307947a777085d/shim.c#L542

How is this code-path reachable?

The vendor cert is addended into MokListRT, thus when loading a vendor signed executable, the certificate check succeeds in check_allowlist in shim.c, measuring the matching certificate under the name MokListRT into PCR7. The success condition in the #if defined(VENDOR_CERT_FILE) section in verify_one_signature never executes for valid executables.

Under PCR7, I am only able to get a measurement named under MokListRT in the eventlog, never Shim.

polarina commented 9 months ago

Is it possible this is a regression introduced by commit 092c2b2bbed950727e41cf450b61c794881c33e7? Previously MokList was referenced, not MokListRT, which generally wouldn't contain copies of the vendor_cert certificates.

The https://github.com/rhboot/shim/blob/main/README.tpm file states that these measurements are done under the MokList and Shim names, which is not the reality today.

dennis-tseng99 commented 7 months ago

In the world, no every case result is what we expected. For example, when check_allowlist() returns EFU_NOT_FOUND or other errors just because the NVRAM DB or NVRAM MokListRT memory hardware is broken right at when get_variable() is running, the lines of #if defined(VENDOR_CERT_FILE) will be executed.

verify_one_signature()    
    check_allowlist(): 
        check_db_hash(L"db",...)
            get_variable()
            check_db_hash_in_ram()
        check_db_cert(L"db",...)
            get_variable()
                get_variable_attr() 
            check_db_cert_in_ram() 
                AuthenticodeVerify()
                tpm_measure_variable()
        check_db_cert(L"MokListRT",...)

    #if defined(VENDOR_CERT_FILE)
    .....
    tpm_measure_variable()