Repository navigation
feat: --proxy and --source-addr on every connecting command; go-s7comm v0.15.0 - #23
Merged
Merged
Conversation
…m v0.14.0 --proxy socks5://host:port (S7COMMCTL_PROXY) connects through a SOCKS5 proxy (RFC 1928 CONNECT, no authentication; socks5h:// resolves names at the proxy), for example an SSH jump host into a plant network. --source-addr <ip> (S7COMMCTL_SOURCE_ADDR) binds the local address on multi-homed hosts. Both go through client.WithDialer and, with go-s7comm v0.14.0, the dialer of Discover, ProbeRackSlots and CompareRead, so discover, probe rackslot, compare read and profile are covered too. Invalid values are usage errors (exit 2); a refusing proxy is a connection failure naming the proxy and its reply. internal/app/netdial holds the dialer and a hand-written SOCKS5 client (no new dependency); internal/socks5test a scripted proxy for tests. discover is now tested end to end with the real client.Discover: the test proxy forwards every CONNECT to the fake PLC. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- go-s7comm v0.15.0: compare read's --timeout now bounds the read too (listed under Breaking changes; README notes it). - netdial: after the SOCKS5 handshake, check ctx on every success instead of in a branch only a timing race reached (Codecov patch gap). - RELEASE.md v0.7.0: previous release is v0.6.1 (v0.6.2 never existed). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
boeboe
changed the title
Closes the last Codecov patch gaps on #23: Start's listen failure is tested through a replaceable listen and a recording testing.TB, and a failed reply write no longer needs its own branch (the copies end on the broken conn). socks5test and netdial are at 100 %. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Nov 9, 2026
boeboe
added a commit
to otfabric/modbusctl
that referenced
this pull request
Nov 9, 2026
…s v1.14.2 (#32) * feat: --proxy and --source-addr on every connecting command; go-modbus v1.14.1 Every client command (sunspec included) and discover take --proxy (socks5:// or socks5h://, RFC 1928 CONNECT, no auth) and --source-addr, set as modbus.Config.Dialer. --proxy applies to tcp, tcp+tls, rtuovertcp and asciiovertcp; with udp or rtuoverudp it is invalid input, as is either flag with rtu:// or ascii://, and discover --resolve-mac with --proxy. --source-addr binds a UDP address for udp transports. TLS verification stays against the URL host. The SOCKS5 dialer and the scripted test proxy are ported from otfabric/s7commctl#23. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: go-modbus v1.14.2 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two flags on every command that connects to a PLC set how the TCP connection is opened:
--proxy socks5://host:port(S7COMMCTL_PROXY) connects through a SOCKS5 proxy, for example an SSH jump host into a plant network (ssh -D 1080 jumphost). It speaks RFC 1928 CONNECT with no authentication.socks5h://sends host names to the proxy to resolve.--source-addr <ip>(S7COMMCTL_SOURCE_ADDR) connects from that local address, on hosts with separate OT and office interfaces.They apply to
identify,read,write,list,block,blocks,diagnostic,fingerprint,scan,record,probe rackslot,compare read,profileanddiscover. They use go-s7comm v0.15.0:client.WithDialer(otfabric/go-s7comm#27), the scan helpers' dialer (otfabric/go-s7comm#28), andCompareReadbounding its read byTimeout(otfabric/go-s7comm#29).Breaking (v0 minor):
compare read --timeoutnow bounds each candidate's read too. A PLC that goes silent after setup used to hang the command; it now times out, and so does a PLC that answers more slowly than--timeout. RELEASE.md## v0.7.0(previous release v0.6.1) lists this under Breaking changes.Behaviour
--source-addrthat is not an IP.socks5: connection not allowed by ruleset.internal/app/netdialis about 150 lines.Not changed: the exposure warning
During the review I suggested moving the public-IP warning into the dialer, so it would check the resolved address. That is unnecessary:
--ipaccepts only an IP literal (validate.ValidateIP), so the typed value already is the resolved address. Through a proxy the warning still checks the PLC's address. SECURITY.md says so.Tests
internal/app/netdial(99.0%): flag validation; source-address binding; CONNECT for IPv4, IPv6 and host names (socks5resolves locally,socks5hsends the name); every reply code and handshake failure (scripted overnet.Pipe); a silent proxy against the deadline; cancelinternal/socks5test(97.8%): the scripted test proxy itselfcmd/s7commctl:identify,probe rackslot,compare readandprofilethrough the proxy to the fake PLCdiscoverend to end with the realclient.Discover: the proxy forwards every CONNECT to192.0.2.0/30:102to the fake PLC. Before, the tests had to replaceclient.Discover.--source-addr; invalid values as usage errors on every command familyinternal/app/config: env variables and the options and probe request that the flags producemake check(per-package coverage floor 95%, docs-check) andmake e2egreen.Docs
README (new "Reaching the PLC: proxy and source address" section and env list), docs/commands.md (both flags on all 16 connecting commands), SECURITY.md, RELEASE.md v0.7.0.
Codecov patch gap: in
netdial, the check ofctxafter the SOCKS5 handshake now runs on every successful handshake. Before, it sat in a branch that only a timing race reached. The behaviour is the same: a ctx that ends as the handshake finishes fails the dial.🤖 Generated with Claude Code