Missing TLS certificate validation

[cddmp]

Versions

• Python: 3.11.8 (doesn't matter)
• OS: Kali Linux (doesn't matter)
• Pymodbus: dev
• Modbus Hardware (if used):

Pymodbus Specific

• Server: tcp with TLS - sync/async
• Client: tcp with TLS - sync/async

Description

In the current implementation, TLS certificate validation is disabled by default for both client and server role since new_sslctx.verify_mode is set to ssl.CERT_NONE. This can be seen here: https://github.com/pymodbus-dev/pymodbus/blob/dev/pymodbus/transport/transport.py#L120

The documentation for ssl.CERT_NONE can be found here: https://docs.python.org/3/library/ssl.html#ssl.CERT_NONE

I'll quote it here:

With client-side sockets, just about any cert is accepted. Validation errors, such as untrusted or expired cert, are ignored and do not abort the TLS/SSL handshake.

In server mode, no certificate is requested from the client, so the client does not send any for client cert authentication.

Therefore, authentication is basically disabled. This is a security issue, as this would enable man-in-the-middle attacks. In addition, if pymodbus is used as a server, any client could authenticate and read/write data.

[janiversen]

But that is on purpose to produce an easy way to test TLS.

To be honest I am no sure if anyone is actually using TLS for modbus, professional users of modbus have long ago switched to more effective ways of transporting Modbus messages (typically via VPN).

The api allow you to supply your own ssl, so I do not see a problem, you could argue that the documentation should state it.

However pull requests are welcome, I see a couple of options:

• remove support for TLS
• update documentation to note that for certificate validation you need to supply your own ssl (if would be very unwise to use the built in, which are open source).
• change the default (which might affect test cases)

[cddmp]

Thank you for your answer! First of all, I know myself that writing open source software is a lot of work. What I write is not meant to be rude or offensive, I just want to help improving pymodbus, though I unfortunately do not have the time to implement that myself at the moment.

You mentioned that this implementation is on purpose to make testing easier. But what would be the purpose of such a test? Assuming you implement a client with pymodbus and you use it without passing a custom SSL context. Then neither the identity of the server is verified nor can you be sure that your client certificate matches (in terms of being signed by the same CA). So the result of the test might be "everything works, I can talk to the server", but you might not even talk to the server you think you are but a rogue server instead.

I would argue that modbus over TLS is a standard and this standard describes how to use modbus over an unsecure channel in a secure way. As such, one could argue that many developers expect that this implementation is secure according to the standard "by default". Therefore, I think the best option would be to "force" the user into either having to explicitly disable TLS certificate verification for debugging or testing purpose (e.g., via an optional parameter verify, which is set to True by default) or being enforced to do it the right way.

From my understanding, the "right" way, would look like that: In the current implementation, when creating a client or a server, there is no way to pass in CA certificate(s) and it is not required since ssl.CERT_NONE is set anyway. The only option would be to pass in a custom SSL context. Therefore, I think option 3 (you mentioned) would be the best one, which is "change the current default behaviour". For this, I think another parameter would need to be introduced, which forces the user to pass in a CA certificate or bundle. From my understanding, this would then need to be passed to the SSLContext.load_verify_locations() function and ssl.CERT_REQUIRED would need to be set. This increases the effort for a developer who uses pymodbus only minimal.

[janiversen]

I do not agree with a number of your arguments.

A lot of testing takes place in the same server, so verifying tls certificates doesn't really bring anything.....but the tls-modbus format is tested.

Normally you will have both modbus server and modbus client on the same network, so again no real need to verify certificates.

If a user have a device connected directly to internet, then he/she is well advised to verify certificates...(I have not seen such a usage the lat 40 years). Typically VPN is used for such cases, with modbus tls framer.

The TLS modbus standard is quite a lot more than verify certificates, the framer is used in a number of situations be a use it reduces the protocol overhead.

Anyhow as I wrote pull requests are welcome, this is not an area we currently focus on.

[cddmp]

It is your project, of course you can do whatever you find good. But why not leaving this open, so that someone could fix it in the future?

Some last thoughts regarding your comments above: https://www.modbus.org/docs/MB-TCP-Security-v36_2021-07-30.pdf

On page 6 line 164-165 it says:

The mbap ADU which is unchanged in the mbaps profile is encapsulated in a TLS Application Protocol message as illustrated in Figure 3 mbap ADU Encapsulated in TLS.

So basically the same ADU as in mbap is transferred over TLS. The only TLS-specific part I can find is the role management (1.3.6.1.4.1.50316.802.1 roles). I didn't find the word "framer" in the specification. From having a look at the code this is an implementation specific thing to pymodbus?

Further referring to the document above, on page 20 it says:

500 R-44: mbaps Servers MUST reject a TLS Handshake where the Client has not responded to a 501 Client Certificate request with certificate.

In the current implementation, no CertificateRequest is sent to the client (see Figure 6 on page 12) due to ssl.CERT_NONE being set. Still, the client would send a certificate, but that is because you implemented that TLS client behavior.

505 R-46: mbaps Devices MUST send the entire certificate chain down to the root CA when sending 506 their certificate

Since the CA certificate cannot be passed without using a custom SSL context, it is also not send.

Regarding your argument "local networks" and "VPN": Attack surfaces should be minimized wherever possible, this is called security-in-depth. What if an attacker places a rogue device in the network (physical attacker)? With proper TLS, such a device could do no harm, as it would not be allowed to talk to the Modbus server due to a lack of a valid client TLS certificate. What if an attacker is able to reach the local Modbus network from outside by pivoting through various systems? An attacker could easily run a MitM attack, e.g., via ARP spoofing when TLS validation is not in place.

A VPN might provide a secure channel (if done correctly, see security-in-depth approach mentioned above), but it does not provide mutual authentication of Modbus devices. And it also does not offer the same security. What if an attacker is able to enter the VPN? Once the attacker has entered the VPN, the attacker sees the whole network inside the VPN. With mbap manipulation would be possible. With mbaps the attacker would first need to also get access to a modbus client with the corresponding trusted certificates to do any harm.

[janiversen]

Sorry if you did not find the word "framer" in the docs you read, that is the terminology used in modbus. Framer is the specification on how to transmit a request/response like ADU .

Regarding your comments of the VPN, signals you want to have a secure channel within an already secure channel....that is a new requirement to secure communication, that is NOT normal. A VPN is considered secure and allows you to have unencrypted channels.

But just to prove your point, one of my old clients, have a worldwide VPN networks, and have VPN within the VPN channels to privatize company communication within the global networks....so yes you can find examples of all kinds strange setups.

We have come to the conclusion, that the best way to provide a solution, that keeps the software simple, and addresses your concerns, is to modify the API by removing "certfile=", "keyfile=" and "password=". That forces the app to supply a SSLContext, and thus moves the responsibility to the app, where it rightfully belongs.

If anybody wants to make a change they do it, because they think the library should work differently, so no need for this Issue. Remark, had you stated that you wanted to solve the issue, we would have kept it open.

[janiversen]

Just ran a test....the real reason for verify_mode = None, is to allow self assigned (not registered) certificates, which is something e.g. our test suite depends heavily on.

[janiversen]

Solved permanently on dev.