Return errors for unauthorized requests or out of bound channel nums

docs/software/TODO.md

@@ -4,8 +4,12 @@ You probably don't care about this section - skip to the next one.
 
 ## before next release
 
-* document how to do remote admin
+* changing channels requires a reboot to take effect https://github.com/meshtastic/Meshtastic-device/issues/752 
+* add UI in android app to reset to defaults https://github.com/meshtastic/Meshtastic-Android/issues/263 
+* bug report with remote info request timing out
 * firmware OTA updates of is_router true nodes fails?
+* move remote admin doc from forum into git
+* ask for a documentation czar
 * DONE timestamps on oled screen are wrong - don't seem to be updating based on message rx (actually: this is expected behavior when no node on the mesh has GPS time)
 * DONE add ch-del
 * DONE channel hash suffixes are wrong on android

proto

@@ -1 +1 @@
-Subproject commit b8c0499f28f9673d1df17d04da562e30703f01cb
+Subproject commit 8a39bac88206a8aa9305ac380d150946c1796ac5

src/mesh/MeshPlugin.cpp

@@ -31,6 +31,39 @@ MeshPlugin::~MeshPlugin()
     assert(0); // FIXME - remove from list of plugins once someone needs this feature
 }
 
+MeshPacket *MeshPlugin::allocAckNak(Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex)
+{
+    Routing c = Routing_init_default;
+
+    c.error_reason = err;
+
+    // Now that we have moded sendAckNak up one level into the class heirarchy we can no longer assume we are a RoutingPlugin
+    // So we manually call pb_encode_to_bytes and specify routing port number
+    // auto p = allocDataProtobuf(c);
+    MeshPacket *p = router->allocForSending();
+    p->decoded.portnum = PortNum_ROUTING_APP;
+    p->decoded.payload.size = pb_encode_to_bytes(p->decoded.payload.bytes, sizeof(p->decoded.payload.bytes), Routing_fields, &c);
+
+    p->priority = MeshPacket_Priority_ACK;
+
+    p->hop_limit = 0; // Assume just immediate neighbors for now
+    p->to = to;
+    p->decoded.request_id = idFrom;
+    p->channel = chIndex;
+    DEBUG_MSG("Alloc an err=%d,to=0x%x,idFrom=0x%x,id=0x%x\n", err, to, idFrom, p->id);
+
+    return p;
+}
+
+MeshPacket *MeshPlugin::allocErrorResponse(Routing_Error err, const MeshPacket *p)
+{
+    auto r = allocAckNak(err, getFrom(p), p->id, p->channel);
+
+    setReplyTo(r, *p);
+
+    return r;
+}
+
 void MeshPlugin::callPlugins(const MeshPacket &mp)
 {
     // DEBUG_MSG("In call plugins\n");
@@ -56,9 +89,17 @@ void MeshPlugin::callPlugins(const MeshPacket &mp)
         bool rxChannelOk = !pi.boundChannel || (mp.from == 0) || (strcmp(ch.settings.name, pi.boundChannel) == 0);
 
         /// We only call plugins that are interested in the packet (and the message is destined to us or we are promiscious)
-        bool wantsPacket = rxChannelOk && (pi.isPromiscuous || toUs) && pi.wantPacket(&mp);
+        if (!rxChannelOk && toUs) {
-        // DEBUG_MSG("Plugin %s wantsPacket=%d\n", pi.name, wantsPacket);
+            // no one should have already replied!
-        if (wantsPacket) {
+            assert(!currentReply);
+
+            if (mp.decoded.want_response) {
+                DEBUG_MSG("packet on wrong channel, returning error\n");
+                currentReply = pi.allocErrorResponse(Routing_Error_NOT_AUTHORIZED, &mp);
+            } else
+                DEBUG_MSG("packet on wrong channel, but client didn't want response\n");
+        } else if ((pi.isPromiscuous || toUs) && pi.wantPacket(&mp)) {
+            // DEBUG_MSG("Plugin %s wantsPacket=%d\n", pi.name, wantsPacket);
             pluginFound = true;
 
             bool handled = pi.handleReceived(mp);
@@ -90,8 +131,7 @@ void MeshPlugin::callPlugins(const MeshPacket &mp)
             DEBUG_MSG("Sending response\n");
             service.sendToMesh(currentReply);
             currentReply = NULL;
-        }
+        } else {
-        else {
             // No one wanted to reply to this requst, tell the requster that happened
             DEBUG_MSG("No one responded, send a nak\n");
             routingPlugin->sendAckNak(Routing_Error_NO_RESPONSE, getFrom(&mp), mp.id, mp.channel);

src/mesh/MeshPlugin.h

@@ -1,5 +1,6 @@
 #pragma once
 
+#include "mesh/Channels.h"
 #include "mesh/MeshTypes.h"
 #include <vector>
 
@@ -90,6 +91,11 @@ class MeshPlugin
      */
     virtual bool wantUIFrame() { return false; }
 
+    MeshPacket *allocAckNak(Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex);
+
+    /// Send an error response for the specified packet.
+    MeshPacket *allocErrorResponse(Routing_Error err, const MeshPacket *p);
+
   private:
     /**
      * If any of the current chain of plugins has already sent a reply, it will be here.  This is useful to allow

src/mesh/generated/deviceonly.pb.h

@@ -125,7 +125,7 @@ extern const pb_msgdesc_t ChannelFile_msg;
 /* Maximum encoded size of messages (where known) */
 #define LegacyRadioConfig_size                   4
 #define LegacyRadioConfig_LegacyPreferences_size 2
-#define DeviceState_size                         4898
+#define DeviceState_size                         4920
 #define ChannelFile_size                         832
 
 #ifdef __cplusplus

src/mesh/generated/mesh.pb.h

@@ -57,7 +57,9 @@ typedef enum _Routing_Error {
     Routing_Error_MAX_RETRANSMIT = 5,
     Routing_Error_NO_CHANNEL = 6,
     Routing_Error_TOO_LARGE = 7,
-    Routing_Error_NO_RESPONSE = 8
+    Routing_Error_NO_RESPONSE = 8,
+    Routing_Error_BAD_REQUEST = 32,
+    Routing_Error_NOT_AUTHORIZED = 33
 } Routing_Error;
 
 typedef enum _MeshPacket_Priority {
@@ -150,6 +152,7 @@ typedef struct _MeshPacket {
     uint8_t hop_limit;
     bool want_ack;
     MeshPacket_Priority priority;
+    int32_t rx_rssi;
 } MeshPacket;
 
 typedef struct _NodeInfo {
@@ -206,8 +209,8 @@ typedef struct _ToRadio {
 #define _CriticalErrorCode_ARRAYSIZE ((CriticalErrorCode)(CriticalErrorCode_Brownout+1))
 
 #define _Routing_Error_MIN Routing_Error_NONE
-#define _Routing_Error_MAX Routing_Error_NO_RESPONSE
+#define _Routing_Error_MAX Routing_Error_NOT_AUTHORIZED
-#define _Routing_Error_ARRAYSIZE ((Routing_Error)(Routing_Error_NO_RESPONSE+1))
+#define _Routing_Error_ARRAYSIZE ((Routing_Error)(Routing_Error_NOT_AUTHORIZED+1))
 
 #define _MeshPacket_Priority_MIN MeshPacket_Priority_UNSET
 #define _MeshPacket_Priority_MAX MeshPacket_Priority_MAX
@@ -228,7 +231,7 @@ extern "C" {
 #define RouteDiscovery_init_default              {0, {0, 0, 0, 0, 0, 0, 0, 0}}
 #define Routing_init_default                     {0, {RouteDiscovery_init_default}}
 #define Data_init_default                        {_PortNum_MIN, {0, {0}}, 0, 0, 0, 0}
-#define MeshPacket_init_default                  {0, 0, 0, 0, {Data_init_default}, 0, 0, 0, 0, 0, _MeshPacket_Priority_MIN}
+#define MeshPacket_init_default                  {0, 0, 0, 0, {Data_init_default}, 0, 0, 0, 0, 0, _MeshPacket_Priority_MIN, 0}
 #define NodeInfo_init_default                    {0, false, User_init_default, false, Position_init_default, 0}
 #define MyNodeInfo_init_default                  {0, 0, 0, "", "", "", _CriticalErrorCode_MIN, 0, 0, 0, 0, 0}
 #define LogRecord_init_default                   {"", 0, "", _LogRecord_Level_MIN}
@@ -239,7 +242,7 @@ extern "C" {
 #define RouteDiscovery_init_zero                 {0, {0, 0, 0, 0, 0, 0, 0, 0}}
 #define Routing_init_zero                        {0, {RouteDiscovery_init_zero}}
 #define Data_init_zero                           {_PortNum_MIN, {0, {0}}, 0, 0, 0, 0}
-#define MeshPacket_init_zero                     {0, 0, 0, 0, {Data_init_zero}, 0, 0, 0, 0, 0, _MeshPacket_Priority_MIN}
+#define MeshPacket_init_zero                     {0, 0, 0, 0, {Data_init_zero}, 0, 0, 0, 0, 0, _MeshPacket_Priority_MIN, 0}
 #define NodeInfo_init_zero                       {0, false, User_init_zero, false, Position_init_zero, 0}
 #define MyNodeInfo_init_zero                     {0, 0, 0, "", "", "", _CriticalErrorCode_MIN, 0, 0, 0, 0, 0}
 #define LogRecord_init_zero                      {"", 0, "", _LogRecord_Level_MIN}
@@ -291,6 +294,7 @@ extern "C" {
 #define MeshPacket_hop_limit_tag                 10
 #define MeshPacket_want_ack_tag                  11
 #define MeshPacket_priority_tag                  12
+#define MeshPacket_rx_rssi_tag                   13
 #define NodeInfo_num_tag                         1
 #define NodeInfo_user_tag                        2
 #define NodeInfo_position_tag                    3
@@ -362,7 +366,8 @@ X(a, STATIC,   SINGULAR, FIXED32,  rx_time,           7) \
 X(a, STATIC,   SINGULAR, FLOAT,    rx_snr,            8) \
 X(a, STATIC,   SINGULAR, UINT32,   hop_limit,        10) \
 X(a, STATIC,   SINGULAR, BOOL,     want_ack,         11) \
-X(a, STATIC,   SINGULAR, UENUM,    priority,         12)
+X(a, STATIC,   SINGULAR, UENUM,    priority,         12) \
+X(a, STATIC,   SINGULAR, INT32,    rx_rssi,          13)
 #define MeshPacket_CALLBACK NULL
 #define MeshPacket_DEFAULT NULL
 #define MeshPacket_payloadVariant_decoded_MSGTYPE Data
@@ -454,12 +459,12 @@ extern const pb_msgdesc_t ToRadio_msg;
 #define RouteDiscovery_size                      40
 #define Routing_size                             42
 #define Data_size                                260
-#define MeshPacket_size                          298
+#define MeshPacket_size                          309
 #define NodeInfo_size                            126
 #define MyNodeInfo_size                          89
 #define LogRecord_size                           81
-#define FromRadio_size                           307
+#define FromRadio_size                           318
-#define ToRadio_size                             301
+#define ToRadio_size                             312
 
 #ifdef __cplusplus
 } /* extern "C" */

src/plugins/AdminPlugin.cpp

@@ -56,13 +56,21 @@ bool AdminPlugin::handleReceivedProtobuf(const MeshPacket &mp, const AdminMessag
 
     case AdminMessage_set_channel_tag:
         DEBUG_MSG("Client is setting channel %d\n", r->set_channel.index);
-        handleSetChannel(r->set_channel);
+        if (r->set_channel.index < 0 || r->set_channel.index >= MAX_NUM_CHANNELS)
+            reply = allocErrorResponse(Routing_Error_BAD_REQUEST, &mp);
+        else
+            handleSetChannel(r->set_channel);
         break;
 
-    case AdminMessage_get_channel_request_tag:
+    case AdminMessage_get_channel_request_tag: {
-        DEBUG_MSG("Client is getting channel %d\n", r->get_channel_request - 1);
+        uint32_t i = r->get_channel_request - 1;
-        handleGetChannel(mp, r->get_channel_request - 1);
+        DEBUG_MSG("Client is getting channel %d\n", i);
+        if (i >= MAX_NUM_CHANNELS)
+            reply = allocErrorResponse(Routing_Error_BAD_REQUEST, &mp);
+        else
+            handleGetChannel(mp, i);
         break;
+    }
 
     case AdminMessage_get_radio_request_tag:
         DEBUG_MSG("Client is getting radio\n");

src/plugins/RoutingPlugin.cpp

@@ -22,13 +22,12 @@ bool RoutingPlugin::handleReceivedProtobuf(const MeshPacket &mp, const Routing *
     return false; // Let others look at this message also if they want
 }
 
-
 MeshPacket *RoutingPlugin::allocReply()
 {
     assert(currentRequest);
 
     // We only consider making replies if the request was a legit routing packet (not just something we were sniffing)
-    if(currentRequest->decoded.portnum == PortNum_ROUTING_APP) {
+    if (currentRequest->decoded.portnum == PortNum_ROUTING_APP) {
         assert(0); // 1.2 refactoring fixme, Not sure if anything needs this yet?
         // return allocDataProtobuf(u);
     }
@@ -37,26 +36,12 @@ MeshPacket *RoutingPlugin::allocReply()
 
 void RoutingPlugin::sendAckNak(Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex)
 {
-    Routing c = Routing_init_default;
+    auto p = allocAckNak(err, to, idFrom, chIndex);
-    
-    c.error_reason = err;
-
-    auto p = allocDataProtobuf(c);
-    p->priority = MeshPacket_Priority_ACK;
-
-    p->hop_limit = 0; // Assume just immediate neighbors for now
-    p->to = to;
-    p->decoded.request_id = idFrom;
-    p->channel = chIndex;
-    DEBUG_MSG("Sending an err=%d,to=0x%x,idFrom=0x%x,id=0x%x\n", err, to, idFrom, p->id);
 
     router->sendLocal(p); // we sometimes send directly to the local node
 }
 
-RoutingPlugin::RoutingPlugin()
+RoutingPlugin::RoutingPlugin() : ProtobufPlugin("routing", PortNum_ROUTING_APP, Routing_fields)
-    : ProtobufPlugin("routing", PortNum_ROUTING_APP, Routing_fields)
 {
     isPromiscuous = true;
 }
-
-