search

tillitis / tillitis-key1

Board designs, FPGA verilog, firmware for TKey, the flexible and open USB security key 🔑
https://www.tillitis.se
382 stars 24 forks source link

Changing names for ram address and data randomization #220

Closed secworks closed 1 week ago

secworks commented 2 months ago

This PR is first attempt att fixing #219. Needs review and checks.

The build of the FPGA gives different results with the patch compared to main. The build should not be affected by simple name changes.

dehanj commented 1 month ago

The CI job check-hashes actually spits out the shasum of application_fpga.bin before it compares, so one easier can compare to the one you get locally with tkey-builder:4.

CI's calculated checksum from the logs

sha256sum application_fpga.bin
4095aaa97f41eba88b6ca846eb1cc81e84f368a86be747809490bb9beab20cb6  application_fpga.bin
mchack-work commented 1 month ago

Rebasing on main gets rid of the merge commit present in this branch and brings in a change for .gitignore.

Verified that the checksum on application_fpga.bin still is different than expected.

Last commit before the name changes, commit 0454e023cd5c05cf8c84df358c45a05f291e153e, from main works fine.

Looked through carefully to see if any constants have changed. Only differences in, say, strings I have found are in the testbench, which shouldn't influence application_fpga.bin at all.

secworks commented 3 weeks ago

Done a lot of testing and added fixes step by step in branch https://github.com/tillitis/tillitis-key1/tree/219_new_names_again Finally identify the issue to be the name of the registers used to store the parameters. If the name contain 'addr' or 'data' the implementation is changed: The number or resources allocated, and of course, the digest. Changing name to 'addraddraddr' still triggers the issue. 'eddr' however does not.

Sent email with a description of the issue to Yosys 2024-06-04.

dehanj commented 1 week ago

Closing this in favor of #229