EC: skip ZLIB compression on local peers - #728
Conversation
amuled unconditionally negotiates EC_FLAG_ZLIB whenever the client advertises EC_TAG_CAN_ZLIB. On loopback or LAN links this is pure overhead: the EC payload (full library tag tree) deflates only ~2-3x but adds a ~37 MB/s ceiling per direction on a typical CPU, well below gigabit LAN line rate and meaningless on lo where there is no transit cost at all. Server-side gate: when the peer IP is 0.0.0.0, loopback (127/8), an RFC1918 LAN range (10/8, 172.16/12, 192.168/16), or RFC3927 link-local (169.254/16), don't set EC_FLAG_ZLIB on the response side even if the client offered it. This covers all client versions (amulegui, amuleweb, amulecmd, third-party) without an EC pref roundtrip and without a client-side rebuild. Adds IsLoopbackIP() and IsLinkLocalIP() helpers next to IsLanIP() in NetworkFunctions; same anti-host order convention. Public WAN clients continue to negotiate ZLIB as before.
…56 MB Initial commit gated ZLIB at auth time: if the peer is on loopback / RFC1918 LAN / RFC3927 link-local, the server cleared EC_FLAG_ZLIB from m_my_flags and every subsequent response went uncompressed. That broke `amulecmd "show shared"` (and the initial amulegui / amuleweb INC_UPDATE) on big libraries (issue amule-project#713 → Stoatwblr's 91k- file shareset): the uncompressed response is 18-45 MB, and CECSocket::ReadHeader has a hard 16 MB cap that closes the socket on "oversize" packets. amulecmd would connect, send `show shared`, then the daemon's response would arrive with `m_curr_packet_len` > 16 MB, the client's ReadHeader would CloseSocket(), and the shell loop would exit with "Ok, exiting…" — the symptom Stoatwblr reported. Two complementary changes: 1. `ReadHeader` raises its cap to 256 MB post-auth. The pre-auth path keeps the historical 16 MB limit so a single bogus header from a not-yet-authorized peer can still only allocate a small bounded buffer (the existing IsAuthorized() guard on resize already enforces this). Post-auth we trust the peer enough to accept a larger announced size. 2. The local-peer ZLIB bypass moves from auth-time on m_my_flags to per-packet in CECSocket::WritePacket. The server now always negotiates ZLIB normally with the client (whatever the client advertised), then on each outbound packet: * peer is local AND packet <= 256 MB -> send uncompressed * peer is local AND packet > 256 MB -> fall back to ZLIB so wire size stays under the receiver gate * peer is remote -> ZLIB as before This preserves the perf win (no deflate/inflate on loopback or LAN for the vast majority of responses) without ever producing a wire packet the receiver will reject. SetLocalPeer(bool) on CECSocket records the locality decision the server makes at auth time; ExternalConn's Authenticate calls it unconditionally with the loopback/LAN/link-local result. Client-side code is unchanged — the wire flag bits on each packet already control inflate vs raw-read.
|
I'm a bit late for this PR, but I think this approach is not optimal: IMHO, the assumption that all LAN IP are high speed physical networks is too optimistic. EC protocol is not a secure protocol: it has no authentication and data travel as plaintext, only the password is hashed, but without authentication that security is very weak. The logical security mechanism when accessing from a remote location will be to use a VPN like WireGuard. This remote access through the public Internet may be quite slow. Also, when using a VPN, the client will look like a LAN IP to the server, thus automatically disabling zlib. Actually, since EC is an insecure protocol, I don't expect anyone to connect using a public IP, hence being identified as "remote" by amuled. Please note that WireGuard provides no compression! If compression is required, that must be provided by others protocol layers. In my opinion, a better solution will keep zlib disabled for local peers as default, as this PR did. But it will give users the option to activate it at will. Ideally, the compression option will be chosen by the client application, not by the server, since the client knows better how is it connecting to the server. A possible approach could move the local peer decision to the client, and advertise or not zlib capability from the client side depending on the server IP. A command line parameter on the client would allow to bypass the local peers default. But if zlib is deactivated, that could break the >256MB packets that are currently always compressed, so this needs further thinking (maybe split the packet, or compress even if zlib support not advertised?) Opinions? |
|
Two reframings that change the right fix here:
Net result: WireGuard-as-LAN works correctly (client knows its dialed IP is the VPN endpoint, not really "local" in transit terms; user can also explicitly |
|
Filed #840 — the client-side preference-tag design we discussed above, with the |
#728 made the daemon skip ZLIB on per-packet basis when the EC peer's IP looked "local" (loopback / RFC1918 / link-local). The check lived on the server side and was based on the peer-IP it saw on the socket, which misclassifies common topologies — most importantly WireGuard / Tailscale tunnel endpoints that resolve to RFC1918 addresses but whose underlying transit is the public Internet. Those connections lost ZLIB compression silently and paid the wire-size cost. Move the locality decision to the client, which is the only side that knows the IP it actually dialed: - New tag EC_TAG_PREFER_NO_ZLIB (0x0014) — a *preference*, not a capability. EC_TAG_CAN_ZLIB stays as the always-on capability so the oversize-payload fallback (>EC_MAX_UNCOMPRESSED forces ZLIB regardless of preference, see ECSocket.cpp:797) still kicks in when the receiver-gate would otherwise reject the packet. - CRemoteConnect::ConnectToCore resolves the dialed host and sets m_preferNoZlib when it points at a loopback / RFC1918 / link-local address. CECLoginPacket takes a new preferNoZlib param and emits the tag when set. - ExternalConn drops the peer-IP block in CECServerSocket:: Authenticate; SetLocalPeer(true) now fires only when the client sent EC_TAG_PREFER_NO_ZLIB. Old clients omit the tag and get the pre-#728 always-ZLIB behaviour. Old servers ignore the unknown tag and treat the connection as remote (also pre-#728). - User override: amulegui dialog gets a "Force ZLIB compression" checkbox (persists under "Remember those settings" to /EC/ForceZLIB); amulecmd / amuleweb get --force-zlib and the same config key in remote.conf. When set, the locality check is skipped and the tag is never sent, so the server compresses as if we were remote. Use case: VPN-as-LAN topologies where the dialed IP looks local but transit isn't. ECSocket.cpp's per-packet send logic (m_isLocalPeer / EC_FLAG_ZLIB / oversize fallback) is unchanged; only the meaning of m_isLocalPeer's input changes. Closes the discussion thread on #728 (danim7 raised the WireGuard case).
…ndex Adds 55+ merged PRs to the 3.0.0 changelog since the last update (amule-project#747, 2026-05-27). Narrative additions cover: - Packaging: expanded the top list to include the macOS per-arch .app bundles and the Windows NSIS installer alongside the existing AppImage / Flatpak / .dmg / .zip entries. New bullets for amule-project#785 (alc/alcc/cas/wxcas everywhere + Windows amuleweb), amule-project#794 (.dmg amuleweb path), amule-project#789 (<OS>-<arch> artifact naming), amule-project#780 / amule-project#796 (Windows DPI + comctl32 manifest), amule-project#784 (FHS share/amule paths). - Bug Fixes & Stability: post-amule-project#744 fixes including EC notification leak (amule-project#797), big-library scaling (amule-project#736, amule-project#840 superseding amule-project#728), amulegui ghost entries (amule-project#810, amule-project#819, amule-project#841, amule-project#824, amule-project#830, amule-project#760), PartFile early hash (amule-project#762), server protocol fixes (amule-project#835, amule-project#788, amule-project#721, amule-project#787), crypto stream UB (amule-project#779), UAF prevention (amule-project#756), Kad rotation (amule-project#795, amule-project#799/amule-project#805), GTK warning silencing (amule-project#833, amule-project#826/amule-project#836), and the clang-tidy worklist (amule-project#770, amule-project#772-amule-project#774). - Translations: late-cycle wave covering French/Turkish manpages (amule-project#753/amule-project#754/amule-project#776), Galician (amule-project#763), Slovenian (amule-project#771), pt-BR (amule-project#768/amule-project#775/amule-project#812), French (amule-project#811), plus man-page tooling for date+version drift (amule-project#802). - Contributors: added ngosang for UX feedback on the late-3.0 cycle (amule-project#817/amule-project#818/amule-project#821/amule-project#828/amule-project#844) and ongoing work on the user-facing manual at amule-org.github.io. - Merged PRs flat index: extended with amule-project#746-amule-project#845 + amule-project#841.
…ndex Adds 55+ merged PRs to the 3.0.0 changelog since the last update (amule-project#747, 2026-05-27). Narrative additions cover: - Packaging: expanded the top list to include the macOS per-arch .app bundles and the Windows NSIS installer alongside the existing AppImage / Flatpak / .dmg / .zip entries. New bullets for amule-project#785 (alc/alcc/cas/wxcas everywhere + Windows amuleweb), amule-project#794 (.dmg amuleweb path), amule-project#789 (<OS>-<arch> artifact naming), amule-project#780 / amule-project#796 (Windows DPI + comctl32 manifest), amule-project#784 (FHS share/amule paths). - Bug Fixes & Stability: post-amule-project#744 fixes including EC notification leak (amule-project#797), big-library scaling (amule-project#736, amule-project#840 superseding amule-project#728), amulegui ghost entries (amule-project#810, amule-project#819, amule-project#841, amule-project#824, amule-project#830, amule-project#760), PartFile early hash (amule-project#762), server protocol fixes (amule-project#835, amule-project#788, amule-project#721, amule-project#787), crypto stream UB (amule-project#779), UAF prevention (amule-project#756), Kad rotation (amule-project#795, amule-project#799/amule-project#805), GTK warning silencing (amule-project#833, amule-project#826/amule-project#836), and the clang-tidy worklist (amule-project#770, amule-project#772-amule-project#774). - Translations: late-cycle wave covering French/Turkish manpages (amule-project#753/amule-project#754/amule-project#776), Galician (amule-project#763), Slovenian (amule-project#771), pt-BR (amule-project#768/amule-project#775/amule-project#812), French (amule-project#811), plus man-page tooling for date+version drift (amule-project#802). - Contributors: added ngosang for UX feedback on the late-3.0 cycle (amule-project#817/amule-project#818/amule-project#821/amule-project#828/amule-project#844) and ongoing work on the user-facing manual at amule-org.github.io. - Merged PRs flat index: extended with amule-project#746-amule-project#845 + amule-project#841.
Adds 55+ merged PRs to the 3.0.0 changelog since the last update (#747, 2026-05-27). Narrative additions cover: - Packaging: expanded the top list to include the macOS per-arch .app bundles and the Windows NSIS installer alongside the existing AppImage / Flatpak / .dmg / .zip entries. New bullets for #785 (alc/alcc/cas/wxcas everywhere + Windows amuleweb), #794 (.dmg amuleweb path), #789 (<OS>-<arch> artifact naming), #780 / #796 (Windows DPI + comctl32 manifest), #784 (FHS share/amule paths). - Bug Fixes & Stability: post-#744 fixes including EC notification leak (#797), big-library scaling (#736, #840 superseding #728), amulegui ghost entries (#810, #819, #841, #824, #830, #760), PartFile early hash (#762), server protocol fixes (#835, #788, #721, #787), crypto stream UB (#779), UAF prevention (#756), Kad rotation (#795, #799/#805), GTK warning silencing (#833, #826/#836), and the clang-tidy worklist (#770, #772-#774). - Translations: late-cycle wave covering French/Turkish manpages (#753/#754/#776), Galician (#763), Slovenian (#771), pt-BR (#768/#775/#812), French (#811), plus man-page tooling for date+version drift (#802). - Contributors: added ngosang for UX feedback on the late-3.0 cycle (#817/#818/#821/#828/#844) and ongoing work on the user-facing manual at amule-org.github.io. - Merged PRs flat index: extended with #746-#845 + #841.
amule-project#728) CUpDownClient had two accessors disagreeing about what a missing credits object means. GetCurrentIdentState() treats it as IS_NOTAVAILABLE; the five boolean predicates tested `credits &&` instead, so with credits still NULL every one of them returned false -- including SUINotSupported(), which is exactly the state the accessor was reporting. credits is NULL from construction until the peer's user hash resolves, so this is reachable, not theoretical. The visible effect was in the Client Details dialog. CClientRef::GetSecureIdentTextStatus() is an if/else-if chain over those five predicates with no final else, and its result is assigned straight into the label. All five false left the string empty, blanking the field and overwriting the _("N/A") placeholder it was built with. The same dialog reads "Not supported" in amulegui, whose predicates compare m_identState directly and whose credits object is always allocated -- so the monolithic GUI was the odd one out, disagreeing with both amulegui and the EC/Web API view, all of which go through GetCurrentIdentState(). Define the predicates in terms of that accessor. The contradiction then cannot recur: they are the accessor's answer rather than a second opinion about it. Only SUINotSupported() changes behaviour, and it has exactly one caller in the tree -- the chain being fixed. For the other four the guard was redundant: a NULL credits yields IS_NOTAVAILABLE, which already compares unequal to the state each of them asks about, so their 6/4/1/1 call sites are unaffected. Give the chain a final else as well. It is unreachable while the five predicates cover every EIdentState, but a sixth state would otherwise silently blank the field again rather than fail visibly. No new catalog string: the fallback reuses the _("N/A") the field is already built with. The catalogs still move, because that msgid's reference list gains src/ClientRef.cpp. Not unit-tested: nothing currently links BaseClient/ClientRef, and CUpDownClient's construction pulls a 49-include closure while the chain needs theApp for CryptoAvailable(), so a harness would dwarf the fix. Routing the predicates through the accessor makes the disagreement unrepresentable instead, which a test could only sample for.
…r.cpp (amule-project#675) (amule-project#725) * chore(gui): delete unreachable bitmap functions/entries from muuli_wdr.cpp First slice of the icon-system cleanup scoped in amule-project#675: remove code with zero call sites anywhere in the tree, before any wxArtProvider migration work starts. - muleToolbar(): whole function unused -- superseded by the main wxToolBar setup in amuleDlg.cpp; nothing calls it. - moreImages(): whole function unused, both of its two icon entries. - amuleDlgImages(): 21 of 35 index blocks have no caller anywhere (0-13, 16, 17, 19, 27, 28, 31, 34). The 14 live ones are untouched -- 10 of those (20-26, 29, 32, 33) are already the fallback path inside amuleDlg.cpp's Add_Skin_Icon, which prefers a wxArtProvider/SVG lookup first; the other 4 (14, 15, 18, 30) are still called directly. - amuleSpecial(): 6 of 26 index blocks have no caller (6, 7, 8, 9, 18, 20) -- checked both direct call sites and the PrefsUnifiedDlg.cpp fallback table (pages[].m_imageidx), which uses neither. convert_xpm in PartFileConvertDlg.cpp was on the same "no literal grep hits" list initially but is not actually dead -- SetIcon(wxICON (convert)) reaches it via the wxICON macro's token-pasting (X##_xpm), invisible to a plain identifier search. Caught by a full build failing on the undeclared identifier, not by inspection; left untouched. Deletes 1373 lines (~16% of the file). No behavior change: every touched entry was unreachable code. clang-format v18 clean; full amule build verified (macOS, CLIENT_GUI unaffected since neither touched symbol is CLIENT_GUI-only). * chore(po): regenerate catalogs after muuli_wdr.cpp dead-code removal muleToolbar() duplicated several msgid source-location references (Networks, Searches, Downloads Window, etc.) already present via amuleDlg.cpp's own toolbar setup. Deleting it drops those now-stale #: comments; no msgid added, removed, or retranslated -- verified via unchanged msgid count (1903 before/after) and a diff limited to source-location comments and POT-Creation-Date. Regenerated after rebasing onto upstream/master to pick up po/ changes from amule-project#723/amule-project#724/amule-project#726/amule-project#728/amule-project#730/amule-project#731, which had drifted our prior regeneration out of sync.
3 participants
Summary
amuled currently negotiates EC zlib compression on every connection where the client advertises
EC_TAG_CAN_ZLIB, and the server then compresses every outbound packet larger than 1 KB regardless of who the peer is. On loopback / LAN / link-local connections that's pure overhead — no transit time onlo, and on a gigabit LAN line rate (~125 MB/s) is already several times higher than the ~37 MB/s ceiling a streaming zlib pipeline hits on a typical CPU core.This patch keeps zlib negotiation as-is and instead makes the per-packet send-time decision local-peer-aware:
0.0.0.0,127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,169.254.0.0/16) and packet ≤ 256 MB → send uncompressed (the perf win)ReadHeadergateReadHeader's historical 16 MB packet-size cap is also raised to 256 MB post-auth. Pre-auth keeps the historical 16 MB cap so an unauthenticated peer still can't make us allocate large buffers from one bogus header — the existingIsAuthorized()resize guard atECSocket.cpp:581is the actual security-relevant gate; 16 MB was just a sanity check that becomes too tight once a class of responses goes uncompressed.SetLocalPeer(bool)onCECSocketrecords the locality decision the server makes at auth time. Client-side code is unchanged — the wire flag bits on each packet already control inflate-vs-raw on the receiving side.Companion to #725 and #727 for issue #713 (slow EC responses on big libraries).
Test plan
EC peer 127.0.0.1 is local (loopback/LAN/link-local) — bypassing ZLIB for small/medium packetsshow sharedon big libraries — confirmed by @Stoatwblr on his 91k-file shareset: uncompressed multi-MB response now passes the (raised) receiver gate and completes faster than the previous zlib-on-loopback path