Remove broken `--par` option from `lf em 4x70`

[henrygab]

Current usage

There are two variables used for parity:

• command_parity - global variable, client sets false by default
• with_parity - Parameter to em4x70_send_nibble() function

All seven entry points in ARM source set global variable command_parity per input etd->parity.

Client source defaults this value to false in all cases.

Therefore, unless explicitly provided via --par for a command, the value in the ARM source of command_parity will always be false.

ARM firmware reliance on command_parity

Details

The value of this variable is only read in two locations:

• send_command_and_read()
• em4x70_send_nibble()

send_command_and_read()

This function calls em4x70_send_nibble() with the provided command, setting the function parameter with_parity equal to the global variable command_parity, and then immediately listens for a tag response.

em4x70_send_nibble()

This function's parameter with_parity does DIFFERENT things, using both parameter with_parity and the global variable command_parity.

When global command_parity is false, all four bits of the nibble are sent. This is the default behavior with the client code.

When global command_parity is true, only the three least significant bits of the nibble are sent. (Mask 0x07).

When parameter with_parity is true, and extra bit is sent, which is the parity for the 3-bits (command_parity == true) or 4-bits (command_parity == false) that was sent.

When parameter with_parity is false, no additional parity bit is sent.

#	command_parity	with_parity	nibble bits	parity bits	total bits
0	false	false	4	0	4
1	false	true	4	1	5
2	true	false	3	0	3
3	true	true	3	1	4

Callers of em4x70_send_nibble()

This function is called by the following, with "Valid" indicating the code appears valid if command_parity is also set to true.

caller	with_parity	Valid	Sends
em4x70_send_word()	true	N	five-bits for each row of 4x4 data
em4x70_send_word()	false	N	parity row for the 4x4 data
authenticate()	true	Y	the command
authenticate()	false	N	last four bits of frn
send_pin()	true	Y	the command
write()	true	Y	the command
write()	true	?	the address for the write
send_command_and_read()	command_parity	Y	the command

In particular, if command_parity is true, then for each nybble of the word of data, em4x70_send_word() will only send the three least significant bits (+ two parity bits ... thus losing data). Then, when sending the parity row, it will ALSO lost the most significant bit (and send a parity bit + zero bit).

In addition, the authenticate() command will similarly lose data from the last four bits of frn.

Finally, the write() command will also lose data for much the same reason.

Conclusion

The --par option is fundamentally broken, and thus is "dead code" as it fails to work in any situation where data is sent to the tag.

Therefore, removal of the --par option and related dead code will improve the codebase.

[henrygab]

The only unaffected ARM entry points are em4x70_info() and em4x70_unlock().

What gets corrupted

if command_parity (set by client --par option) is set, then em4x70_send_nibble() will only send the three least significant bits. Thus, ANY call to em4x70_send_nibble() where all four bits contain data intended to reach the tag will result in corrupted data being sent.

Notably, em4x70_send_byte() unconditionally sends eight bits.

Arm Entry	What Corrupted	Info
em4x70_info()	N/A	OK because only reads data from tag
em4x70_write()	block data + parity row	https://github.com/RfidResearchGroup/proxmark3/blob/2bc7c5030234e43a1436d98bb7f5fec34802f29c/armsrc/em4x70.c#L287-L292
em4x70_unlock()	N/A	OK because only uses em4x70_send_byte() for data
em4x70_auth()	last four bits of frn	https://github.com/RfidResearchGroup/proxmark3/blob/2bc7c5030234e43a1436d98bb7f5fec34802f29c/armsrc/em4x70.c#L338
em4x70_brute()	last four bits of frn	https://github.com/RfidResearchGroup/proxmark3/blob/2bc7c5030234e43a1436d98bb7f5fec34802f29c/armsrc/em4x70.c#L338
em4x70_write_pin()	pin (!!!)	https://github.com/RfidResearchGroup/proxmark3/blob/2bc7c5030234e43a1436d98bb7f5fec34802f29c/armsrc/em4x70.c#L868-L870
em4x70_write_key()	key (!!!)	https://github.com/RfidResearchGroup/proxmark3/blob/2bc7c5030234e43a1436d98bb7f5fec34802f29c/armsrc/em4x70.c#L482-L483

[iceman1001]

Great write-up and findings!

Is there a way to do a regression test for these things? So we can capture it with the pm3_test.sh script?

[henrygab]

A regression test is a good idea. At present, I don't see where em4x70 could have much testing without a tag present? Or, maybe I'm not understanding the regression test yet.

[iceman1001]

if we have some dumps, we can do some tests to make sure the offline commands works.
if crypto involved, we can add a self test for it.

[henrygab]

Ok. ~~All current client commands require a tag.~~

Ah... lf em 4x70 recover is available without a tag. Ok.

Maybe also add a command to directly expose calculation of challenge / response?

[henrygab]

... and added lf em 4x70 calc to allow calculating challenge (frn) and response (grn) for a given key + rnd combination ... and added tests to tools\pm3_tests.sh ... just waiting for GitHub Actions to build successfully before pushing that commit, and checking its result.

Should be ready to merge within a few hours, if all goes well.

[iceman1001]

Nice, some heads up,

We use two different styles when adding a self test function.

1. Parameter --test
2. Command lf em 4070 test

which both should give a outcome line like:

PrintAndLogEx(SUCCESS, "------------------- Selftest %s", (testresult == NUM_OF_TEST) ? _GREEN_("ok") : _RED_("fail"));

If added like this we can now easily add a run of it in the tools/pm3_tests.sh script

[henrygab]

I will leave the PR as-is, as it is functionally correct, mirrors existing tests, is validated to work, and improves the validation as per your request.

The PR modeled the added tests based on those already in tools\pm3_tests.sh. I don't think there's significant value in rewriting the tests in PR #2385 to be embedded in the client source code (translating to C). Of course, if you disagree, please feel free to do that. There are higher-value changes to be made.

higher-value work

Some items that will provide greater value and are on my radar:

1. Fix ARM code for lf em 4x70. Code never worked on two of my three PM3 Easy devices. Recently, I discovered one potential cause ... various PM3 Easy devices have a (slow) sawtooth DC bias in the traces. This may cause problems because the em4x70 codebase has hard-coded noise threshold.
2. Add logging for lf em 4x70. As part of this, update ARM code to generate entire bitstream first, then send the pre-generated bitstream (rather than calculating + sending as one operation).
3. Add decoder for lf em 4x70 sniffs. Manually decoding analog traces is still not a great experience. Existing data plot commands have issues. One example is when clock should be extracted from signal, and shifts during a long trace. Or, when sniffed trace loses samples, this accumulates over time, causing clock drift. Even if the decoder just spits out results on the console (not modifying plot window), that will still save hours of time manually decoding.

[henrygab]

Update: After adding some logging to more easily see what bits are sent/received, it confirmed that ... this code is messy.

The following is what currently occurs (not using the --par option):

• The three commands that read ID, UM1, and UM1 would send the command as a simple four-bit value ... no parity added.
• The three commands that exchange data with the tag were hard-coded to always send a fifth parity bit after sending the four-bit command value.

Preliminary logs and notes are my em4x70_dev branch .

Todo:

• [ ] Check what variations of commands (3 or bit) + parity bit (added or not) work for each command
• [x] ~~Determine possible bug in logging of last four bits of authentication FRN~~ Log too small, auth needed 98 bits
• [ ] Simplify code (pre-generate bitstream to be sent)
• [ ] Integrate and use the existing logging system (vs. my current hack)
• [ ] Acquire at least one tag that would use the --par option to the commands?

[iceman1001]

@cmolson was the one who created the commands, maybe he has some input ?

[cmolson]

Hi, Thanks for all the investigation, and sorry about the mess/confusion.

The tag I was looking into (ID48?) was a slightly modified version of the EM4170 which is why I tried to make this code work for both. I think the differences were around sending parity with the commands.

I still have all my tags and proxmark device, I need to get it set up again and re-familiarize myself with this to offer any useful input. I should be able to do this over the next few days.

[henrygab]

Oh, nothing to apologize for; Quite the opposite. It's only because of your foundational work that I made progress myself ... so thank you for making this possible!

I have ID48 tags from my old vehicle, and XT27A tags that can be configured to ID48. What I've yet to acquire is any tag that requires use of the --par option.

If you're going to poke and prod, consider using my dev branch ... It has a functional trace built-in that will show all bits sent, and all bits received (+start/stop timing for each).

Or, if you have any extra V4070 tags (or an actual EM4170 tag ... both hinted as working differently), I'd be happy to do the poking and prodding, if you're open to loaning those out.

[cmolson]

I would be more than happy to send you a few of the various tags I bought. I will also try your branch with the tags I have, thanks!

[iceman1001]

.... don't be a stranger, if you have some spares :)

[cmolson]

I need to figure out what exactly I have.. but I have two bags with 10 tags each. One is "ID48" and the other says "ID48 EM4170"... I only want to keep 2 of each. Let me know how to get your mailing addresses and I'll send you both some.

[iceman1001]

if you on the discord server, do DM

[henrygab]

Let me know how to get your mailing addresses and I'll send you both some.

Ping me on Iceman's RFID discord server? I have a hunch what your discord id is, but not conclusive.

(Mine is obvious. 🙂)