Using RF24 in reverse engineering protocols

[2bndy5]

Hey @2bndy5 , thanks for your hard work on this library - much appreciated.

If this should be in a new issue, please split it out.

Just wanted to point out that there's a really important use case for setting the first address byte to 0x00, namely protocol reverse engineering. Here's some reading if you're unfamiliar:

• https://travisgoodspeed.blogspot.com/2011/02/promiscuity-is-nrf24l01s-duty.html
• http://www.remote-exploit.org/articles/keykeriki_v2_0__8211_2_4ghz/index.html
• https://github.com/Yveaux/NRF24_Sniffer

Right now, to get around the limitations of this library, something like this is required:

radio.setAutoAck(false);
writeRegister(RF_SETUP, 0x21); // Disable PA, 250kbps, LNA enabled in a single op - no ctor to do this in one go?
radio.setPayloadSize(sz);
radio.setChannel(ch);
writeRegister(EN_RXADDR, 0x00);    // this is critical - seems no way to do this with RF24?
writeRegister(SETUP_AW, 0x00);  // though perhaps passing in a_width of 2 would work? https://github.com/nRF24/RF24/blob/2819745c727cccc2cf2308595a77c637cc84cd95/RF24.cpp#L1417
radio.openReadingPipe(0, 0xAALL);
radio.openReadingPipe(1, 0x55LL);
radio.disableCRC();
radio.startListening();
// ...

where writeRegister is a raw SPI transfer that also handles CSN toggling. Not pleasant!

I see 70786aa which looks encouraging to help with some of this - can you clarify how the use case I've outlined might be supported by this commit? I expect other language bindings (like the Python one) will need to have this variable exposed as well...) Thanks!

Originally posted by @wohali in #496 (comment)

[2bndy5]

This is dense; grab a drink now - then continue

I see 70786aa which looks encouraging to help with some of this

That commit is on my experimental fork's "revamped" branch... I haven't tested it in a while (at least a few months at this point). The main idea is that the added private bool _is_pipe0_rx is toggled true when pipe 0 is used for RX operations (via calling openReadingPipe(0, some_addr), then toggled false when pipe 0 is closed for RX operations (via calling closeReadingPipe()). This allows more intuitive handling of pipe 0 when startListening() is called. Really, this was hinted by @hexwell (thus the tag in commit title) to bypass the "null vs 0" problem that #496 raises/discusses.

The use of an extra bool to do this would occupy a whole byte in memory and only serves to resolve #496 seeming edge case (in spite of poor addressing outlined by ManiacBug)

I expect other language bindings (like the Python one) will need to have this variable exposed as well...)

Not likely because the added bool would not need a public scope (only the lib would need to use it), and the python wrapper does not expose private members (using either boost.python or pybind11). If it was a pure python lib (like my CirPy lib), then it could be accessed (because nothing is really private in python). However, the entire "null vs 0" problem is avoided with the difference between python's None vs the buffer b"\0"

---

writeRegister(RF_SETUP, 0x21); // Disable PA, 250kbps, LNA enabled in a single op - no ctor to do this in one go?

Correct, each part of this register is handled according to it's relative feature. Instead, this would be accomplished with

radio.setPALevel(RF24_PA_MIN, true); // `true` asserts the LNA_HCURR bit (which is done by default)
radio.setDataRate(RF24_250KBPS); 

---

writeRegister(EN_RXADDR, 0x00);    // this is critical - seems no way to do this with RF24?

This can be done with RF24 lib like so

for (uint8_t i = 0; i < 6; ++i)
    radio.closeReadingPipe(i);

However, it is no where near as efficient as clearing the whole register at once. I remember a comment in src from @TMRh20 explaining the lib's intention about handling pipes individually:

RF24/RF24.cpp

Lines 1402 to 1405 in e22a8be

	// Note it would be more efficient to set all of the bits for all open
	// pipes at once. However, I thought it would make the calling code
	// more simple to do it this way.
	write_register(EN_RXADDR, read_register(EN_RXADDR) | _BV(pgm_read_byte(&child_pipe_enable[child])));

---

writeRegister(SETUP_AW, 0x00);  // though perhaps passing in a_width of 2 would work? https://github.com/nRF24/RF24/blob/2819745c727cccc2cf2308595a77c637cc84cd95/RF24.cpp#L1417

I often refer to the datasheet when faced with such questions. Observe

However, the datasheet is known to be inaccurate in some areas. Maybe the "'00' - Illegal" is yet another instance of inaccuracy(?).

---

where writeRegister is a raw SPI transfer that also handles CSN toggling. Not pleasant!

1. This is actually a common library convention for SPI implementation. Although your use case could be an exception (I'll come back to that later). I should note that I had at one point (when developing my CirPy lib) tried performing several SPI commands while the CSN remained LOW. This resulted in the nRF24L01 ignoring everything after the first command was completed. The datasheet mandates,

   Every new command must be started by a high to low transition on CSN

   My experience concurs.

2. Unfortunately, this function is a private member. We can discuss making writeRegister() protected in more detail on [Request] make read_register() protected for possible derivatives #782 ; personally, I'm open to accepting it as a protected member.

   • As a protected member, one could easily inherit from RF24 class and handle the CSN pin as needed.

---

Where to go from here

I think your use case actually falls outside the scope of the RF24 lib's original intention which was to provide a beginner-friendly interface for the nRF24L01 transceiver. I talked a bit about inheriting from the RF24 class because it could be extended to suite your use case. Although, some things might need to be made protected for 2 reasons

1. backward compatibility
2. The RF24 class simply wasn't designed to be altered via inheritance. I'm willing to help fix this, but any "tooling" derivative would need a home of its own (not belonging to this repo).

[2bndy5]

I forgot to mention 1 thing:

radio.setPayloadSize(sz);

The minumum payload size that the RF24 class allows is 1 byte. This is because the datasheet states that setting the payload size for any pipe would effectively disable the pipe.

And, yes it could be done individually per pipe, but this lib doesn't support that (for code-size reasons). My CirPy lib does support individual pipe management for payload_size, dynamic_payloads, and auto_ack 😉 .

[2bndy5]

• https://travisgoodspeed.blogspot.com/2011/02/promiscuity-is-nrf24l01s-duty.html

This is great reading! I've been wanting to reverse engineer Logitech's unifying receiver dongle, so that maybe an open-source mouse doesn't need any new receiver plugged into the PC's USB ports... These findings can't be shared publicly as Logitech does well to patch wireless exploits. The last exploit published prompted Logitech to use encryption on keyboards (& possibly mice now too).

[wohali]

Hey @2bndy5 , thanks for splitting this out. I've skimmed most of this and understand, but wanted to focus on one thing:

I think your use case actually falls outside the scope of the RF24 lib's original intention which was to provide a beginner-friendly interface for the nRF24L01 transceiver. I talked a bit about inheriting from the RF24 class because it could be extended to suite your use case.

My understanding was that this was a forked version of the original beginners' library, as the library's documentation states:

More compliant with the manufacturer specified operation of the chip, while allowing advanced users to work outside the recommended operation.

I guess I gravitated towards this library in the hopes that it would be more amenable to RE use cases, rather than having to bitbang SPI to get what I am looking for. It's so very close -- and only needs a couple of very minor tweaks.

Given I'd prefer to be calling this most often from something like JS or Python (and there are already many issues installing this library in a managed Python dependency toolchain such as Poetry), I'd rather not have to sub-class everything myself. Otherwise I'm looking at a) maintaining a fork of the C++ code (ugh), and b) having to sort out publishing Python bindings to that myself, whereas I'd much rather some sort of compile-time option to your library (something like -DUNSAFE_RX) and to help contribute to the Python bindings to make this library's use simpler for standard tx/rx use cases.

FYI my intention has nothing to do with sniffing keyboard/mouse protocol stuff; I actually only got into this library to reverse engineer the protocol for my house's baseboard heaters, which turned out to be rather simple -- ONCE I got past the issues mentioned in my first post. 😉

Does my counter-proposal interest you at all?

[wohali]

Two other minor comments:

The minumum payload size that the RF24 class allows is 1 byte.

That's fine, I've not needed to mess with this lower.

my CirPy lib

I'm actually doing most of this on a BeagleBone right now for the added bandwidth, especially when sniffing packets. I don't know if CirPy is fast enough in this situation, but I'd be willing to simply drop this use case from here and migrate to CirPy if you felt it would be more fruitful.

[2bndy5]

Does my counter-proposal interest you at all?

Yes! Sorry about the info dump; I got overexcited when you mentioned RE. I've also been looking at appropriating the firmware for the (now deprecated) steam controller which literally uses the Nordic ESB protocol when its not in BLE mode. IIRC, controller uses nRF51822 while the receiver uses a nRF24LU1.

It's so very close -- and only needs a couple of very minor tweaks.

Specifically, what are you requesting? The code snippet in the OP is kinda broad and leaves me with a partial idea.

• A solution for great library - but when opening for reading on pipe0 #496 is now being considered.
• Were you hoping for some new methods (like open/closeAllReadingPipes())?
• How exactly were you planning to manipulate the CSN?

a managed Python dependency toolchain such as Poetry
...
I don't know if CirPy is fast enough in this situation

1. I did some preliminary work on re-writing the python wrappers with pybind11 on one of my forks (see use pybind11 instead of boost.python pyRF24#3). The original intention is to have pre-built wheels released to pypi.org.
2. My rf24-network branch of my CirPy lib has some speed-ups implemented. The most promising of which is depending on the SpiDev module (a C extension available from pypi.org) for Linux - this proved to be at least 7X faster than using adafruit's pureIO lib for SPI transactions (but still slower than this lb's python wrapper). I don't have BeagleBone, so I haven't been able to test if that branch works on that hardware.
   • I'm not fluent with poetry, but you can try the branch with

     pip3 install git+https://github.com/nRF24/CircuitPython_nRF24L01@rf24-network
     

     More info on the features of that branch (there's a lot) are listed in the PR (see RF24Network, RF24Mesh, and RXing BLE CircuitPython_nRF24L01#33)

Ultimately the difference between this lib and the CirPy lib is C++'s amazing execution speed. Although I generally like to do rapid developing in python (for experiments and proof-of-concepts), then port the python algorithms to C++. Although, having a pip installable C extension of this lib would be more ideal.

something like -DUNSAFE_RX

If needed, I could easily add something like this to the CMake offering (still being tested on the rp2xxx branch but getting all good feedback). The old ./configure; make; sudo make install is not future proof, so I'm trying to start weening off that approach.

[2bndy5]

@wohali I just pushed a solution to #496 on the rp2xxx branch. I would greatly appreciate any other advice or requests you may need to accomplish your RE goals.

[wohali]

Hey @2bndy5 , sorry for the delay - been very busy with other stuff this past week, I'll try and review everything during the course of this coming week.

[2bndy5]

@TMRh20 Would you approve a new function called void toggleAllPipes(bool isEnabled)? It is the only other task that the snippet in the OP can't accomplish with current public API.

@Avamander Am I correct in assuming that this new function would not affect existing applications' compile size if they never call it?

[Avamander]

@2bndy5

Am I correct in assuming that this new function would not affect existing applications' compile size if they never call it?

Usually yes, much more certain with -fLTO.

[2bndy5]

Hmm, I see ATTiny core and AVR core uses -flto explicitly, but I don't see that's exactly the same case for SAMD core.

[2bndy5]

@wohali I took some pointers from the readings you provided and altered the CirPy lib's scanner example. It seems to work better now, and I added a noise() function to show ambient noise for a specified channel. 🤓

[TMRh20]

Yeah I’m ok with a new function.

…

On Oct 26, 2021, at 4:20 AM, Brendan ***@***.***> wrote:  @wohali I took some pointers from the readings you provided and altered the CirPy lib's scanner example. It seems to work better now, and I added a noise() function to show ambient noise for a specified channel. 🤓 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

[2bndy5]

@wohali I've added the new function toggleAllPipes() on the "RE-tactics" branch. If there are no other requests or concerns about RF24 lib limitations, then I'll open a PR that will close out this issue.

1. Do you really need to manipulate the CSN pin?
2. Do you really need to set the RF_SETUP register in a single transaction?

   writeRegister(RF_SETUP, 0x21);
   
   // synonomous with
   radio.setPALevel(RF24_PA_MIN); // LNA_HCURR is asserted by default here
   radio.setDataRate(RF24_250KBPS); 

[wohali]

Hey @2bndy5 , sorry for the endless delays here. So very busy!

@wohali I've added the new function toggleAllPipes() on the "RE-tactics" branch. If there are no other requests or concerns about RF24 lib limitations, then I'll open a PR that will close out this issue.

Can you give me a short summary of what this does - assuming I don't have time at the moment to read the code?

1. Do you really need to manipulate the CSN pin?

Only insofar as the writeRegister operation needed it:

inline uint8_t writeRegister(uint8_t reg, uint8_t value)
{
  uint8_t status;

  digitalWrite(CSN, LOW);
  status = SPI.transfer(W_REGISTER | (REGISTER_MASK & reg));
  SPI.transfer(value);
  digitalWrite(CSN, HIGH);
  return status;
}

and similar if passing in a const uint8_t *buf, uint8_t len instead, with a loop over buf to SPI.transfer().

If your abstractions now handle all of the things I had to use writeRegister for (e.g. writeRegister(EN_RXADDR, 0x00); writeRegister(SETUP_AW, 0x00);, then I won't need to poke CSN directly.

2. Do you really need to set the `RF_SETUP` register in a single transaction?
   ```c++
   writeRegister(RF_SETUP, 0x21);
   
   // synonomous with
   radio.setPALevel(RF24_PA_MIN); // LNA_HCURR is asserted by default here
   radio.setDataRate(RF24_250KBPS); 
   ```

The issue here is that order matters and has a significant impact on what the chip actually does - at least, in reveng scenarios; I haven't checked for normal operation. Things didn't work right if I setDataRate ahead of setPALevel, or if I didn't setAutoAck(false) before either of those. Doing it in a single step removed my footgun.

I can live with the current limitation, but in the interest of making the library easier for end users, it might make sense to offer a ctor for the radio object that allows specifying this all at once, and letting the library take care of proper ordering.

[2bndy5]

writeRegister(EN_RXADDR, 0x00); is the same as calling toggleAllPipes(false);.
writeRegister(EN_RXADDR, 0x3F); is the same as calling toggleAllPipes(true);.
writeRegister(SETUP_AW, 0x00); is the same as calling setAddressWidth(2);

---

Things didn't work right if I setDataRate ahead of setPALevel, or if I didn't setAutoAck(false) before either of those.

1. The RF_SETUP register is the only register that is slightly different in the nRF24 clones. Organizationally, it should not impact the RF24 lib interaction; meaning the lib should work the same for different clones.

2. The only thing that might cause a problem calling setDataRate() & setPALevel() separately is that both functions capture the RF_SETUP register's value before manipulating it. Additionally, setDataRate() also re-captures the register to confirm the value was written correctly (even though it doesn't need to - this was done on an old & erroneous assumption that the 250kbps setting wouldn't stick for plus-variants of nRF24L01).

   • setAutoAck(false) also calls disableAckPayload() which manipulates the FEATURE register, but only if ACK payloads were previously enabled.

   Anything else that would cause this problematic behavior would have to be in your transceiver's firmware. For example, CRC is only truly disabled after auto-ack is disabled.

I'm very hesitant to allow users to directly manipulate the RF_SETUP register because it also exposes vital hardware manipulation, namely the PLL_LOCK (& continuous carrier wave flag for non-plus variants). Even for RE purposes, poorly configuring the PLL_LOCK will have detrimental side effects that could damage the radio or (at best) make it seem broken.

it might make sense to offer a ctor for the radio object that allows specifying this all at once

I guess we could add a c'tor that would take RF_SETUP parameters, but

• initializing the radio is actually done with begin(), not the RF24::RF24() c'tor. Technically, it is done by the private function _init_radio() (since v1.4.0).
• it would be workaround that doesn't really address the problematic experience you described
• it would have to be added to the python wrapper (which is not automatically done/maintained).

[2bndy5]

I would be more inclined to add a new function

void setupRF(uint8_t level, uint8_t rate, bool lna_enabled=true)
{
    // manipulate certain bits while keeping all other bits set to 0

    // write register value all at once
}

[2bndy5]

Looking into the src code

Turns out that begin() doesn't actually call setPALevel(). it only calls setDataRate(RF24_1MBPS). This means that whatever the PA level is on boot (or MCU hard-reset which doesn't reset radio's registers) is persistent despite calling begin(). While this isn't ideal, it might give a hint as to @wohali problematic experience. This discovery would also imply that begin() can be improved with setupRF() instead of separate calls to setDataRate() & setPALevel().

Furthermore, adding a setupRF() may not be as non-impactful as I assumed. This is because setDataRate() configures the txDelay (used in stopListening()).

• If I simply copy the needed code from setDataRate() into setupRF(), then we would have 2 instances where txDelay would need to be altered 👎🏼 .
• If I make a private function that allows re-using the same code from setDataRate() in setupRF(), then it might increase the code-size for applications that don't ever call setupRF().
  • @Avamander I'd like to assume that making the new private re-usable function explicitly inline, we might avoid the increase in compile size. Am I assuming too much? Sorry to lean on you like our resident compiler expert, but you seem to know your stuff in this regard.

Any application that wasn't previously calling setDataRate() and/or setPALevel() will see an increase in compile size when they do manipulate the RF_SETUP register, despite the existence of setupRF(). Most examples already use setPALevel() as a power saving "hack", so it is likely that people are copying that function call into their own applications (unless they are following a third-party tutorial 😥 ).

ps - I'm open to a better function name than setupRF() (which feels bland & slightly confusing) as it is starting to look like I'm leaning toward deprecating the separate functions about the RF_SETUP register.
My first-guess alternatives are setRadiation() or setPALevelDataRate(); the last thing we need is someone guessing that setupRF() or setupRadio() is replacing begin().

[2bndy5]

I just tried compiling the RE-tricks branch (with the new setRadiation() and toggleAllPipes() functions) and the GettingStarted.ino sketch showed no increase on the ATMega328P. I guess the explicit inline private function payed off. Changes also include an update to begin() using the new setRadiation() function.

@wohali I should note that begin() was calling setDataRate() before setting the EN_AA register. So, I have to ask, what exact transceiver are you using? It may also have had something to do with your scenario's context (especially if you were writing values directly to the registers).

[wohali]

@2bndy5 One of the standard nRF24L01+ transceivers you see everywhere for about US$10 - the ones with the extra LNA chip and the SMA connector.

[wohali]

Also your finding re: begin() suggests that it is overriding the DataRate - is that correct? This would be a surprising side effect to me.

[2bndy5]

Also your finding re: begin() suggests that it is overriding the DataRate - is that correct?

I don't know when or where you were using the setDataRate() and setPALevel(). I'm simply stating that calling begin() inherently called other functions (please review the code) that were intended to emulate a soft reset. It was a mistake on my part to have not been calling setPALevel() in begin().

the ones with the extra LNA chip and the SMA connector.

That would be the PA/LNA module. You would need to disable auto-ack first for those because it cuts down the power consumption significantly. I say this because those modules are famous for excessive power consumption when TX-ing (including ACK packets). Changing the PA level also helps cut down the power consumption, so that would also need priority over data rate.

How are you supplying power to that module? What MCU board are you using?

[wohali]

@2bndy5 Bench power supply with meter, driven by a BeagleBone. And yes the code disables auto-ack first. It does work, just fine - no hardware issues - I just ended up with the code you see to make it work.

[2bndy5]

I ask because usually the linear regulator on a RPi doesn't supply enough current to the PA/LNA radio module. Unless you're trying to say that you use the bench power supply to feed the radio and BB in parallel. Otherwise, if you're using a 3v pin from the BB to supply the radio, then you might want to monitor the current spikes from the BB to the radio and make sure they don't flatline (typically indicating the current limit isn't high enough).

[2bndy5]

I would ask that you test the "RE-tricks" branch as that PR will close this issue when it gets merged.