Project

General

Profile

Actions
Copy link

Feature #1006

closed

Drop nodejs - it doesn't build properly anymore, replace with mongoose

Added by Hammel 9 months ago. Updated 2 months ago.

Status:
Closed
Priority:
Immediate
Assignee:
Hammel
Category:
Monitor
Target version:
PiBox - 3.0 - Corrino
Start date:
21 Jul 2023
Due date:
% Done:

100%

Estimated time:
Severity:
01 - Critical

Description

Nodejs doesn't build properly with the Corinno (v3.0) toolchain and buildroot. I can either bump it back to the version used in 2.0 or drop it in favor of mongoose.

Since nodejs is a server side thing, I can still use javascript on the client side. I just don't need nodejs on the server.

Actions
Copy link
 #1

Updated by Hammel 6 months ago

Mongoose is not being used with PiBox, but lighttpd is. See if that works as a drop in replacement for nodejs.

Actions
Copy link
 #2

Updated by Hammel 6 months ago

  • Target version changed from 2.0.0 - Dors Venabili to 3.0 - Corrino
Actions
Copy link
 #3

Updated by Hammel 5 months ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 10

The problem here will be that the server side is written in JS using Node. And there are a ton of Node modules in use, likely for things like encrypting and decrypting.

I need to switch to a PHP only implementation on the server side. Client side JS is fine. imwww is implemented like this for configuring the AP and Client Wifi connections (which is what PiNet on PiSentry and PiStore use).

So before I can continue work on sensors for Ironman, I need to port imrest from Node/JS to PHP.

Actions
Copy link
 #4

Updated by Hammel 3 months ago

After looking at the imrest Node.js code I found that it should be very straight-forward to convert these to C functions and embed them in a Mongoose-based server.

Actions
Copy link
 #5

Updated by Hammel 3 months ago

  • Severity changed from 03 - Medium to 01 - Critical
Actions
Copy link
 #6

Updated by Hammel 3 months ago

I've created an app project based on mongoose for this: imrest. It currently builds a simple HTTP server that can responsd to the "api/hi" REST API. This is enough of an example to start porting the old imrest nodejs code to this.

I've started porting miot.js, which is the front end to the old server. This is just a router for the various APIs.

Once this is implemented with stubs for the APIs, I can move to the next js module. This is the order I should port them.

Actions
Copy link
 #7

Updated by Hammel 2 months ago

  • Subject changed from Drop nodejs - it doesn't build properly anymore to Drop nodejs - it doesn't build properly anymore, replace with mongoose
  • % Done changed from 10 to 60
Actions
Copy link
 #8

Updated by Hammel 2 months ago

  • % Done changed from 60 to 80

The port is essentially complete. The unit tests are written but not fully working (which means there are bugs in the port).

Tests 3, 4 and 5 (see unittest.sh) generate valgrind errors. I need to fix those next. Then write some negative tests and add some configuration options to make the tests more flexible.

Actually, running the tests one at a time and restarting valgrind on each test shows no errors. But if I run tests 1, 2 and 3 without restarting valgrind I get the following:

==3962864== Invalid write of size 1
==3962864==    at 0x10DAD1: cryptoDoDecode (crypto.c:92)
==3962864==    by 0x10E0C2: cryptoDecrypt (crypto.c:285)
==3962864==    by 0x10F02C: deviceListDevices (device.c:441)
==3962864==    by 0x10BB99: handlePost (imrest.c:165)
==3962864==    by 0x10BB99: frontEnd (imrest.c:265)
==3962864==    by 0x10BB99: frontEnd (imrest.c:248)
==3962864==    by 0x1190EB: mg_call (mongoose.c:1351)
==3962864==    by 0x1190EB: http_cb.part.0 (mongoose.c:3231)
==3962864==    by 0x118839: mg_call (mongoose.c:1352)
==3962864==    by 0x118839: iolog (mongoose.c:7128)
==3962864==    by 0x11F833: read_conn (mongoose.c:7316)
==3962864==    by 0x11F833: mg_mgr_poll (mongoose.c:7724)
==3962864==    by 0x10B75C: main (imrest.c:340)
==3962864==  Address 0x5b61e40 is 0 bytes after a block of size 32 alloc'd
==3962864==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==3962864==    by 0x10D781: unbase64 (base64.c:258)
==3962864==    by 0x10DA09: cryptoDoDecode (crypto.c:74)
==3962864==    by 0x10E0C2: cryptoDecrypt (crypto.c:285)
==3962864==    by 0x10F02C: deviceListDevices (device.c:441)
==3962864==    by 0x10BB99: handlePost (imrest.c:165)
==3962864==    by 0x10BB99: frontEnd (imrest.c:265)
==3962864==    by 0x10BB99: frontEnd (imrest.c:248)
==3962864==    by 0x1190EB: mg_call (mongoose.c:1351)
==3962864==    by 0x1190EB: http_cb.part.0 (mongoose.c:3231)
==3962864==    by 0x118839: mg_call (mongoose.c:1352)
==3962864==    by 0x118839: iolog (mongoose.c:7128)
==3962864==    by 0x11F833: read_conn (mongoose.c:7316)
==3962864==    by 0x11F833: mg_mgr_poll (mongoose.c:7724)
==3962864==    by 0x10B75C: main (imrest.c:340)
==3962864==
==3962864== Invalid read of size 1
==3962864==    at 0x483BC94: strlen (vg_replace_strmem.c:459)
==3962864==    by 0x4AF3B77: __vfprintf_internal (vfprintf-internal.c:1647)
==3962864==    by 0x4B04605: __vsnprintf_internal (vsnprintf.c:114)
==3962864==    by 0x49AB240: _piboxLogger (log.c:262)
==3962864==    by 0x10DAEF: cryptoDoDecode (crypto.c:94)
==3962864==    by 0x10E0C2: cryptoDecrypt (crypto.c:285)
==3962864==    by 0x10F02C: deviceListDevices (device.c:441)
==3962864==    by 0x10BB99: handlePost (imrest.c:165)
==3962864==    by 0x10BB99: frontEnd (imrest.c:265)
==3962864==    by 0x10BB99: frontEnd (imrest.c:248)
==3962864==    by 0x1190EB: mg_call (mongoose.c:1351)
==3962864==    by 0x1190EB: http_cb.part.0 (mongoose.c:3231)
==3962864==    by 0x118839: mg_call (mongoose.c:1352)
==3962864==    by 0x118839: iolog (mongoose.c:7128)
==3962864==    by 0x11F833: read_conn (mongoose.c:7316)
==3962864==    by 0x11F833: mg_mgr_poll (mongoose.c:7724)
==3962864==    by 0x10B75C: main (imrest.c:340)
==3962864==  Address 0x5b61e40 is 0 bytes after a block of size 32 alloc'd
==3962864==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==3962864==    by 0x10D781: unbase64 (base64.c:258)
==3962864==    by 0x10DA09: cryptoDoDecode (crypto.c:74)
==3962864==    by 0x10E0C2: cryptoDecrypt (crypto.c:285)
==3962864==    by 0x10F02C: deviceListDevices (device.c:441)
==3962864==    by 0x10BB99: handlePost (imrest.c:165)
==3962864==    by 0x10BB99: frontEnd (imrest.c:265)
==3962864==    by 0x10BB99: frontEnd (imrest.c:248)
==3962864==    by 0x1190EB: mg_call (mongoose.c:1351)
==3962864==    by 0x1190EB: http_cb.part.0 (mongoose.c:3231)
==3962864==    by 0x118839: mg_call (mongoose.c:1352)
==3962864==    by 0x118839: iolog (mongoose.c:7128)
==3962864==    by 0x11F833: read_conn (mongoose.c:7316)
==3962864==    by 0x11F833: mg_mgr_poll (mongoose.c:7724)
==3962864==    by 0x10B75C: main (imrest.c:340)

I'm pretty sure this is an off-by-1 allocation problem for a buffer. Possibly more than 1. Note that the problem only shows up on multiple API calls in a single session.

So far, I've tested
  • 1
  • 1, 2
  • 1, 2, 3

And each unit test was also run by itself. I'll need to continue running these tests, adding one more to the list each time. Then try running them in varying order.

Actions
Copy link
 #9

Updated by Hammel 2 months ago

While much of this "just worked", I'm not actually using the encrypting functions incorrectly. The IV is okay because I truncate a UUID for that. But the message must be in a buffer that is a multiple of 16, and that buffer must be pkcs7 padded before I attempt encrypting. Decrypting is the same but shouldn't be a problem since the buffers will be the same as the encrypted one.

I need to fix cryptoEncrypt() and cryptoDoDecode() to do these correctly. They sort of do it, but not quite right.

Actions
Copy link
 #10

Updated by Hammel 2 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 80 to 100

Okay, I was actually doing things correctly. But I had a bug in where I did matching of POST's URIs. Fixing that seemed to clear up all valgrind errors.

Then I found bugs in cross builds and packaging. I've now fixed those.

There are updates to the lightsw Arduino and Jarvis code required to match minor changes to the C port of imrest, this is essentially done. I'll debug it with Jarvis and sensors later, though I think the unit tests are good enough to validate proper operation (but not necessarily handling of bad inputs).

The metabuild has been tested with this new package.

So this is essentially done for now. New issues will be opened as bugs are found.

Code committed and pushed.

Closing issue.

Actions
Copy link

Also available in: Atom PDF