Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mbusd - read only option #117

Open
melicherm opened this issue Nov 28, 2024 · 14 comments
Open

mbusd - read only option #117

melicherm opened this issue Nov 28, 2024 · 14 comments

Comments

@melicherm
Copy link

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!

@mStirner
Copy link

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?

@melicherm
Copy link
Author

melicherm commented Nov 28, 2024

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
Copy link
Author

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

@mStirner
Copy link

mStirner commented Dec 10, 2024

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.

@lacithehun
Copy link

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

@mStirner
Copy link

mStirner commented Jan 5, 2025

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

@lacithehun
Copy link

lacithehun commented Jan 6, 2025

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 {

@mStirner
Copy link

mStirner commented Jan 6, 2025

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

@lacithehun
Copy link

lacithehun commented Jan 8, 2025

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)

@mStirner
Copy link

mStirner commented Jan 8, 2025

I have seen you created a PR yesterday: #119
Why did you close it if you can create PRs?

@lacithehun
Copy link

lacithehun commented Jan 8, 2025 via email

@3cky
Copy link
Owner

3cky commented Jan 10, 2025

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?

@mStirner
Copy link

mStirner commented Jan 10, 2025

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.

@lacithehun
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants