RPi Gateaway: Dropping root privilege

[Julien]

I'd prefer not running as root a process that parse radio messages and do a lot of stuff on it.

The mysGateway on Raspberry Pi needs root privileges for GPIO and SPI. Using a GPIO group (rasbian package raspberrypi-sys-mods) is not enough, SPI si not working.

I suggest SUID binary and drop root privileges after mmaping /dev/mem, but I don't know the better place to do this. I suggest here in bcm2835.c, but I don't know if it really clean and tit doesn't mess something else.

chmod u+s /usr/local/bin/mysGateway

diff --git a/drivers/RPi/bcm2835.c b/drivers/RPi/bcm2835.c
index 26aa8d6..7b262b5 100644
--- a/drivers/RPi/bcm2835.c
+++ b/drivers/RPi/bcm2835.c
@@ -1358,6 +1358,10 @@ int bcm2835_init(void)
       
       /* Base of the peripherals block is mapped to VM */
       bcm2835_peripherals = mapmem("gpio", bcm2835_peripherals_size, memfd, (uint32_t)bcm2835_peripherals_base);
+
+      /* Drop root privileges if SUID */
+      setuid(getuid());
+
       if (bcm2835_peripherals == MAP_FAILED) goto exit;
       
       /* Now compute the base addresses of various peripherals, 

When doing this a bug appears in GPIO for IRQ, the delay between export and opening is not long enough. The folder folder is already created, but group permissions are not yet propagated (raspbian udev rule 99-com.rules use a chown). Running mysGateway failed half the time. I encountered errors for values under 30ms on RPi 3.

diff --git a/drivers/RPi/rpi_util.cpp b/drivers/RPi/rpi_util.cpp
index 7ee1a92..96ff78f 100644
--- a/drivers/RPi/rpi_util.cpp
+++ b/drivers/RPi/rpi_util.cpp
@@ -236,7 +236,8 @@ void rpi_util::attachInterrupt(uint8_t physPin, void (*func)(), uint8_t mode) {
        fclose(fd);
 
        // Wait a bit the system to create /sys/class/gpio/gpio<GPIO number>
-       delay(1L);
+       // Udev chown on raspbian takes time. Measured up to 30ms on RPi 3, leave good a margin
+       delay(100L);
 
        snprintf(fName, sizeof(fName), "/sys/class/gpio/gpio%d/direction", gpioPin) ;
        if ((fd = fopen (fName, "w")) == NULL) {

[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

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.