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. 🙂)