cleanup packet encrypt/descrypt

src/mesh/MeshTypes.h

@@ -12,10 +12,8 @@ typedef uint32_t PacketId; // A packet sequence number
 #define NODENUM_BROADCAST UINT32_MAX
 #define ERRNO_OK 0
 #define ERRNO_NO_INTERFACES 33
-#define ERRNO_UNKNOWN 32 // pick something that doesn't conflict with RH_ROUTER_ERROR_UNABLE_TO_DELIVER
+#define ERRNO_UNKNOWN 32  // pick something that doesn't conflict with RH_ROUTER_ERROR_UNABLE_TO_DELIVER
 #define ERRNO_DISABLED 34 // the itnerface is disabled
-#define ERRNO_TOO_LARGE 35
-#define ERRNO_NO_CHANNEL 36
 
 /**
  * the max number of hops a message can pass through, used as the default max for hop_limit in MeshPacket.
@@ -35,7 +33,7 @@ typedef int ErrorCode;
 extern Allocator<MeshPacket> &packetPool;
 
 /**
- * Most (but not always) of the time we want to treat packets 'from' the local phone (where from == 0), as if they originated on the local node.
- * If from is zero this function returns our node number instead
+ * Most (but not always) of the time we want to treat packets 'from' the local phone (where from == 0), as if they originated on
+ * the local node. If from is zero this function returns our node number instead
  */
 NodeNum getFrom(const MeshPacket *p);

src/mesh/RadioInterface.cpp

@@ -361,12 +361,8 @@ ErrorCode SimRadio::send(MeshPacket *p)
 
 void RadioInterface::deliverToReceiver(MeshPacket *p)
 {
-    assert(rxDest);
-    assert(rxDest->enqueue(p, 0)); // NOWAIT - fixme, if queue is full, delete older messages
-
-    // Nasty hack because our threading is primitive.  interfaces shouldn't need to know about routers FIXME
     if (router)
-        router->setReceivedMessage();
+        router->enqueueReceivedMessage(p);
 }
 
 /***

src/mesh/RadioInterface.h

@@ -42,7 +42,6 @@ typedef struct {
 class RadioInterface
 {
     friend class MeshRadio; // for debugging we let that class touch pool
-    PointerQueue<MeshPacket> *rxDest = NULL;
 
     CallbackObserver<RadioInterface, void *> configChangedObserver =
         CallbackObserver<RadioInterface, void *>(this, &RadioInterface::reloadConfig);
@@ -82,17 +81,11 @@ class RadioInterface
     float freq = 915.0;
 
     /** pool is the pool we will alloc our rx packets from
-     * rxDest is where we will send any rx packets, it becomes receivers responsibility to return packet to the pool
      */
     RadioInterface();
 
     virtual ~RadioInterface() {}
 
-    /**
-     * Set where to deliver received packets.  This method should only be used by the Router class
-     */
-    void setReceiver(PointerQueue<MeshPacket> *_rxDest) { rxDest = _rxDest; }
-
     /**
      * Return true if we think the board can go to sleep (i.e. our tx queue is empty, we are not sending or receiving)
      *

src/mesh/Router.cpp

@@ -65,6 +65,22 @@ int32_t Router::runOnce()
     return INT32_MAX; // Wait a long time - until we get woken for the message queue
 }
 
+/**
+ * RadioInterface calls this to queue up packets that have been received from the radio.  The router is now responsible for
+ * freeing the packet
+ */
+void Router::enqueueReceivedMessage(MeshPacket *p)
+{
+    if (fromRadioQueue.enqueue(p, 0)) { // NOWAIT - fixme, if queue is full, delete older messages
+
+        // Nasty hack because our threading is primitive.  interfaces shouldn't need to know about routers FIXME
+        setReceivedMessage();
+    } else {
+        printPacket("BUG! fromRadioQueue is full! Discarding!", p);
+        packetPool.release(p);
+    }
+}
+
 /// Generate a unique packet id
 // FIXME, move this someplace better
 PacketId generatePacketId()
@@ -130,13 +146,8 @@ ErrorCode Router::sendLocal(MeshPacket *p)
 {
     // No need to deliver externally if the destination is the local node
     if (p->to == nodeDB.getNodeNum()) {
-        if (fromRadioQueue.enqueue(p, 0)) {
-            printPacket("Enqueued local", p);
-            setReceivedMessage();
-        } else {
-            printPacket("BUG! fromRadioQueue is full! Discarding!", p);
-            packetPool.release(p);
-        }
+        printPacket("Enqueued local", p);
+        enqueueReceivedMessage(p);
         return ERRNO_OK;
     } else if (!iface) {
         // We must be sending to remote nodes also, fail if no interface found
@@ -190,36 +201,14 @@ ErrorCode Router::send(MeshPacket *p)
 
     // If the packet is not yet encrypted, do so now
     if (p->which_payloadVariant == MeshPacket_decoded_tag) {
-        static uint8_t bytes[MAX_RHPACKETLEN]; // we have to use a scratch buffer because a union
-
-        // printPacket("pre encrypt", p); // portnum valid here
-
-        size_t numbytes = pb_encode_to_bytes(bytes, sizeof(bytes), Data_fields, &p->decoded);
-
-        if (numbytes > MAX_RHPACKETLEN) {
-            abortSendAndNak(Routing_Error_TOO_LARGE, p);
-            return ERRNO_TOO_LARGE;
-        }
-
-        // printBytes("plaintext", bytes, numbytes);
-
         ChannelIndex chIndex = p->channel; // keep as a local because we are about to change it
-        auto hash = channels.setActiveByIndex(chIndex);
-        if (hash < 0) {
-            // No suitable channel could be found for sending
-            abortSendAndNak(Routing_Error_NO_CHANNEL, p);
-            return ERRNO_NO_CHANNEL;
+
+        auto encodeResult = perhapsEncode(p);
+        if (encodeResult != Routing_Error_NONE) {
+            abortSendAndNak(encodeResult, p);
+            return encodeResult; // FIXME - this isn't a valid ErrorCode
         }
 
-        // Now that we are encrypting the packet channel should be the hash (no longer the index)
-        p->channel = hash;
-        crypto->encrypt(getFrom(p), p->id, numbytes, bytes);
-
-        // Copy back into the packet and set the variant type
-        memcpy(p->encrypted.bytes, bytes, numbytes);
-        p->encrypted.size = numbytes;
-        p->which_payloadVariant = MeshPacket_encrypted_tag;
-
         if (mqtt)
             mqtt->onSend(*p, chIndex);
     }
@@ -244,7 +233,7 @@ void Router::sniffReceived(const MeshPacket *p, const Routing *c)
     // FIXME, update nodedb here for any packet that passes through us
 }
 
-bool Router::perhapsDecode(MeshPacket *p)
+bool perhapsDecode(MeshPacket *p)
 {
     if (p->which_payloadVariant == MeshPacket_decoded_tag)
         return true; // If packet was already decoded just return
@@ -285,6 +274,42 @@ bool Router::perhapsDecode(MeshPacket *p)
     return false;
 }
 
+/** Return 0 for success or a Routing_Errror code for failure
+ */
+Routing_Error perhapsEncode(MeshPacket *p)
+{
+    // If the packet is not yet encrypted, do so now
+    if (p->which_payloadVariant == MeshPacket_decoded_tag) {
+        static uint8_t bytes[MAX_RHPACKETLEN]; // we have to use a scratch buffer because a union
+
+        // printPacket("pre encrypt", p); // portnum valid here
+
+        size_t numbytes = pb_encode_to_bytes(bytes, sizeof(bytes), Data_fields, &p->decoded);
+
+        if (numbytes > MAX_RHPACKETLEN)
+            return Routing_Error_TOO_LARGE;
+
+        // printBytes("plaintext", bytes, numbytes);
+
+        ChannelIndex chIndex = p->channel; // keep as a local because we are about to change it
+        auto hash = channels.setActiveByIndex(chIndex);
+        if (hash < 0)
+            // No suitable channel could be found for sending
+            return Routing_Error_NO_CHANNEL;
+
+        // Now that we are encrypting the packet channel should be the hash (no longer the index)
+        p->channel = hash;
+        crypto->encrypt(getFrom(p), p->id, numbytes, bytes);
+
+        // Copy back into the packet and set the variant type
+        memcpy(p->encrypted.bytes, bytes, numbytes);
+        p->encrypted.size = numbytes;
+        p->which_payloadVariant = MeshPacket_encrypted_tag;
+    }
+
+    return Routing_Error_NONE;
+}
+
 NodeNum Router::getNodeNum()
 {
     return nodeDB.getNodeNum();

src/mesh/Router.h

@@ -1,12 +1,12 @@
 #pragma once
 
+#include "Channels.h"
 #include "MemoryPool.h"
 #include "MeshTypes.h"
 #include "Observer.h"
 #include "PointerQueue.h"
 #include "RadioInterface.h"
 #include "concurrency/OSThread.h"
-#include "Channels.h"
 
 /**
  * A mesh aware router that supports multiple interfaces.
@@ -22,7 +22,6 @@ class Router : protected concurrency::OSThread
     RadioInterface *iface = NULL;
 
   public:
-
     /**
      * Constructor
      *
@@ -32,11 +31,7 @@ class Router : protected concurrency::OSThread
     /**
      * Currently we only allow one interface, that may change in the future
      */
-    void addInterface(RadioInterface *_iface)
-    {
-        iface = _iface;
-        iface->setReceiver(&fromRadioQueue);
-    }
+    void addInterface(RadioInterface *_iface) { iface = _iface; }
 
     /**
      * do idle processing
@@ -53,7 +48,7 @@ class Router : protected concurrency::OSThread
     ErrorCode sendLocal(MeshPacket *p);
 
     /** Attempt to cancel a previously sent packet.  Returns true if a packet was found we could cancel */
-    bool cancelSending(NodeNum from, PacketId id);    
+    bool cancelSending(NodeNum from, PacketId id);
 
     /** Allocate and return a meshpacket which defaults as send to broadcast from the current node.
      * The returned packet is guaranteed to have a unique packet ID already assigned
@@ -69,6 +64,12 @@ class Router : protected concurrency::OSThread
      */
     void setReceivedMessage();
 
+    /**
+     * RadioInterface calls this to queue up packets that have been received from the radio.  The router is now responsible for
+     * freeing the packet
+     */
+    void enqueueReceivedMessage(MeshPacket *p);
+
   protected:
     friend class RoutingPlugin;
 
@@ -83,7 +84,7 @@ class Router : protected concurrency::OSThread
 
     /**
      * Should this incoming filter be dropped?
-     * 
+     *
      * FIXME, move this into the new RoutingPlugin and do the filtering there using the regular plugin logic
      *
      * Called immedately on receiption, before any further processing.
@@ -97,18 +98,11 @@ class Router : protected concurrency::OSThread
      */
     virtual void sniffReceived(const MeshPacket *p, const Routing *c);
 
-    /**
-     * Remove any encryption and decode the protobufs inside this packet (if necessary).
-     *
-     * @return true for success, false for corrupt packet.
-     */
-    bool perhapsDecode(MeshPacket *p);
-
     /**
      * Send an ack or a nak packet back towards whoever sent idFrom
      */
     void sendAckNak(Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex);
-    
+
   private:
     /**
      * Called from loop()
@@ -134,6 +128,17 @@ class Router : protected concurrency::OSThread
     void abortSendAndNak(Routing_Error err, MeshPacket *p);
 };
 
+/** FIXME - move this into a mesh packet class
+ * Remove any encryption and decode the protobufs inside this packet (if necessary).
+ *
+ * @return true for success, false for corrupt packet.
+ */
+bool perhapsDecode(MeshPacket *p);
+
+/** Return 0 for success or a Routing_Errror code for failure
+ */
+Routing_Error perhapsEncode(MeshPacket *p);
+
 extern Router *router;
 
 /// Generate a unique packet id

src/mqtt/MQTT.cpp

@@ -2,6 +2,7 @@
 #include "NodeDB.h"
 #include "main.h"
 #include "mesh/Channels.h"
+#include "mesh/Router.h"
 #include "mesh/generated/mqtt.pb.h"
 #include <WiFi.h>
 #include <assert.h>
@@ -26,6 +27,11 @@ void MQTT::onPublish(char *topic, byte *payload, unsigned int length)
         DEBUG_MSG("Received MQTT topic %s, len=%u\n", topic, length);
 
         // FIXME, ignore messages sent by us (requires decryption) or if we don't have the channel key
+        if (e.packet) {
+            MeshPacket *p = packetPool.allocCopy(*e.packet);
+            if (router)
+                router->enqueueReceivedMessage(p);
+        }
 
         // make sure to free both strings and the MeshPacket (passing in NULL is acceptable)
         free(e.channel_id);