mbusd icon indicating copy to clipboard operation
mbusd copied to clipboard

mbusd - read only option

Open melicherm opened this issue 1 year ago • 14 comments

Hello dear community, with the aspect of security, is there the possibility to allow only read operations through the gateway?

or could this feature be added with a context switch -ro ?

One could modify the code and build the app without write features, but this could be interesting.... to have a hardened security feature.

Thanks!

melicherm avatar Nov 28 '24 09:11 melicherm

How should that work? To read a modbus register/value, you have to write on the Bus.

The only "read only" modus i can image is a simple monitoring of the messages on the Bus/USB Adapter.

Can you elaborate more what you expect?

mStirner avatar Nov 28 '24 12:11 mStirner

Hello, good point.

The use case:

We have a network of around 20 frequency controllers behind an Modbus RTU to Modbus IP gateway running mbusd. We use Zabbix to monitor the values over the gateway with the use of ,,modbus_read" function. Obwiously it's a separated dedicated network, but still...

example: modbus_read[{HOST.CONN}:{$MODBUS_PORT},{$MODBUS_SLAVE},1,4,uint16]

modbus_read[192.168.1.10:502,20,1,4,uint16]

But we are hardening our security and we would like to reject "write" commands coming to the IP side. So no Write command could be directed to RTU side from IP side. Hope i have explained it clearly -> the use case.

Buf if there would be a function modbus_write[] this could bring security issues -> someone could turn ON or OFF devices, etc... we have seen such articles in the news :)

melicherm avatar Nov 28 '24 13:11 melicherm

@mStirner - Any thoughts on this? How to mitigate possible write command coming over IP to mbusd?

melicherm avatar Dec 10 '24 09:12 melicherm

afaik there is no build in method. There are two possible solutions.

  1. Restrict the access to mbusd via firewall rules
  2. write a custom gateway that checks the modbus function code

The second solution could be done in python or node, just a few lines of code:

const { createConnection, Server } = require("net");

const server = new Server();

server.on("connection", (socket) => {

    let connected = false;

    // listen for incoming data once (from zabixx)
    // if function code is valid, the client & mbusd streams are piped below
    socket.once("data", (chunk) => {

        // not sure which byte is function code in a Modbus IP packet
        // chat gpt said byte 8
        // example: 00 01 00 00 00 06 01 03 00 00 00 02
        /*
        - Transaction ID: 00 01 (2 Bytes)
        - Protocol ID: 00 00 (2 Bytes)
        - Length: 00 06 (6 Bytes folgen)
        - Unit Identifier: 01 (1 Byte)
        - Function Code: 03 (Read Holding Registers)
        - Data: 00 00 00 02 (Startadresse 0x0000, 2 Register read)
        */

        // check if byte 8 is either 1, 2 or 3
        // and connect to mbusd + forward data
        // TODO: not sure if need to hex formated (0x01) or plain int
        if ([1, 2, 3].includes(chunk[7])) {
            if (!connected) {

                // connect to mbusd
                // IMPPORTANT: configure mbusd to listen only on the loopback interface!!!!
                let client = createConnection(502, "127.0.0.1", () => {
                    connected = true;
                });

                client.pipe(socket); // pipe data from mbusd to client
                socket.pipe(client); // pipe data from client to mbusd

                client.write(chunk); // write initial message chunk to mbusd

            }
        } else {

            // wrong modbus function code
            // drop connection
            socket.end();

        }

    });

});

// "expose" our gateway instead of mbusd to the network
// mbusd should only listen on the loopback interface
server.listen(502, "192.168.1.123", (err) => {
    console.log(err || "Modbus gateway listening on tcp://192.168.1.123:502");
});

Not tested, just put quickly together. The code above is not perfect, it missing error handling, and im not sure if the stream pipeing is even needed because of the low size of modbus messages, this could be even done with a single "data"/"chunk" event, which should make the code above even smaller/simpler.

mStirner avatar Dec 10 '24 11:12 mStirner

How should that work? To read a modbus register/value, you have to write on the Bus.

The only "read only" modus i can image is a simple monitoring of the messages on the Bus/USB Adapter.

Can you elaborate more what you expect?

With the Modbus read-only option should only pass modbus messages which contain function code 1 ... 4.(read coils / registers). These are the so called read only requests. This way there would be no way to accidentally or intentioonally change a modbus rtu device's settings from the tcp/ip line. It would be really helpful if this could be a command line option (for. exmaple the /ro switch as mentioned above).

Thanks

lacithehun avatar Jan 05 '25 18:01 lacithehun

Feel free to create a pull request if the approach above does not suit your needs.

mStirner avatar Jan 05 '25 19:01 mStirner

Feel free to create a pull request if the approach above does not suit your needs.

Creating pull request doesn't work for unknown reason. But the solution would be:

  • main.c: insert after line 389:
      case 'o': /* -o parameter for readonly mode (only Function codes 1...4 are transmitted to serial side)*/
        cfg.readonlyoption = 1;
        break;
  • cfg.h: insert after line 82: int readonlyoption;

  • cfg.c: insert after line 82: cfg.readonlyoption=0;

  • conn.c: insert after line 836: }

  • conn.c: insert after line 812:

                if (cfg.readonlyoption)
                {
                  switch (fc)
                  {
                    case 1: /* Read Coil Status */
                    case 2: /* Read Input Status */
                    case 3: /* Read Holding Registers */
                    case 4: /* Read Input Registers */
                    {
                      /* set data length for requests with fixed length */
                      conn_fix_request_header_len(curconn, 6);
                      state_conn_set(curconn, CONN_RQST_TAIL);
                    }
                    break;
                    default:
                      /* unknown function code, closing connection */
                      curconn = conn_close(curconn);
                    break;
                    }
                } else {

lacithehun avatar Jan 06 '25 09:01 lacithehun

@lacithehun Note that im not the maintainer of mbusd. Should i create the PR for you?

mStirner avatar Jan 06 '25 10:01 mStirner

Yes, please. With replacing line 205 in main.c with "p:s:m:A:P:C:N:R:W:T:c:b:o")) != RC_ERR)

lacithehun avatar Jan 08 '25 11:01 lacithehun

I have seen you created a PR yesterday: https://github.com/3cky/mbusd/pull/119 Why did you close it if you can create PRs?

mStirner avatar Jan 08 '25 11:01 mStirner

I got upset because I couldn't make a pull request.Then I found that a fork is needed first, so I forked.Made the changes, but only could make PRs file-by-file.Despite the system told that the branches will be merged automatically, it didn't happen.I neither have time, nor have help, so gave up and asked your assistance

lacithehun avatar Jan 08 '25 18:01 lacithehun

Hello all,

I like the idea as a whole, but still not sure about the implementation. Should mbusd in read-only mode ignore write function codes completely and trigger the timeout in Modbus TCP master, or send some exception code in the response?

3cky avatar Jan 10 '25 10:01 3cky

or send some exception code in the response?

Is it not the case, that exceptions codes triggerd/apply/thrown only by the underlayiing modbus device? How could you differ between mbusd send the exception or the modbus device?

If currently mbusd does not send any modbus expections, i would personally prefer a tcp/ip based approach.

EDIT:

I just seen, that there is a "Gateway Error", The gateway is overloaded or not correctly configured.: grafik https://product-help.schneider-electric.com/ED/ES_Power/NSX_Modbus_Guide/EDMS/DOCA0091EN/DOCA0091xx/NSX_MB_Modbus_Protocol/NSX_MB_Modbus_Protocol-5.htm

I think that could work, as it differ clearly from other exepctions that may the modbus device could throw.

mStirner avatar Jan 10 '25 11:01 mStirner

The simplest (and) way is to ignore the write function (timeout method). The more elegant way is to give an exception with code = 0x01 (illegal function), this would give the modbus master some information about the source of error. Since this software is about to make a transparent connection between serial modbus RTU devices and the TCP line, giving a gateway type exception can be misleading. When someone using the read-only option, they do it intentionally to protect the serial devices from wiring into them, so normally there is no try to use writing functions at all.

For protection purposes the timeout method is sufficent, because normally the cyclical readings are passed to the serial devices, so it can be easily checked if the are up or not.

lacithehun avatar Jan 10 '25 12:01 lacithehun