[pull] main from InterNetNews:main #62
Merged
Conversation
This file contains 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
![pull[bot]](https://avatars.githubusercontent.com/in/12910?s=60&v=4){kind=small}
A bug about stripping IP options not working for IPv6 connections was mentioned in the innd manual page. Stripping IP options for IPv4 was a vestige introduced in INN 1.6b1 and already removed in INN 2.6.0 in 2014 when modernizing the network library. Anyway, even the initial code from INN 1.6b1 was incomplete and did not prevent INN from being vulnerable to IP spoofing attacks with source-routed TCP connections. Other software that also does IP-based restrictions never bothers with this: xinetd, for example, or the rsync server. That implies that this is no longer considered a significant security issue, probably because it's been fixed upstream in the kernel network stack. close #183 (more information in the ticket)
The current code only removes the first component of the domain, which is not enough in some cases like "ec2-13-236-155-204.ap-southeast-2.compute.amazonaws.com" and "ec2-18-184-57-186.eu-central-1.compute.amazonaws.com" which would end up in several lines gathered by AWS datacenters whereas we would just want a report of AWS connections from "*.compute.amazonaws.com". To improve the report, just keep up to the last 3 components of the hostname. (Some country code top-level domains have 2 components, like ".co.uk" so we need a minimum of 3 components, which looks fine during my testings.) Also, add code to correctly report IPv6 addresses as unresolved instead of unknown. close #305
Also improve (a bit) the name of the sections. Notably, the stats by domain are for all the connections and not only readership connections (that is to say which are not "curious" connections, reported afterwards, without any NNTP command related to reading or posting).
Elapsed times, read articles and groups, etc. were wrong in totals. When innreport tags a reader as a curious one (that is to say a connection which does not send NNTP commands for reading nor posting) or finds out that the client did not manage to authenticate, it was removed from the readership statistics by client but is still counted in the statistics per domain. Now fixed. Also remove some unused variables while refactoring the related code. close #306
It shouldn't have been committed, sorry.
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.
See Commits and Changes for more details.
Created by
pull[bot]
Can you help keep this open source service alive? 💖 Please sponsor : )