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

Adding explanation and changing name of personalize.py #189

Closed dehanj closed 3 months ago

dehanj commented 3 months ago

Changed filename in order to be a bit less cryptic about what the python script does. Also adding a more detailed explanation about what it intends to do.

secworks commented 3 months ago

I prefer the old name (though not the description in the hader). The use case is to create a unique TKey (i.e. personalize it) by generating unique UDS and UDI. Patching is just how these unique fields are put into the design.

I have no problem with tools that have names longer than three letters. 'personalize_uds_udi.py', 'gen_unique_tkey.py' 'gen_udi_uds.py' are some ideas of names that reflect what the tool does.

dehanj commented 3 months ago

I prefer the old name (though not the description in the header). The use case is to create a unique TKey (i.e. personalize it) by generating unique UDS and UDI. Patching is just how these unique fields are put into the design.

While I agree, I also disagree with the use case you mention.

The use case of this particular script is to simply patch your already personalized (hopefully) UDS or UDI. This tool does not make sure you have personalized anything, nor does it change the already set UDS or UDI. it simple takes some data and patch it into another file, hence the choice of the filename.

To personalize your UDS and UDI your are using tpt.py, (Tillitis Provisioning Tool), and this tool to actually put it in the bitstream - so a both of them achieves a personalization according to me. Or you simpy update the uds.hex and udi.hex by yourself.

So I actually think it is a bit misleading and might give false hopes that all you need to do to get a unique UDS and UDI is to run this tool, which is not true.

secworks commented 3 months ago

Ah, I'm mixing up tpt with personalize.py. Then i totally agree with the new name.

dehanj commented 3 months ago

I think the changes look good, but suggest we squash all the commits.