Attention is currently required from: Julius Werner, Jan Dabros.

View Change

4 comments:

• File tests/lib/Makefile.inc:

  • Patch Set #2, Line 199: CONFIG_NO_CBFS_MCACHE=0 \

    I disabled MCache for this test. It will be tested in another one.

    I guess you have this for the cbfs_init_boot_device() stuff... maybe it makes more sense to move that to a separate test?

    cbfs_init_boot_device() has some of verification code, so I think it is necessary to include this in this test. I would like not to split tests too much, because it will make them unreadable (the same for too big tests).

  • Patch Set #2, Line 202: VB2_SUPPORT_SHA512=0

    would probably be a good idea to build this test twice
    Done. One test source, but four major cases tested. Great idea :)

• File tests/lib/cbfs-verification-test.c:

  • Patch Set #2, Line 235: assert_int_equal(CB_SUCCESS, cbfs_init_boot_device(&cbd, &hash));

    Oh, and this is the one where you test it with a metadata_hash. Okay. […]

    Done

  • Patch Set #2, Line 244: NULL

    The real lib/cbfs.c should never pass NULL to cbfs_init_boot_device() when CBFS_VERIFICATION is enabled.
    I have seen cbfs_init_boot_device(cbd, NULL) in ./src/security/vboot/vboot_loader.c, so I tought tat it would be nice to include it in the test as well.
    Do you think, that I should remove it from this test and move it to e.g. lib/cbfs-metadata-test?

    Regarding CBFS_WALK_WRITEBACK_HASH, I think we could mock vb2_digest_*()`functions to eliminate need of linking vboot. vb2_digest_extend() could look like this (assuming we are using SHA256 for this test):

    vb2_error_t vb2_digest_extend(struct vb2_digest_context *dc, const uint8_t *buf,
    			      uint32_t size) {
        for(size_t i = 0; i < size; ++i)
            ((uint8_t *)dc->sha256.h)[i % VB2_SHA256_DIGEST_SIZE] += buf[i];

        return VB2_SUCCESS;
    }

To view, visit change 56601. To unsubscribe, or for help writing mail filters, visit settings.