channel mode +M - #5
Merged
Merged
Conversation
channel moderation where only registered users can speak
entrope
reviewed
May 5, 2019
entrope
left a comment
entrope
left a comment
Contributor
There was a problem hiding this comment.
I am not a fan of this kind of mode; it creates second-class members of a channel.
Move MODE_DELJOINS Co-Authored-By: Michael Poole <mdpoole@troilus.org>
entrope
reviewed
Dec 22, 2019
Ratler
added a commit
that referenced
this pull request
Aug 28, 2026
…nd close semantics (#99) * Fix TLS sendq accounting on partial writes (con_rexmit) TLS short writes (SSL_MODE_ENABLE_PARTIAL_WRITE and the GnuTLS/libtls equivalents) could leave con_rexmit pointing mid-message while send_queued() advanced the MsgQ via msgq_delete(count_out). Finishing the remainder then called msgq_excise() with a mid-message pointer, which only matches the start of a MsgBuf: default builds abort(), and --disable-asserts builds leave the queue wrong and corrupt the TLS stream. Clients dropped with no IRC ERROR (after NOTICE AUTH, or under large NAMES / sendq bursts). Send path (all TLS backends + deliver_it): - Credit successful rexmit bytes through count_out so msgq_delete() advances the queue's own Msg.sent offset; con_rexmit is then always head->msg + head->sent. msgq_excise() is removed. - Drain short TLS writes until the message finishes or the backend reports a real block (WANT_WRITE / EAGAIN); return IO_BLOCKED only then. - Do not set FLAG_BLOCKED on TLS short IO_SUCCESS (that busy-loops ET_WRITE while POLLOUT stays ready). Still set it for plain short writev. Credit sendB on IO_BLOCKED when some bytes already went out. - Clear con_rexmit beside every MsgQClear (dead_link / close_connection / dealloc_connection). Tests: TLS auth-drop pipeline and NAMES-burst regressions (tls_stress marker, pytest-timeout dependency). * Fix AuthRequest freelist/timer lifecycle and double check_auth_finished - destroy_auth_request() during timer_run() while the timeout timer is GEN_MARKED must not freelist the AuthRequest yet: timer_del() is a no-op while marked and timer_run() still owns the Timer. Defer with AR_FREE_PENDING and freelist on ET_DESTROY. Detach from the client first, and timer_del() before freelist reuse so a leftover queue link cannot form a self-loop that pegs timer_enqueue() at 100% CPU. - Split start_dns_ident_queries() from start_dns_ident() so start_auth() finishes auth exactly once. start_dns_ident() remains the deferred WebSocket resume path from s_bsd and finishes auth itself. Previously start_auth() called check_auth_finished() twice on one AuthRequest, which could use a freelisted request (SIGSEGV after client pointer nulling) when the first call completed or exited the client. * TLS: fail negotiation cleanly when the session is gone ircd_tls_negotiate() returned 1 (success) when s_tls() was already NULL, so FLAG_NEGOTIATING_TLS stayed set and start_auth() was invoked on every subsequent ET_WRITE. Clear the flag and return -1 in all three backends. OpenSSL: return an explicit 1 on handshake success; the previous 'return res' at the end of the function returned X509_digest()'s result, which reads as "still negotiating" when the digest fails. * GnuTLS: align close and EOF semantics with OpenSSL and libtls - gnutls_record_recv() == 0 is a peer close -> IO_FAILURE. It was treated as IO_BLOCKED (gnutls_error_is_fatal(0) is false), leaving the session open until the client gave up on its SSL shutdown wait (30s for asyncio). - ircd_tls_close(): only send close_notify after a completed handshake, matching OpenSSL SSL_is_init_finished() and libtls tls_close(). ircd_tls_negotiate() marks success via gnutls_session_set_ptr(); gnutls_protocol_get_version() cannot be used for this since it is already set before any ClientHello arrives. A stalled handshake now yields TCP EOF instead of a TLS alert record. Tests: test_stalled_handshake_times_out asserts a plain EOF (no alert byte accepted); new test_stalled_handshake_after_clienthello_times_out drives the client side in a MemoryBIO so an encrypted TLS 1.3 alert after ServerHello is detected too. Verified on openssl, gnutls, libtls. * GnuTLS: accept peer certificate when fingerprint extraction fails If gnutls_x509_crt_import() or gnutls_x509_crt_get_fingerprint() failed after a successful handshake, ircd_tls_negotiate() returned 1 without clearing FLAG_NEGOTIATING_TLS, so send_queued() never sent anything and the connection wedged until ping timeout. Log the error, leave the fingerprint empty and complete the handshake normally, as the OpenSSL and libtls backends do. * Handle ircd_tls_negotiate() failure in completed_connection() The outbound server-link path discarded the negotiate result and only looked at FLAG_NEGOTIATING_TLS. A handshake timeout (which returns -1 without clearing the flag) was read as "still negotiating" and the link sat until ping timeout; a missing session (flag cleared, -1) fell through to sending PASS/SERVER on a socket without TLS. Handle the result like tls_negotiate_client(): notify opers, close the TLS session and fail the connection. * tests: debug capture robustness and TLS container debug settings - conftest: never abort the suite when the failure snapshot cannot be written (e.g. root-owned failures/ from a prior docker run); fall back under /tmp. - debug_support: snapshot whichever hub container is running (tls-hub, hub, limits) instead of hard-coding ircu-hub. - docker-compose: debug volume, ASAN options, seccomp:unconfined and core/nofile ulimits for ircd-tls-hub / ircd-tls-leaf, matching the other ircd services. * TLS send path: remove drained rexmit by identity; no progress on fatal error Follow-up to the TLS PR, addressing review findings #1 and #2. #1 (correctness): the con_rexmit drain reported a raw byte count that send_queued() fed to msgq_delete(), which deletes in (partial-normal, prio, normal) order. When a whole normal message was deferred (sent==0) and a priority message (e.g. a check_pings PING) was enqueued before the next ET_WRITE, msgq_delete() deleted the never-sent PING first and left the normal message's tail to be re-sent as duplicate bytes -- desyncing the P10 link and dropping the PING. Restore identity-based removal: a corrected msgq_excise() (matching the head message by buffer containment, checking both queues) removes exactly the drained message, and its bytes are no longer credited to *count_out. The in-loop drain stays count-based (it sends in mapiov order within one synchronous call). #2 (disclosure): on a fatal mid-drain error the backend frees the TLS session and returned IO_FAILURE with count_out > 0; deliver_it() reported that as progress, so send_queued() looped back and flushed the rest of the sendq -- private messages included -- as plaintext on the raw socket. Zero *count_out on every IO_FAILURE return so deliver_it() reports no progress and the link dead-links immediately, matching pre-PR behaviour. Applied consistently to the OpenSSL, GnuTLS and libtls backends. * s_bsd: fix outbound TLS negotiation-failure handling Follow-up to the TLS PR, addressing review findings #3 and #5. #3: completed_connection()'s TLS-failure branch freed the session but did not mark the client dead, so exit_client() flushed its ERROR line as plaintext into the half-open handshake stream. Set FLAG_DEADSOCKET so can_send() rejects the write, matching tls_negotiate_client(). #5: tls_handshake_succeeded() discarded completed_connection()'s return, so a link whose Connect block vanished during the handshake (rehash) was left half-initialized until the ping timeout. Exit the client on a 0 return, as the ET_CONNECT path already does. * s_auth: assert freelisted AuthRequest timer invariant instead of repairing Follow-up to the TLS PR, addressing review finding #6. The defensive timer_del() in start_auth() was a no-op on a still-GEN_MARKED timer, after which memset() zeroed t_header links timer_run() still owned -- recreating the timer-enqueue self-loop (100% CPU) it was meant to prevent. AR_FREE_PENDING already guarantees a freelisted request's timer is fully destroyed before reuse, so assert that invariant rather than silently repairing a state that should never occur. * tests: fix debug-snapshot exception guard and TLS-hub clone throttling Follow-up to the TLS PR, addressing review findings #4 and #7. #4: the failure-snapshot guard caught only OSError, but the docker inspect/logs calls raise subprocess.TimeoutExpired (a SubprocessError) on a hung daemon -- turning every test failure into a pytest internal error. Catch subprocess.SubprocessError too. #7: the TLS stress tests open many concurrent connections from the single docker-network IP; add permissive IPCHECK_CLONE_LIMIT/PERIOD to the TLS hub config (matching the plaintext hub) so clone throttling does not fail them spuriously. * tests: add msgq_excise unit test for the TLS rexmit prio-reorder fix Covers review finding #1 at the C level. A normal message deferred to con_rexmit (sent == 0) with a priority message enqueued behind it must be removed by identity, leaving the priority message intact; the test also checks a mid-message con_rexmit pointer (multi-partial drain), excising a priority message, and the lone-message case. Wired into `make check` as msgq_excise_t. Verified to abort on the pre-fix count-based deletion, which removes the priority message in place of the drained one. * TLS: surface detailed handshake and verification failure reasons ircd_tls_negotiate() collapsed every failure to a bare -1, so operator notices and disconnect logs read only "TLS negotiation failed". It now fills a caller-provided reason buffer (TLS_REASON_LEN) with a specific, human-readable cause, propagated to the SNO_OLDSNO operator notice and the disconnect reason on both the inbound (tls_negotiate_client) and outbound (completed_connection) paths. A categorical ERROR line is still written to the peer where the channel is plaintext; TLS-layer rejections reach the peer as the backend's own TLS alert instead. Reasons per backend: - OpenSSL: X509_verify_cert_error_string() for a verification abort (SSL_get_verify_result() is set even when SSL_accept() fails), otherwise the OpenSSL error reason; plus certificate-required and handshake errors. - GnuTLS: gnutls_certificate_verification_status_print() for a bad verdict, gnutls_strerror() for the verify call or handshake error. - libtls: tls_error() for the handshake error. A stalled handshake still closes with a plain TCP EOF (no peer write, which would corrupt a mid-handshake peer's TLS stream). The server-link fingerprint-mismatch notice now reports the presented and configured fingerprints (m_server.c). Example operator notices: TLS negotiation failed from unknown server: self-signed certificate TLS negotiation failed from unknown server: certificate has expired TLS negotiation failed to leaf2.test.net: no shared cipher * listener: enlarge show_ports flags buffer; add STATS p regression tests show_ports() built its flag string in char flags[8], which was sized for the original C/S + H + 4- + 6- letters (6 + NUL). The TLS ('E') and Cloudflare ('F') additions push the worst case (hidden TLS cloudflare listener with both address families unbound after a rehash) to 8 characters plus NUL, one byte past the buffer. Bump it to 16. Add tests/stats_ports covering STATS p locally, hunted to a remote server, after REHASH, and on a server with TLS/websocket/cloudflare listeners. * GnuTLS: keep count_out credit on non-fatal send errors Both crediting error paths in ircd_tls_sendv() zeroed *count_out before deciding whether the error was fatal, so a non-fatal error (e.g. a warning alert) mid-batch returned IO_BLOCKED with zero credit. send_queued() then deleted nothing from the sendq and the messages already handed to gnutls_record_send() were retransmitted on the next ET_WRITE, duplicating lines mid-stream. Zero the credit only on IO_FAILURE, matching the OpenSSL and libtls backends. --------- Co-authored-by: Stefan Wold <ratler@undernet.org>
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
2 participants
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.
New channel mode to prevent unauthenticated users from speaking or using part messages in a channel (unless opped or voiced).
Helpful to combat channel abuse (especially the recent troubles) where +r is too restrictive and the use of +Dm with bots is too complex for many users. This new mode could be used either permanently for many channels, or temporarily during periods of abuse.