View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003720 | unreal | ircd | public | 2008-08-17 18:57 | 2010-11-17 17:06 |
| Reporter | fez | Assigned To | syzop | ||
| Priority | normal | Severity | major | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Platform | x86 | OS | Linux Mandrake | OS Version | 2.6.24.2-1fez |
| Product Version | 3.2.7 | ||||
| Fixed in Version | 3.2.9-RC1 | ||||
| Summary | 0003720: ULines and Server can set channel mode +z when insecure users are present causing an "invalid state" of security | ||||
| Description | M_MODE does not do any checking to see if insecure users are on a channel when setting mode +z, IF the setter of the mode is ULined or a server. This means that, for example, I could set MLOCK +z on a channel (via services), then a new (insecure) user could join the empty channel, and services would set mode +z, and the user would remain on the channel. Since M_SJOIN enforces +z rules, in the case of a netsplit/netrejoin, so, too, should m_mode enforce +z rules even if the MODE command comes from services/uline/server. | ||||
| Additional Information | Here is the fix: I forget what is the diff command to create the appropriately readable patch. ********** src/modules/m_mode.c line 829 --OLD (from 3.2.7 release)-- case MODE_ONLYSECURE: notsecure = 0; if (what == MODE_ADD && modetype == MODE_ONLYSECURE && !(IsServer(cptr) || IsULine(cptr))) { for (member = chptr->members; member; member = member->next) { if (!IsSecureConnect(member->cptr) && !IsULine(member->cptr)) { sendto_one(cptr, err_str(ERR_CANNOTCHANGECHANMODE), me.name, cptr->name, 'z', "all members must be connected via SSL"); notsecure = 1; break; } } member = NULL; /* first break nailed the for loop, this one nails switch() */ if (notsecure == 1) break; } goto setthephuckingmode; ********** src/modules/m_mode.c line 829 --FIXED-- case MODE_ONLYSECURE: notsecure = 0; // modified by fez, so if servers set onlysecure mode, it honors it and removes insecure users if (what == MODE_ADD && modetype == MODE_ONLYSECURE) { if (IsServer(cptr) || IsULine(cptr)) { kick_insecure_users(chptr); } else { for (member = chptr->members; member; member = member->next) { if (!IsSecureConnect(member->cptr) && !IsULine(member->cptr)) { sendto_one(cptr, err_str(ERR_CANNOTCHANGECHANMODE), me.name, cptr->name, 'z', "all members must be connected via SSL"); notsecure = 1; break; } } member = NULL; /* first break nailed the for loop, this one nails switch() */ if (notsecure == 1) break; } } goto setthephuckingmode; | ||||
| Tags | No tags attached. | ||||
| 3rd party modules | |||||
|
|
CAVEAT In this scenario, although "secure" channels are now properly safeguarded (aside, of course, from having to trust server admins that server-server links are secure), it does raise one possible problem. If a user has their client to "auto-rejoin-on-kick", and they join an empty channel, and services sets the channel to secure, the server will kick the user from the channel, causing the now empty channel to be destroyed, allowing the user to rejoin the empty channel again. In other words, it can create a server-client join-kick flood battle. For this I recommend configuring services to remain present in any channels that are set to secure. I still recommend implementing the afformentioned patch, though. Please discuss.... - Eric / fez |
|
|
One possibility We could have "insecure" kicks caused by MODE +z to result in PART messages going to the users instead of kick messages. Insecure kicks cause by SJOIN would still result in KICK messages. Or perhaps it could alternate between KICK and PART, or perhaps it could be a configurable setting.... In any case, something clearly needs to be done, cause it should not be possible to create a +z channel with non-secure users on it. I'll test it out by duplicating kick_insecure_users() as part_insecure_users(), and trying it on irc://chi.wrongway.org Peace |
|
|
FIX To implement the PART idea, where Servers/Ulines issuing MODE +z on channels with insecure users cause the server to send "PART" messages to insecure users rather than KICK messages, you must make these modifications: 1. Apply the simple change I noted in the original post, changing kick_insecure_users to part_insecure_users, like so: ********** src/modules/m_mode.c line 829 --OLD (from 3.2.7 release)-- case MODE_ONLYSECURE: notsecure = 0; if (what == MODE_ADD && modetype == MODE_ONLYSECURE && !(IsServer(cptr) || IsULine(cptr))) { for (member = chptr->members; member; member = member->next) { if (!IsSecureConnect(member->cptr) && !IsULine(member->cptr)) { sendto_one(cptr, err_str(ERR_CANNOTCHANGECHANMODE), me.name, cptr->name, 'z', "all members must be connected via SSL"); notsecure = 1; break; } } member = NULL; /* first break nailed the for loop, this one nails switch() */ if (notsecure == 1) break; } goto setthephuckingmode; ********** src/modules/m_mode.c line 829 --FIXED-- case MODE_ONLYSECURE: notsecure = 0; // modified by fez, so if servers set onlysecure mode, it honors it and removes insecure users if (what == MODE_ADD && modetype == MODE_ONLYSECURE) { if (IsServer(cptr) || IsULine(cptr)) { part_insecure_users(chptr); } else { for (member = chptr->members; member; member = member->next) { if (!IsSecureConnect(member->cptr) && !IsULine(member->cptr)) { sendto_one(cptr, err_str(ERR_CANNOTCHANGECHANMODE), me.name, cptr->name, 'z', "all members must be connected via SSL"); notsecure = 1; break; } } member = NULL; /* first break nailed the for loop, this one nails switch() */ if (notsecure == 1) break; } } goto setthephuckingmode; 2.modify include/h.h, like so: ********** include/h.h line 784 --OLD (from 3.2.7 release)-- extern void kick_insecure_users(aChannel *); ********** include/h.h line 784 --FIXED-- extern void kick_insecure_users(aChannel *); extern void part_insecure_users(aChannel *); 3. in src/s_misc.c, at the bottom of the file, insert: ********** src/s_misc.c at bottom of file: /** Parts all insecure users on a +z channel */ void part_insecure_users(aChannel *chptr) { Member *member, *mb2; aClient *cptr; char *comment = "Insecure user not allowed on secure channel (+z)"; for (member = chptr->members; member; member = mb2) { mb2 = member->next; cptr = member->cptr; if (MyClient(cptr) && !IsSecureConnect(cptr) && !IsULine(cptr)) { RunHook4(HOOKTYPE_LOCAL_PART, cptr, &me, chptr, comment); if ((chptr->mode.mode & MODE_AUDITORIUM) && is_chanownprotop(cptr, chptr)) { sendto_chanops_butone(cptr, chptr, ":%s!%s@%s PART %s :%s", cptr->name, cptr->user->username, GetHost(cptr), chptr->chname, comment); sendto_prefix_one(cptr, &me, ":%s!%s@%s PART %s :%s", cptr->name, cptr->user->username, GetHost(cptr), chptr->chname, comment); } else { sendto_channel_butserv(chptr, &me, ":%s!%s@%s PART %s :%s", cptr->name, cptr->user->username, GetHost(cptr), chptr->chname, comment); } sendto_serv_butone_token(&me, cptr->name, MSG_PART, TOK_PART, "%s :%s", chptr->chname, comment); remove_user_from_channel(cptr, chptr); } } } 5. recompile. If you're on win32, you'll have to re-make SYMBOLFILE. 6. viola... |
|
|
I have found the PART idea works nicer than the KICK idea for this particular purpose. It avoids the dreaded kick/rejoin war in empty channels with mlock +z activated. An alternative solution would be to make services do the work, and have ChanServ join the channel making it non-empty, setting mode +z, and explicitly kicking users, then perhaps leaving the channel after a time limit. Nevertheless, this way, the ircd can actually provide some assurance that secure channels only have secure users. Another option to consider down the line would be a config option to have secure channels ensure secure links, but that would be very difficult to implement as leafs more than 1 hop away do not even report whether they are securely linked or not; only that they exist. I think this is an acceptable comprimise -- avoiding chanserv mlock +z causing mode +z to be set on channels with insecure users without causing possible kick/join wars. Please discuss |
|
|
you can test it at chi.wrongway.org #securetest |
|
|
Um, helllloooooo! This was my f*ing idea like months ago! EDIT This was discussed a LONG time ago, like....3.2.something like 2 years ago, i thought we came to the conclusion that it needed fixed and was, maybe I'm wrong. *shrugs* |
|
|
Well, now you have a fix right here. Hope it gets incorporated into 3.2.8. - fez personally i like using part_insecure_users for the MODE +z method, though KICK may be just as good (possibly causing KICK/JOIN floods). Another option would be to cause a temporary (e.g. 2 second) SHUN on users who are affected by MODE +z KICK's to prevent their auto-rejoin from functioning. This may be a better option. |
|
|
I have been source diving and this is what I have determined: kick_insecure_users is defined in src/s_misc.c shuns are executed by the function m_tkl_line in file src/modules/m_tkl.c this means that modifying kick_insecure_users to take in a SHUN parameter would make s_misc.c dependent on the module M_TKL, which may or may not be loaded. This won't compile. The other option is to move kick_insecure_users into another file such as M_TKL.c, M_SJOIN.c or M_MODE.c, but this would again cause dependency issues as M_SJOIN and M_MODE would be dependent on M_TKL. The only way to resolve this, essentially, is to move the SHUN system (and thus, the entire TKL system) into the core of the IRCd to prevent dependency issues. An alternative (just as difficult), would be to make M_SJOIN and M_MODE check for the M_TKL module at run-time, but that would require something like making an API for modules to load other modules, still a big mess. Perhaps if there was some sort of generic dummy TKL interface built-in to the IRCd, then M_TKL could just implement it, that would be better, but also still difficult. In conclusion, I recommend using the part_insecure_users method on MODE +z, and perhaps adding a config-file or compile-time directive, like PART_ON_ULINE_MODE_SECURE. That's the easiest "neat" answer for this problem... Please discuss - fez |
|
|
REVISED FIX Here is yet another idea!!! I have edited the fix described above to act slightly differently: 1. only send PART if there is one user on the channel (e.g., the channel was just created), otherwise, send the mass KICK (e.g., if the channel is already populated and the owner just did something like /chanserv set #chan mlock +z). 2. when PART is sent, also send ERR_ONLYSECURECHAN so that the client actually gets two messages, in case (as in the case of mIRC) the PART reason is not seen. I have tested this with mIRC and irssi and it seems to work well, without baffling users. To implement this patch, do the following: 1. modify src/modules/m_mode.c, like so: ********** src/modules/m_mode.c line 829 --OLD (from 3.2.7 release)-- case MODE_ONLYSECURE: notsecure = 0; if (what == MODE_ADD && modetype == MODE_ONLYSECURE && !(IsServer(cptr) || IsULine(cptr))) { for (member = chptr->members; member; member = member->next) { if (!IsSecureConnect(member->cptr) && !IsULine(member->cptr)) { sendto_one(cptr, err_str(ERR_CANNOTCHANGECHANMODE), me.name, cptr->name, 'z', "all members must be connected via SSL"); notsecure = 1; break; } } member = NULL; /* first break nailed the for loop, this one nails switch() */ if (notsecure == 1) break; } goto setthephuckingmode; ********** src/modules/m_mode.c line 829 --FIXED-- case MODE_ONLYSECURE: notsecure = 0; // modified by fez, so if servers set onlysecure mode, it honors it and removes insecure users if (what == MODE_ADD && modetype == MODE_ONLYSECURE) { if (IsServer(cptr) || IsULine(cptr)) { if (chptr->users == 1) part_insecure_users(chptr); else kick_insecure_users(chptr); } else { for (member = chptr->members; member; member = member->next) { if (!IsSecureConnect(member->cptr) && !IsULine(member->cptr)) { sendto_one(cptr, err_str(ERR_CANNOTCHANGECHANMODE), me.name, cptr->name, 'z', "all members must be connected via SSL"); notsecure = 1; break; } } member = NULL; /* first break nailed the for loop, this one nails switch() */ if (notsecure == 1) break; } } goto setthephuckingmode; 2.modify include/h.h, like so: ********** include/h.h line 784 --OLD (from 3.2.7 release)-- extern void kick_insecure_users(aChannel *); ********** include/h.h line 784 --FIXED-- extern void kick_insecure_users(aChannel *); extern void part_insecure_users(aChannel *); 3. in src/s_misc.c, at the bottom of the file, insert: ********** src/s_misc.c at bottom of file: /** Parts all insecure users on a +z channel */ void part_insecure_users(aChannel *chptr) { Member *member, *mb2; aClient *cptr; char *comment = "Insecure user not allowed on secure channel (+z)"; for (member = chptr->members; member; member = mb2) { mb2 = member->next; cptr = member->cptr; if (MyClient(cptr) && !IsSecureConnect(cptr) && !IsULine(cptr)) { RunHook4(HOOKTYPE_LOCAL_PART, cptr, &me, chptr, comment); if ((chptr->mode.mode & MODE_AUDITORIUM) && is_chanownprotop(cptr, chptr)) { sendto_chanops_butone(cptr, chptr, ":%s!%s@%s PART %s :%s", cptr->name, cptr->user->username, GetHost(cptr), chptr->chname, comment); sendto_prefix_one(cptr, &me, ":%s!%s@%s PART %s :%s", cptr->name, cptr->user->username, GetHost(cptr), chptr->chname, comment); } else { sendto_channel_butserv(chptr, &me, ":%s!%s@%s PART %s :%s", cptr->name, cptr->user->username, GetHost(cptr), chptr->chname, comment); } sendto_one(cptr, err_str(ERR_SECUREONLYCHAN), me.name, cptr->name, chptr->chname); sendto_serv_butone_token(&me, cptr->name, MSG_PART, TOK_PART, "%s :%s", chptr->chname, comment); remove_user_from_channel(cptr, chptr); } } } 5. recompile. If you're on win32, you'll have to re-make SYMBOLFILE. Note: you may have to clean up the tab stops from spaces. my copy/paste function didn't seem to preserve it correctly. I think this is the most manageable solution thus far... Please discuss... |
|
|
>>// modified by fez, so if servers set onlysecure mode, it honors it and removes insecure users I believe this was suggested by another user at one point and was agreed that removing all the insecure users was to risky, some op could just mlock +z and easily take over the channel, or during a netsplit some random user could acquire op on a server, Set +z, then when they relinked & +z went to all the servers, he/she could easily take over the channel. It would be better to just remove +z if there are insecure users in the channel. |
|
|
It is true that it is possible to "takeover" a channel by setting mode +z on it when there insecure users present. This is possible even without my patch. There are a few things to consider here. 1. we assume that if a user has ops on a channel then they should be allowed to set +z/-z on it. 2. if a user acquired ops by rejoining an empty chan during a netsplit, then set +z, the +z will be removed anyway thanks to the timestamp protocol. (channels with younger timestamps do not have their modes merged with equivalent channels with older timestamps; rather their modes are simply removed). 3. although, yes, simply removing mode +z if there are any insecure users present will alleviate the issue of "secure desynching", another problem then arises. Essentially, a channel could be made -z, even though secure users joined it with the assumption that it was secure. there would be a trust violation here... Although it may not be fair to assume that once a channel is +z, it will remain that way, there should at least be some guarantee that -z can only be set by human interaction... 4. finally, there is the practical issue here. If you're setting mode -z after a ULINE set +z, then there are two problems: first, the assumption that ULINES can set any mode they want has been broken and proves inconsistent. Unreal was specifically designed to let ULINEd servers have special control without being contradicted, because those contradictions generally leads to problems. Secondly, an example of such a problem. If you set chanserv MLOCK +z on a channel, and then unreal overrides it because there are insecure users on the channel, then you trigger a war between (ULINEd) services setting mode +z, and unreal setting mode -z. This mode war would be even worse than the initial problem of a possible kick/join flood. Especially for this last practical reason, I cannot agree with the recommendation that +z should simply be overriden if the channel contains insecure users. I might be inclined to agree with that implementation *IF* the originator of the +z command was *not* ULINEd, but that does not resolve the rather important original issue of what to do if a ULINE sends mode +z to a channel with insecure users. |
|
|
I would suggest everyone who considered this to take a deep breath, look over it again. I mean, just look at it, can you see how ugly and intrusive this patch is?? Perhaps it started as a good idea, but if you read over it again, you should see it.. I hope. It's definitely not something I'll implement. As for MLOCK ignoring the rules of +z, which seems to be the issue. Simple solution: the services coders should fix this. If you're not happy with their solution, then fix it in a different way (in services). I really don't see any reason for us to fix services bugs. |
|
|
Sorry to reopen this. I have implemented the fix as a module since it seems no one cares about this. See http://wrongway.org/mods/f_ulinessl.zip for the fix. It adds a config setting to specify if you want a user to be kicked or parted if a channel contains only insecure users, as set::uline-secure-action. Feel free to comment on it's functionality. Peace - fez |
|
|
I really do think this should be in the core... An issue like this can go back and fourth between users and developers and carry on for ages (and this one has been around for quite some time). Users want to know their channel is secure, especially after a services or network failure... and no one wants to implement it. IRCd developers say it isn't their problem because services should be able to determine what modes can be set when and what modes require what conditions. Services developers say it's up to the IRCd to enforce the modes set on a channel. Unreal has the protocol where anything a ULine sends gets done, which doesn't help this situation at all. Someone needs to step up and be the better part of this and just implement some kind of check. Either way, users will be booted from a channel, so it doesn't really matter who does it in the end, it just needs to happen. I think it should be the IRCd. > can you see how ugly and intrusive this patch is?? Ugly, perhaps. Intrusive? I think that's a stretch. It may not be pretty, but if you think about the number of situations this will actually happen in everyday IRC usage it's something that would make many people with secure channels a little happier, especially during network or service failure. As I see it, there are a few ways this can happen. - Users get KICKed from the channel with the friendliest reason we can come up with. - Users get a nice gentle PART from the channel (with reason). - We implement mode +Z that is set until all the non-SSL users have left, while disallowing non-SSL users to join. With the last non-SSL user, the channel is set to +z. - We implement mode +Z that is set along with +z to let people know there are insecure users still in the channel. (+Z is unset when the last one leaves, -Z will not be accepted from U:Lines) - We ignore the fact that +z was set (unless it can be). - We don't send channel messages to non-SSL users, as well as not allowing them to send messages to the channel. (Which leaves them in the channel, just unable to participate) |
|
|
I honestly am surprised that you don't understand how ugly and bad the previously proposed hacks are. Anyway... I'm glad you came with a better idea, +Z :P. Though, I heard you might have taken it from another ircd? If so, do you know which one? The +Z idea makes sense. However, I would rather implement it like: * Keep +z like it is now, "Only clients on a Secure (SSL) Connection may join". This is how it's already documented, note the 'may join' and NOT 'only clients on a SSL connection are on the channel'! * +Z will be set when all users on the channel are secure. So that means: * /mode #chan +z, and everyone is secure, +zZ will be set. * /mode #chan +z, and not everyone is secure, +z will be set. * once everyone on the channel is secure, set +Z * If for some reason (remote) insecure people join, set -Z * For any +Z/-Z mode switching there'll be a server notice, to say why it is being done, and what it means. Same for when +z gets set but not +Z. That sounds like a better idea than transforming a z into a Z or doing it the other way around. That way we can also get rid of the kick_insecure_users(). Well, partially, as we'll keep it for channels where a lower TS (+z) wins from a higher TS (not +z). But it can be removed from merge (equal TS). I don't know why I did it on merge, looking back and reading logs I probably just didn't think of it, but it's pretty stupid, merge should merge both sides, and now the one without +z is a 'loser', this violates the principle of 'merge'. If we do this +Z thing, then we could also allow +z to be set with non-SSL users present. Now, sure, you can say "users might accidentally set +z and lock themselves out", but this is no argument for me, as there are many ways (and channel modes) to do that. As for compatibility, it's an addition of a paramless channel mode so is ok to add. And software like services shouldn't care much about it, like they don't even have to implement support for it. [*] 'secure' in this context obviously means that the people who are on it are on SSL/TLS (umode +z), nothing else. |
|
|
In that case you might as well remove all the code that prevent +z being set when insecure users are present. +z should be settable at any time, since it offers no assurances. |
|
|
yeah, like I said.. 'If we do this +Z thing, then we could also allow +z to be set with non-SSL users present.' |
|
|
This post comes from the trolling side of me, but has some interesting questions/points about questions/topics that have come up in the past... - How will +Z be handled over server links? Will it be up to each local IRCd to set, or will the IRCd with the last non-SSL user be responsible for setting it? - What happens when a crappily coded ULine wants to set +Z on a channel with non-SSL users? Will they be parted, or will the mode simply be denied. - What about SSL users on non-SSL links? (I am sure this is rare, but a valid point when trying to create truly secure channels) - Will they be allowed to join even though the link between servers is plaintext? - Will -Z be set until all the SSL users from the non-SSL link leave? I know #3 will probably create some confusion over users and possibly expose some network vulnerability in identifying some of the server routing and which links are plaintext, but unfortunately it's a problem unless you simply -Z the channel and not tell anyone, but then people will whine that all the users in the channel are secure and +Z isn't working... Oh, and I don't know which other IRCd does the +Z thing (or much of the details behind it), all I remember is reading about it somewhere and thinking that wasn't such a bad idea. |
|
|
+Z can all be handled locally. This will alleviate issues where servers (or services) falsely send +Z requests. Generally speaking the server should ignore all requests to set +Z and only set them if and when the server thinks it's appropriate. I wrote a function count_secure_users in the module linked in my above post) which could easily be converted to count_insecure_users, so a few hooks on PART, QUIT or KICK could simply ask: is channel +z? Are insecure users present? Is channel +Z? Set/unset +Z as necessary. This could also be done at remove_user_from_channel() (or the like). If performance is an issue, struct Channel could include a member like secureusers (similar to Channel::users) So who is going to implement this? As for ensuring that server-server links are SSL, I reckon that for simplicity's sake we simply rely on the server administrators to ensure that inter-server links are secure. Thinks would be more complicated if +Z could only be set when all server-server links are SSL. For example, what if server A-B is SSL, and B-C is non-ssl, and a channel exists which has exactly 5 SSL users from A and 5 SSL from B (and none from C). Should +Z be rejected because the irrelevant B-C link is non-SSL? Then again... what if B and C are both on the same machine connected via localhost? Then their being SSL is irrelevant. No, I definately say let's leave the server-server link security up to the admins. If it's really a problem perhaps a channel notice could be generated saying link A-B is non SSL. |
|
|
I like the idea of leaving the server linking up to the admins, however this will generate some concern with knowledgeable users. If we do a notice to warn users, we should perhaps not disclose link specifics and make the notice like "NOTICE @#Channel :All users in this channel are connected with SSL, however channel messages may travel in plain text between servers.", as well as send a onotice such as "Warning -- #Channel has +Z, however the link between serverA.example.com and serverB.example.com is not secure." Of course this would only be if there are users from both sides of the insecure link in the channel. (perhaps the onotice can be sent to the eyes snomask) |
|
|
Note: I do understand that more notes may be messy, annoying, and unwanted by many admins, so there should probable be a set option for disabling them (set::options::no-ssl-onotices or something)... I do suggest more notices with the hopes that Unreal can be one of the few IRCds that expresses concerns about connection security. There are many features Unreal *doesn't* have for the whole reasoning of "false sense of security", so why not add a couple things to keep people from having such a false sense :) |
|
|
I'll do it. Though, not this week, perhaps next.. we'll see. It's something I'd like to have in 3.2.9. As for taking serverlinks into account, I've thought about it, but I agree with fez here. We must leave this up to the server admins. Other servers can't see if far servers are SSL linked, and the localhost / safe localnet issue also came up in my mind. Let's not make this overcomplicated. |
|
|
The other issue that comes to mind, is what do you do with +S users that aren't secure. I have yet to see a ULine use secure links (I usually link them locally, then the ircd is ssl enabled, to avoid this) but the service bots won't have +z. So does the IRCD ignore the fact that ulined hosts aren't ssl? Again, do we follow the "Allow ULines to do anything" mentality, since some ulines probably aren't good to have non-ssl. Such as stats, and where the stats are stored (plain text databases, on a potentially exploitable system since it's not hidden from users) I personally would go with, (pseudo) if (LastInsecureUser(cptr) && IsSecure(cptr) && !IsULine(cptr)) set_+Z; Just ignoring if a ULine is on the channel, and letting it set +Z. Also, what happens when an Oper with can_override forces himself in? Do you continue to leave it +Z, or do you at that point, -zZ the channel or just -Z the channel? As currently, it let's you override with /join #channel override; and then it sets -z (which is interesting, since a +z on mlock, resets it and then the ircop is in a +z channel thanks to the ULine). Will the /join #channel override as a key, only unset -Z, or will the override even be required (once you /invite)? I think with this implementation, it's safe to remove the /join #channel override requirement, once an /invite has been ensued; and then just -Z but leave it +z for everyone else (the oper can then override a -z if required) Discuss? |
|
|
+S users (services) joining secure channels would be along the lines of leaving it to admins to set up "secure" (either ssl or localhost-localhost) server links, and letting them worry about that. I wouldn't worry about +S users joining +z channels since services are usually linked via localhost or at least LAN. |
|
|
> The other issue that comes to mind, is what do you do with +S users that aren't secure. > I have yet to see a ULine use secure links (I usually link them locally, then the ircd is ssl enabled, to avoid this) but the service bots won't have +z. > So does the IRCD ignore the fact that ulined hosts aren't ssl? This has been discussed many times. Sure there's no service package that supports SSL, but I have yet to see a sane network configuration where the services connection actually reaches a network where it would be vulnerable to eavesdropping. Most services are linked with a loopback connection (localhost, or the box's own IP). Also, for those netadmins with users that insist on linking servers over the internet, and are also concerned about keeping everything encrypted they would probably use sTunnel, or hack their own SSL connectivity into their services. This was also explained by Syzop's comment saying that keeping links secure is entirely up to the admin and the software will not warn/deny users to stop confusion. > Also, what happens when an Oper with can_override forces himself in? Do you continue to leave it +Z, or do you at that point, -zZ the channel or just -Z the channel? Opers with OperOverride using plaintext connections have never been able to force themselves into a +z channel without forcing -z on the channel. This will remain the same. As for services MLOCKing +z, +z will be readded but +Z will not be. +Z should ALWAYS be removed when -z is set. |
|
|
>Opers with OperOverride using plaintext connections have never been able to force themselves into a +z channel without forcing -z on the channel. Well, they don't explicitly have to set -z, the ircd does it for them (/invite, then /join #chan override). The question is, will the ircd set -z, or does it respond the same as if +z was set while a non-ssl user was on the channel. (it would simply set -Z, but leave +z) Same as, what happens when a user is /sajoin'ed into a +zZ channel, does it remove both -z and -Z, or just the -Z leaving it +z? To me, it makes sense to change the current behavior of setting -z, and instead leave +z (so new users can't join, using syzops proposal), and just setting -Z. Edit: Though, at this point. I think we are all kind of on the same page, I'm just saying it differently.. |
|
|
Why is this module not in the modules database? People do care, thanks for the module Fez! |
|
|
For tons of reasons, but nr 1 being that this will be solved in 3.2.9 itself! |
|
|
Committed to CVS. I would appreciate some testing before I actually mark this as 'resolved' :). I only did 'light' testing. - Some small updates to the extended channel mode system: it now has minimal support for 'local channel modes'. This is really only meant for channel mode +Z (upcase z), see next. - Added Channel Mode Z which indicates if a channel is 'secure' or not. This mode works in conjunction with +z (lower case z). If +z is set ('only secure users may join'), then the IRCd scans to see if everyone in the channel is connected through SSL. If so, then the channel is set +Z as well ('channel is secure'). Whenever an insecure user manages to join, the channel is -Z. And whenever all insecure users leave, the channel is set +Z. The 'insecure user being present in a +z channel' can be because: - An IRCOp joined the channel, and he's not secure - When servers link together and a user on the other side is not secure This only happens on net merge (equal time stamp). On different time stamp, we still kick insecure users on the new side. - At the time when +z is set, there are insecure users present. This feature was implemented after a heavy discussion in bug 0003720 by fez and others, and was suggested by Stealth. Tech note: +Z/-Z is handled locally by each server. Any attempt to remotely set +Z/-Z (eg: by services) will be ignored. - As mentioned above, +z can now be set even if any insecure users are present. Previously, this was not permitted. Now, as soon as the last non-SSL user leaves, the channel will be set +Z. - An oper not connected through SSL previously had to /INVITE himself to a channel and then /JOIN the channel with the key 'override'. This 'override' key is no longer required, a simple JOIN will suffice. - Sorted channel modes in /HELPOP ?CHMODES - Re-enabled 'fishy timestamp' errors in MODE. For some reason this was commented out, even though the (more annoying and less useful) code in JOIN was enabled so that did not make a lot of sense. It also now logs to ircd.log (or whatever you configure). This enables people to easier find the cause of any timestamp issues (which usually is badly coded services). |
|
|
I did some light testing aswell and this seems to work nicely, except for this minor thing: on a channel which has unsecure users, an operator sets it +z, now when the last unsecure user parts, a ":<server> MODE <channel> +Z" is also sent to that last unsecure client that just parted, this shouldn't be sent to that client since he is not on the channel anymore and shouldn't care (or even know) about that. Also, on a sidenote: for quote some time now I'm using this define in include/config.h: SECURECHANMSGSONLYGOTOSECURE - it seems as this define is forgotten about? Maybe this new cmode +Z can work together with that define? Or define it by default? Or just remove the define and force it enabled all the time? :) |
|
|
Regarding MODE +Z.. Ah yes, something small I missed indeed. I'll take a look... time for yet another _butone function. Regarding SECURECHANMSGSONLYGOTOSECURE, yes it's not really in the picture :P, but.. I don't see how anything of that would change with resolving this bug? The option can be quite confusing for users I think (just like +u is already), so I don't think it should be the default. If you have some other suggestion with it, though, like a Config -advanced option (if not already), or making it configurable in unrealircd.conf, and/or adding better documentation about it, then feel free to open up a new bug report for that. |
|
|
CVS .895: - Little tweak to +Z: when the last insecure user parts and the channel is set +Z (secure), the parting user saw the MODE too, which was silly. Reported by Robby22 (0003720). |
|
|
I'm going to mark this as 'resolved'. I hope to release a 3.2.9-RC1 soon (or whatever it will be called), so IF you find an issue with this new feature, then please report ASAP ;) |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2008-08-17 18:57 | fez | New Issue | |
| 2008-08-17 23:02 | fez | Note Added: 0015362 | |
| 2008-08-17 23:12 | nate | Status | new => acknowledged |
| 2008-08-17 23:48 | fez | Note Added: 0015363 | |
| 2008-08-18 05:40 | fez | Note Added: 0015365 | |
| 2008-08-18 05:47 | fez | Note Added: 0015366 | |
| 2008-08-18 05:54 | fez | Note Added: 0015367 | |
| 2008-08-26 23:13 | Bricker | Note Added: 0015375 | |
| 2008-08-26 23:28 | Bricker | Note Edited: 0015375 | |
| 2008-08-27 00:35 | fez | Note Added: 0015380 | |
| 2008-08-27 01:23 | fez | Note Added: 0015381 | |
| 2008-08-27 01:38 | fez | Note Edited: 0015365 | |
| 2008-08-27 06:25 | fez | Note Added: 0015382 | |
| 2008-11-23 20:49 | Strawberry_Kittens | Note Added: 0015427 | |
| 2008-11-23 22:11 | fez | Note Added: 0015428 | |
| 2010-02-13 14:19 | syzop | QA | => Not touched yet by developer |
| 2010-02-13 14:19 | syzop | U4: Need for upstream patch | => No need for upstream InspIRCd patch |
| 2010-02-13 14:19 | syzop | Note Added: 0016022 | |
| 2010-02-13 14:19 | syzop | Status | acknowledged => closed |
| 2010-02-13 14:19 | syzop | Resolution | open => wont fix |
| 2010-02-16 05:15 | fez | Note Added: 0016026 | |
| 2010-02-16 05:15 | fez | Status | closed => feedback |
| 2010-02-16 05:15 | fez | Resolution | wont fix => reopened |
| 2010-02-16 05:26 | Stealth | Note Added: 0016027 | |
| 2010-02-16 06:38 | fez | Note Edited: 0016026 | |
| 2010-02-16 18:51 | syzop | Note Added: 0016028 | |
| 2010-02-16 19:12 | fez | Note Added: 0016029 | |
| 2010-02-16 20:57 | syzop | Note Added: 0016030 | |
| 2010-02-16 23:10 | Stealth | Note Added: 0016031 | |
| 2010-02-17 01:19 | fez | Note Added: 0016032 | |
| 2010-02-17 01:51 | Stealth | Note Added: 0016033 | |
| 2010-02-17 01:55 | Stealth | Note Added: 0016034 | |
| 2010-02-17 11:33 | syzop | Note Added: 0016035 | |
| 2010-02-17 11:33 | syzop | Status | feedback => assigned |
| 2010-02-17 11:34 | syzop | Relationship added | child of 0003776 |
| 2010-04-03 12:17 | driew | Note Added: 0016052 | |
| 2010-04-03 12:20 | driew | Note Edited: 0016052 | |
| 2010-04-03 18:47 | fez | Note Added: 0016057 | |
| 2010-04-03 18:56 | Stealth | Note Added: 0016058 | |
| 2010-04-03 19:02 | driew | Note Added: 0016059 | |
| 2010-04-03 19:04 | driew | Note Edited: 0016059 | |
| 2010-04-03 19:04 | driew | Note Edited: 0016059 | |
| 2010-07-02 10:03 | syzop | Assigned To | => syzop |
| 2010-09-09 15:21 | chotaire | Note Added: 0016352 | |
| 2010-09-09 19:19 | syzop | Note Added: 0016355 | |
| 2010-11-13 20:25 | syzop | Note Added: 0016398 | |
| 2010-11-13 20:25 | syzop | Status | assigned => feedback |
| 2010-11-14 18:03 | Robby22 | Note Added: 0016399 | |
| 2010-11-15 14:55 | syzop | Note Added: 0016404 | |
| 2010-11-15 15:01 | syzop | Note Added: 0016405 | |
| 2010-11-17 17:06 | syzop | Note Added: 0016420 | |
| 2010-11-17 17:06 | syzop | Status | feedback => resolved |
| 2010-11-17 17:06 | syzop | Fixed in Version | => 3.2.9-RC1 |
| 2010-11-17 17:06 | syzop | Resolution | reopened => fixed |
