View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0004214 | unreal | ircd | public | 2013-05-25 00:30 | 2015-05-24 16:21 |
| Reporter | Assigned To | syzop | |||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | resolved | Resolution | fixed | ||
| Target Version | 3.4-alpha1 | Fixed in Version | 3.4-alpha3 | ||
| Summary | 0004214: bounce links that are too far out of sync | ||||
| Description | new text: Bounce server links if their time is too much off, see last comment of Syzop. Original idea to kill of NTP code has been rejected. original text by nenolod: Title: remove NTP, TSCTL, etc - just bounce links that are too far out of sync thinking about it a little, i think that this offset stuff can be manipulated to the advantage of splitriders. i'd like to just kill it off. anyone who is not properly using NTP on their host deserves to be yelled at by the ircd. one should especially look at this in the context of how the eventloop has to compensate for changes in the offset etc. it's just bad. | ||||
| Tags | No tags attached. | ||||
| 3rd party modules | |||||
|
|
It adds a lot of complexity and attack surface for very little gain, and should be removed. In its place, warning and error thresholds (admin configurable) should be set up and enforced both at link connect and periodically through timestamp checking. That said, the event loop will still have to deal with timestamp jumping and backwards clock advancement, because the server clock can change arbitrarily (eg. through the action of NTP on a fast system clock). |
|
|
indeed -- but it's easier to do that if we don't have an offset jumping around all the time. :) |
|
|
Syzop, if you don't object, I'd like to do this for alpha2. |
|
|
I see talk about attack vector and split riding etc. Of course there's an issue if IRCd clocks differ. But builtin NTP / TSCTL itself don't cause that. In fact, without NTP built-in and TSCTL you will probably have MORE clocks out of synch and MORE vulnerable networks. Then, the other argument about clock changes, that too can still happen, as you rightfully point out. In fact a lot of these events occur on the system and not by TSCTL. (and built-in NTP is ruled out as a cause as it happens before the event loop) But, to get back to the issue, without all the other claims: Should we remove NTP and TSCTL, and just reject links that have a too large offset? That's a good question. I'd like some feedback from others on this. There are PRO's and CON's... Even just a few seconds off does already create a 'security issue', like for nick lag colliding. If one servers clock is 2 seconds off it makes it much easier. That's why a build-in NTP client can be so helpful if you don't run system NTP. The current implementation is not that accurate, but it can be improved. I saw a discussion on IRC to reject an offset of 600, but 600 seconds are already a gigantic time difference. Which is another question, are you sure you want to disallow linking if apparently NTPd isn't doing it's job? It adds another point of failure in the system. |
|
|
I like the idea of removing the internal NTP, and the servers to just drop the link if the timestamp is too far off. However, I don't like the idea of removing TSCTL's ability to set a time offset. Other (related) thoughts: I liked InspIRCd's idea to designate one IRCd on a network as a time server to automatically set linking servers to the correct time. I think this should only be permitted if the server is allowed to link in the first place with a time difference of nothing serious (like less than 10 seconds). |
|
|
You really don't want ircd setting the system time. ntpd is better at it, and there can be only one (do you really want them fighting over your clock?). |
|
|
I think keeping an offset biased against NTP is fine, but we should remove TSCTL. We should also have a hard requirement of, "oh, you can't link if you're this far out." It's the TSCTL command that is bugging me. |
|
|
@falconkirtaran The IRCd will not be messing with the system time, just being the time authority on the IRC network using offsets to the system timestamp. For example: you have 4 servers, you make the "hub" server the time server. When the other servers link it sends out the current timestamp, and the linking server adjusts its time accordingly. We may want to do this multiple times to ensure the servers have the correct time (like NTP over IRC or something). The result would be the IRCds having a timestamp independent from the system time, and everyone accurate within a couple seconds without the need for sysadmin intervention. |
|
|
We already do pretty much that. I really dislike the added complexity. I'd also like to see the concept of a hub replaced with STP, as a slightly contradictory adjunct. |
|
|
What's so complex about it? 1) The NTP timesynch. code is completely contained and doesn't interfere with the rest. 2) As for time offsets (and TSCTL), we already use a macro, TStime(), for all time stuff.. nothing complex about it. Perhaps more an issue of getting used to. 3) And regarding time going forward or backwards, which is bad (I always said), as I and you already posted you'll keep that problem if system time is adjusted.. which is one of the main reasons for time shifts already. The code dealing with that too shouldn't just be removed because 'ahh ugly, complicated, del del del', no, it fixes a real problem. Of course, in a perfect world, none of the three are necessary, but we don't live in a perfect world. Speaking in general: There often was a good reason to add a feature. And for removal there should even be a BETTER reason, just a 'I don't like XYZ' is often insufficient. Note that I haven't decided on any yet ;p. Hope some more people will jump in to discuss, and feel free to comment on my points of course. |
|
|
well my 2 cents is this: regards to NTP; 1> server runs ntp and thus we don't need an internal one. 2> server does not run ntp and if it's enabled, services can desync even on 127.0.0.1 (bad thing). 3> server does not run ntp, but is linked to a network that does. If services isn't local then all clear. In the even of #1 and #2, our internal ntp should be disabled because it's not needed or it's likely to cause bad juju because of services. In the event of #3, having an internal ntp is a benefit if the admin of the machine refuses to install ntp and we don't have to keep track of clock difference, just drift. If server admin has root, they need to be advised to run ntp and thus condition #1. It's my humble opinion that if we keep NTP, it needs to be disabled by default and the appropriate entries in the config file need to be added so admins can see how and when they should use the feature. Also #3 in my opinion is the really the only reason to keep NTP. If that wasn't 2 cents worth .. Please keep the change! :) |
|
|
Thanks katsklaw, scenario 2) wasn't mentioned yet before, so is one to take into account. IMHO time must be correct, always, even if you have two clocks that are both the same amount 'off', such as on localhost. While it doesn't directly cause a server synchronization issue it does still cause silly things to happen, such as times being incorrect in 'channel created on', 'topic last changed on', and 'online since' (in WHOIS) etc. etc. Another suggestion to consider (again, not saying this is the way to proceed, just adding a possibility!): What we can also do is keep NTP/TSCTL and *add* the bounce functionality. If services would support this protocol change too, then scenario 2) is handled properly as well (link is rejected). 1) Works with system ntp enabled: unreal ntp can/should be disabled, keeping it enabled shouldn't hurt anything either though (TSdiff=0) 2) Works with system ntp disabled: unreal ntp enabled will address most issues 3) Works with both system & unreal ntp disabled/out-of-order (either this system or a remote server such as services), because we would reject links if our TS & their TS differs more than XYZ. This way we encourage everyone to use correct time, while with the original idea (if you remove unreal NTP and only do the TSdiff server reject) you still permit incorrect time in the services scenario katsklaw mentioned. Of course what Stealth suggested is also a possibility, but it requires active intervention for the admin to configure, and is probably not done on 2-3 server networks (though, perfectly possible). Plus, the same can be achieved and is already done with TSCTL / by some services. If you run a bit larger network you hopefully know about NTP and care enough to have the time on your servers to be correct. What katsklaw mentioned in 0004220 is that he still often sees the 'incorrect time' problem with users. This is also a factor. I was hoping that by now system NTP would be so widespread that unreal NTP would be unneeded. But apparently that is not the case. |
|
|
Added in 3.4-alpha3: https://github.com/unrealircd/unrealircd/commit/0a42cedf77aae8bbb6769e7477e485aee3ca1340 Bounce links that have their clock too far out of sync (0004214). Currently set at 1 minute. TODO: make configurable. This only works with newer servers as it relies on PROTOCTL TS=xyz very early in the synch. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-05-25 00:30 |
|
New Issue | |
| 2013-05-25 00:34 | falconkirtaran | Note Added: 0017666 | |
| 2013-05-25 00:38 |
|
Note Added: 0017668 | |
| 2013-05-25 01:33 |
|
Note Added: 0017671 | |
| 2013-05-25 15:46 | syzop | Note Added: 0017675 | |
| 2013-05-25 15:46 | syzop | Status | new => feedback |
| 2013-05-25 16:55 | Stealth | Note Added: 0017677 | |
| 2013-05-26 00:36 | falconkirtaran | Note Added: 0017678 | |
| 2013-05-26 01:30 |
|
Note Added: 0017679 | |
| 2013-05-26 06:39 | Stealth | Note Added: 0017681 | |
| 2013-05-26 06:45 | falconkirtaran | Note Added: 0017682 | |
| 2013-05-26 06:46 | falconkirtaran | Note Edited: 0017682 | |
| 2013-05-26 10:36 | syzop | Note Added: 0017683 | |
| 2013-06-08 00:20 | Stealth | Relationship added | related to 0004220 |
| 2013-06-08 00:52 | katsklaw | Note Added: 0017707 | |
| 2013-06-08 11:30 | syzop | Note Added: 0017709 | |
| 2014-03-14 01:14 | peterkingalexander | Issue cloned: 0004299 | |
| 2014-05-31 22:14 | syzop | Summary | remove NTP, TSCTL, etc - just bounce links that are too far out of sync => bounce links that are too far out of sync |
| 2014-05-31 22:14 | syzop | Description Updated | |
| 2015-05-24 16:20 | syzop | Note Added: 0018339 | |
| 2015-05-24 16:20 | syzop | Status | feedback => resolved |
| 2015-05-24 16:20 | syzop | Fixed in Version | => 3.4-alpha3 |
| 2015-05-24 16:20 | syzop | Resolution | open => fixed |
| 2015-05-24 16:20 | syzop | Assigned To | => syzop |
