RPi Gateaway: Dropping root privilege

alexvanbelle

Hi,

Sorry for bringing back an old topic but it seems an important one nowadays. I've been running MySensors for 2 years now and I truly believe its community did an awesome work.

I have similar goal (defence in depth): running a daemon as root is kind of an issue. While I'm sure developers do their best to avoid a security issue, we never know. Even if it can sound a little bit paranoid, an attacker could try to inject packets that would execute a command on the gateway host.

I'm running the mysgw on raspbian (raspberry 1b). As Julien, I've created a new user.

First results:

First error: Could not open /sys/class/gpio directory => just add the user to the gpio group
Second error: You need root privilege to use SPI

It would be awesome to solve this concern. I've seen Julien didn't get any reply.
Does it mean the community doesn't see any advantages or just it mean you just need some help? I can contribute to documentation.

I guess it's a low concern for gateways on microcontrolers while it's a bigger one for microprocessors one.

Let me know if I can help :relaxed:

Alexandre

mfalkvidd

My guess is that nobody with sufficient interest and knowledge has had enough time to dive into this.

alexvanbelle

Ok, then let me try to help.

Right now, I have added a user named mysensors. He is member of its own group + GPIO + SPI:

mysensors@pi$ groups mysensors
mysensors : mysensors spi gpio

I'm running mysensors 2.3.0. If I run it with this user, I currently get:

mysensors@pi$ mysgw
Sep 23 14:58:56 INFO  Starting gateway...
Sep 23 14:58:56 INFO  Protocol version - 2.3.0
Sep 23 14:58:56 DEBUG MCO:BGN:INIT GW,CP=RNNGL--X,VER=2.3.0
Sep 23 14:58:56 DEBUG TSF:LRT:OK
Sep 23 14:58:56 DEBUG TSM:INIT
Sep 23 14:58:56 DEBUG TSF:WUR:MS=0
Segmentation fault

Everything works well with root:

root@pi:~# mysgw
Sep 23 15:01:34 INFO  Starting gateway...
Sep 23 15:01:34 INFO  Protocol version - 2.3.0
Sep 23 15:01:34 DEBUG MCO:BGN:INIT GW,CP=RNNGL--X,VER=2.3.0
Sep 23 15:01:34 DEBUG TSF:LRT:OK
Sep 23 15:01:34 DEBUG TSM:INIT
Sep 23 15:01:34 DEBUG TSF:WUR:MS=0
Sep 23 15:01:34 DEBUG TSM:INIT:TSP OK
Sep 23 15:01:34 DEBUG TSM:INIT:GW MODE
Sep 23 15:01:34 DEBUG TSM:READY:ID=0,PAR=0,DIS=0
Sep 23 15:01:34 DEBUG MCO:REG:NOT NEEDED
Sep 23 15:01:34 DEBUG Listening for connections on 0.0.0.0:5003
Sep 23 15:01:34 DEBUG MCO:BGN:STP
Sep 23 15:01:34 DEBUG MCO:BGN:INIT OK,TSP=1

Any idea of the possible missing rights that I need to avoid the segfault? Or to get better error message?

Will continue to look at it...

alexvanbelle

I've added some traces.
Seems like seg fault happens on these lines of MyTransport.cpp

void transportUpdateSM(void)
{
        if (_transportSM.currentState) {
                _transportSM.currentState->Update();
        }
}

alexvanbelle

Hi,

I'm not able to quickly understand the code base.

I guess the problem is with transport (call to _transportSM.currentState->Update())
Any idea of what kind of call might create the segfault when user is not root (since everything works fine with root on this system)

My setup is:

Hardware: Raspberry pi 1B
OS: Raspbian
Mysensors: latest release (2.3.0)

User is already member of groups GPIO and SPI.

Any hint would be appreciated :)

Alexandre

rozpruwacz

I run mysgw with gdb and for me it segfaults when trying to access gpio:

(gdb) run
Starting program: /usr/local/bin/mysgw 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/arm-linux-gnueabihf/libthread_db.so.1".
Sep 29 13:04:51 INFO  Starting gateway...
Sep 29 13:04:51 INFO  Protocol version - 2.3.0
Sep 29 13:04:51 DEBUG MCO:BGN:INIT GW,CP=RPNGL---,VER=2.3.0
Sep 29 13:04:51 DEBUG TSF:LRT:OK
Sep 29 13:04:51 DEBUG TSM:INIT
Sep 29 13:04:51 DEBUG TSF:WUR:MS=0

Program received signal SIGSEGV, Segmentation fault.
bcm2835_peri_read (paddr=0x7) at drivers/BCM/bcm2835.c:123
123			ret = *paddr;
(gdb) bt
#0  bcm2835_peri_read (paddr=0x7) at drivers/BCM/bcm2835.c:123
#1  bcm2835_st_read () at drivers/BCM/bcm2835.c:1126
#2  bcm2835_delayMicroseconds (micros=1) at drivers/BCM/bcm2835.c:449
#3  0x0002ce5c in BCMClass::digitalWrite (this=<optimized out>, gpio=<optimized out>, value=<optimized out>) at drivers/BCM/BCM.cpp:62
#4  0x0002d7ac in RPiClass::digitalWrite (this=<optimized out>, physPin=physPin@entry=24 '\030', value=value@entry=1 '\001') at drivers/BCM/RPi.cpp:56
#5  0x0001c22c in hwDigitalWrite (value=1 '\001', pin=24 '\030') at ./hal/architecture/Linux/MyHwLinuxGeneric.cpp:158
#6  RFM69_initialise (frequencyHz=868000000) at ./drivers/RFM69/new/RFM69_new.cpp:206
#7  0x00020da4 in transportInit () at ./hal/transport/RFM69/MyTransportRFM69.cpp:26
#8  stInitUpdate () at ./core/MyTransport.cpp:100
#9  0x000258a4 in transportUpdateSM () at ./core/MyTransport.cpp:384
#10 transportProcess () at ./core/MyTransport.cpp:464
#11 transportWaitUntilReady (waitingMS=0) at ./core/MyTransport.cpp:453
#12 _begin () at ./core/MySensorsCore.cpp:155
#13 0x00012f2c in main (argc=1, argv=0x7efff4c4) at ./hal/architecture/Linux/MyMainLinuxGeneric.cpp:447

On RPI platform the gpio kernel driver is not used, instead gpio is accessed with mmap :(. When I forced the build to use gpio kernel driver all works without problem when running as non root. So maybe the solution should be to add configure option for rpi to use gpio kernel driver ?

alexvanbelle

@rozpruwacz Sounds good.
"When I forced the build to use gpio kernel driver all works without problem when running as non root". How have you done that?

If you show me the right command, I could work on a patch on the configuration file and ask for a pull request :)

Currently, I'm using this command for configuration

 ./configure --soc=BCM2835 --my-gateway=ethernet --my-port=5003 --my-rf24-encryption-enabled --my-rf24-channel={{ rf24_channel }}

rozpruwacz

@alexvanbelle Well, I forced it really hard by modifying the code :) in drivers/Linux/Arduino.h in line 35 there is an ifdef that decides what to use for GPIO. For RPI case the RPi.h is included instead of GPIO.h, so I just reversed the if, this is the patch You can apply to MySensors:

diff --git a/drivers/Linux/Arduino.h b/drivers/Linux/Arduino.h
index eb9ee38..9592919 100644
--- a/drivers/Linux/Arduino.h
+++ b/drivers/Linux/Arduino.h
@@ -32,7 +32,7 @@
 #include <algorithm>
 #include "stdlib_noniso.h"
 
-#ifdef LINUX_ARCH_RASPBERRYPI
+#ifndef LINUX_ARCH_RASPBERRYPI
 #include "RPi.h"
 #define pinMode(pin, direction) RPi.pinMode(pin, direction)
 #define digitalWrite(pin, value) RPi.digitalWrite(pin, value)

alexvanbelle

@rozpruwacz Thanks, I will try

#ifdef LINUX_ARCH_RASPBERRYPI
#include "RPi.h"
#define pinMode(pin, direction) RPi.pinMode(pin, direction)
#define digitalWrite(pin, value) RPi.digitalWrite(pin, value)
#define digitalRead(pin) RPi.digitalRead(pin)
#define digitalPinToInterrupt(pin) RPi.digitalPinToInterrupt(pin)
#else
#include "GPIO.h"
#define pinMode(pin, direction) GPIO.pinMode(pin, direction)
#define digitalWrite(pin, value) GPIO.digitalWrite(pin, value)
#define digitalRead(pin) GPIO.digitalRead(pin)
#define digitalPinToInterrupt(pin) GPIO.digitalPinToInterrupt(pin)
#endif

By reading the code, it looks like this path is only for raspberry and, if it works, I see a proper fix such as: just remove the if and do what is in GPIO whatever the platform is.

However, I have doubts this block was added by mistake. I need to understand how automated tests would catch any side effects of this fix (regressions?). This specific code was written few years ago: therefore, maybe it was required before but is no more needed now.

Will do that:

Test it on my setup (my raspberry gateway is connected with 10+ sensors (arduinos with nrf))
If it works end to end, I will have to understand a little bit more how automated tests are launched on mysensors code base. If it seems OK, I will submit a PR.

alexvanbelle

Just for traces, this is the commit which seems to add this code block:
https://github.com/mysensors/MySensors/pull/734/commits/074f016a69fd5d67e033b6473b3e39535fd9e2d5

alexvanbelle

Hi,

As a follow-up:

I tried your patch but it doesn't compile on my raspberry
Whatever, I believe that, if it's the right problem, we might have a better solution :)

Solutions:

I've looked to the code and it happens that, by following the calls, the final call are made to /drivers/BCM/bcm2835.h/cpp
This library is imported from here and is currently on v1.50 in MySensors codebase. Latest release is 1.57.
1.50 of this library introduced the right to run as non root.
1.51 is fixing a bug and its description is "1.51 2016-11-03 Added documentation about SPI clock divider and resulting SPI speeds on RPi3. Fixed a problem where seg fault could occur in bcm2835_delayMicroseconds() if not running as root. Patch from Pok."

This is the related thread on the patch: https://groups.google.com/forum/#!topic/bcm2835/5VqsHKISDfQ

My best plan is then just to update the library version included in mysensors to its latest release

Will keep this thread updated.

alexvanbelle

Ok. Good news and bad news.

Good news:

Updating the library is no brainer, just copy/paste and rebuild. Signatures have not changed
After the build mysensors just works as before in root
With non root user, you don't get any segfault.

Bad news:

You still get an error (SPI requires root privilege => which is better than a segfault). Actually, the library doesn't require root for GPIO but it's still required for SPI and I2C.
I've read the code of the library and some other blog posts on this subject and currently, using SPI and I2C without root seems not to be an option. The library is initializing itself by defining memory pointers => for SPI and I2C, it's undefined and there's no clear approach to set them (root and non-root init is really different for GPIO)

I still believe updating the library is good idea: cleaner error message and get some other fixes.

I have opened a thread on the library google group to get more information on the reasons: https://groups.google.com/forum/#!topic/bcm2835/i3LvA2c38j4

Alexandre

rozpruwacz

@alexvanbelle what is your configure options? And what is i2c used for?

alexvanbelle

@rozpruwacz Sorry, it's hard for me to work on this subject during week days.
I2C is in the code, I'm just trying to find a generic solution. Currently, my raspberry has only the nrf24, therefore, I'm probably not using I2C.

My configure options are:

./configure --soc=BCM2835 --my-gateway=ethernet --my-port=5003 --my-rf24-encryption-enabled --my-rf24-channel={{ rf24_channel }}

Note that since my network is only sending information (nothing is controlled), I'm not worried about not using signatures between nodes. I enabled it in the past but then, disabled it.

RPi Gateaway: Dropping root privilege

18

11.7k

11.2k

113.1k