canonical / openthread-border-router-snap

BSD 3-Clause "New" or "Revised" License
8 stars 2 forks source link

Use confined path for posix socket #10

Closed farshidtz closed 1 year ago

farshidtz commented 1 year ago

Changes the default posix socket basename from /run/openthread-%s to /run/snap.openthead-border-router/openthread-%s. Confined snaps have access to /run/snap.$SNAP_NAME without the need for granting additional permissions.

Closes #8

Also, changes the LIBDIR and SYSCONFDIR to write the respective build files in the snapcraft part.

I've adopted the configuration from here.

prash813 commented 1 year ago

Yes @farshidtz, it is good approach to use /run/$SNAP_NAME dir for scoket path, I do not see any pitfall in it.

MonicaisHer commented 1 year ago

Testing the snap built from this PR. After starting the snap, the process of forming a Thread network failed on the web GUI due to "OpenThread daemon is not running" error. Here are the errors:

Aug 16 09:24:54 ubuntu openthread-border-router.otbr-web[332712]: otbr-web[332712]: [ERR ]-WEB-----: OpenThread daemon is not running.
Aug 16 09:24:54 ubuntu openthread-border-router.otbr-web[332712]: otbr-web[332712]: [ERR ]-WEB-----: Wpan service error: 13

Full logs:https://pastebin.ubuntu.com/p/qFzWr2Rb56/

farshidtz commented 1 year ago

Testing the snap built from this PR. After starting the snap, the process of forming a Thread network failed on the web GUI due to "OpenThread daemon is not running" error. Here are the errors:

Aug 16 09:24:54 ubuntu openthread-border-router.otbr-web[332712]: otbr-web[332712]: [ERR ]-WEB-----: OpenThread daemon is not running.
Aug 16 09:24:54 ubuntu openthread-border-router.otbr-web[332712]: otbr-web[332712]: [ERR ]-WEB-----: Wpan service error: 13

Looks like otbr-agent is running but otbr-web doesn't see it. Perhaps OPENTHREAD_POSIX_CONFIG_DAEMON_SOCKET_BASENAME cflag is not passed to the web component during build.


I verified that by injecting a line here. The socket base path was still /run/openthread-%s.

Useful for debugging: sed -i '/not running/i otbrLogErr("OT Posix Socket: %s", OPENTHREAD_POSIX_DAEMON_SOCKET_NAME);' src/web/web-service/ot_client.cpp


I tried other OTBR_OPTIONS such as -DCFLAGS=-DOPENTHREAD_POSIX_CONFIG_DAEMON_SOCKET_BASENAME and -DOPENTHREAD_POSIX_CONFIG_DAEMON_SOCKET_BASENAME and neither were effective. I believe this is because the cmake recipe for web doesn't pass CFLAGs to the compiler, unlike the one for the agent.


I added ${OT_CFLAGS} to the web's CMakeLists and this make it possible to pass the cflag. However, because the code doesn't check if OPENTHREAD_POSIX_CONFIG_DAEMON_SOCKET_BASENAME has already been defined or not, I end up with compilation error:

:: [447/451] Building CXX object src/web/CMakeFiles/otbr-web.dir/web-service/ot_client.cpp.o         
:: FAILED: src/web/CMakeFiles/otbr-web.dir/web-service/ot_client.cpp.o                               
:: /usr/bin/c++ -DBOOST_ALL_NO_LIB -DMBEDTLS_CONFIG_FILE=\"/root/parts/otbr/build/third_party/openthread/mbedtls-config.h\" -DOTBR_ENABLE_BACKBONE_ROUTER=1 -DOTBR_ENABLE_BORDER_AGENT=1 -DOTBR_ENABLE_DBUS_SERVER=1 -DOTBR_ENABLE_DNSSD_DISCOVERY_PROXY=1 -DOTBR_ENABLE_NOTIFY_UPSTART=1 -DOTBR_ENABLE_REST_SERVER=1 -DOTBR_ENABLE_SRP_ADVERTISING_PROXY=1 -DOTBR_ENABLE_VENDOR_INFRA_LINK_SELECT=0 -DOTBR_MESHCOP_SERVICE_INSTANCE_NAME="\"OpenThread BorderRouter\"" -DOTBR_PACKAGE_NAME=\"OpenThread_BorderRouter\" -DOTBR_PACKAGE_VERSION=\"0.3.0-thread-reference-20230119-dirty\" -DOTBR_PRODUCT_NAME=\"BorderRouter\" -DOTBR_SYSLOG_FACILITY_ID=LOG_USER -DOTBR_VENDOR_NAME=\"OpenThread\" -DWEB_FILE_PATH=\"/usr/share/otbr-web/frontend\" -I/root/parts/otbr/install/usr/include/jsoncpp -I/root/parts/otbr/build/third_party/Simple-web-server/repo -I/root/parts/otbr/build/include -I/root/parts/otbr/build/src -I/root/parts/otbr/build/build/otbr/third_party/openthread/repo/etc/cmake -I/root/parts/otbr/build/third_party/openthread/repo/etc/cmake -I/root/parts/otbr/build/third_party/openthread/repo/include -I/root/parts/otbr/build/third_party/openthread/repo/src/posix/platform/include -I/root/parts/otbr/build/third_party/openthread/repo/src -I/root/parts/otbr/build/third_party/openthread/repo/third_party/mbedtls/repo/include -isystem /root/parts/otbr/install/usr/include -Wall -Wextra -Werror -Wfatal-errors -Wuninitialized -Wno-missing-braces -DOPENTHREAD_POSIX_CONFIG_DAEMON_SOCKET_BASENAME=\"/run/snap.openthread-border-router/openthread-%s\" -Wno-deprecated-declarations -Wno-unused-lambda-capture -std=c++11 -MD -MT src/web/CMakeFiles/otbr-web.dir/web-service/ot_client.cpp.o -MF src/web/CMakeFiles/otbr-web.dir/web-service/ot_client.cpp.o.d -o src/web/CMakeFiles/otbr-web.dir/web-service/ot_client.cpp.o -c /root/parts/otbr/build/src/web/web-service/ot_client.cpp                                                                               
:: /root/parts/otbr/build/src/web/web-service/ot_client.cpp:53: error: "OPENTHREAD_POSIX_CONFIG_DAEMON_SOCKET_BASENAME" redefined [-Werror]                                                                
::    53 | #define OPENTHREAD_POSIX_CONFIG_DAEMON_SOCKET_BASENAME "/run/openthread-%s"               
::       |                                                                                           
:: compilation terminated due to -Wfatal-errors.                                                     
:: cc1plus: all warnings being treated as errors     

I tried setting OPENTHREAD_POSIX_DAEMON_SOCKET_NAME instead, which is defined conditionally for the web component, but got errors because the macro gets redefined elsewhere.

farshidtz commented 1 year ago

Eventually, I ended up defining another variable called WEB_CFLAGS to set the socket name and pass it as target compile options to the web component. It should be easy to upstream this change.

Alternatively, I could simply replace the default in code: sed -i 's|/run/openthread-%s|'$SNAP_RUN_OTBR'|' src/web/web-service/ot_client.cpp but that can't be up streamed!

farshidtz commented 1 year ago

I tested on Ubuntu Desktop 23.04 (amd64) by doing the following:

  1. Fresh installation of chip tool snap (latest/stable)
  2. Install the snap build from this PR, connect all the documented interfaces, configure infra-if:
    
    sudo snap remove openthread-border-router
    sudo snap install --dangerous *.snap

sudo snap connect openthread-border-router:system-etc-iproute sudo snap connect openthread-border-router:system-etc-sysctl

sudo snap connect openthread-border-router:avahi-control sudo snap connect openthread-border-router:firewall-control sudo snap connect openthread-border-router:raw-usb sudo snap connect openthread-border-router:network-control sudo snap connect openthread-border-router:bluetooth-control sudo snap connect openthread-border-router:bluez

sudo snap set openthread-border-router infra-if=<main TCP/IP interface>


3. Start the snap
4. Use [ot-ctl](https://github.com/canonical/openthread-border-router-snap/wiki/Commission-and-control-a-Matter-Thread-device-via-the-OTBR-Snap#form-a-thread-network) or GUI to form a network (tested both)
5. Query the active dataset key: `sudo openthread-border-router.ot-ctl dataset active -x`
6. Reset the Thread device's persistent storage as described [here](https://github.com/canonical/openthread-border-router-snap/pull/6#issuecomment-1647546925)
7. Continue as described [here](https://github.com/canonical/openthread-border-router-snap/wiki/Commission-and-control-a-Matter-Thread-device-via-the-OTBR-Snap#pair-the-thread-lighting-device)
farshidtz commented 1 year ago

Thank you, code looks good. I wasn't able to test the above step 7 - Pair the Thread lighting device.

I also tested it successfully on a fresh Ubuntu Server 22.04.3 (amd64) installation with avahi-daemon and bluez installed with debian packages.