View Issue Details

IDProjectCategoryView StatusLast Update
0004008unrealircdpublic2012-08-17 10:31
Reporterkatsklaw Assigned Toohnobinki  
PrioritynormalSeverityfeatureReproducibilityhave not tried
Status resolvedResolutionfixed 
Product Version3.2.9-RC1 
Fixed in Version3.2.10-rc1 
Summary0004008: Add oper::require-modes (to enforce being registered with nickserv or having an SSL connection)
Descriptionoper::require-ssl Will make it so the user must have umode +z to successfully oper.
TagsNo tags attached.
Attached Files
unreal-4008-r2.patch (6,292 bytes)
3rd party modules

Activities

ohnobinki

2011-02-20 05:38

reporter   ~0016598

Here is the argument against using plaintext passwords at all ;-):

- If the oper's communication line should be secured (to prevent others from seeing what the oper can see, such as uncloaked hosts and communication which only opers can do), the oper should know enough to be using SSL.

- Also, requiring SSL in this manner makes no sense because it's too late to prevent the OPER from sending his password in plaintext by the time this check would tell him to use SSL. However, having a require-ssl option would be helpful to bug an oper who is setting up an IRC client.

- Lastly, if opers are actually worried about security, they should be using the ``password "certs/joe.pem" { sslclientcert; };'' construct. This requires the oper to connect and present an SSL certificate (which essentially requires the connection to be encrypted in the first place). ( http://unrealircd.org/docs.php#operblock_password_sslclientcert ).

Thus, IMO, providing the oper::options::require-ssl option hides the above problems with plaintext password authentication from the server admins.

On the other hand, one might want to restrict OPERing up to include other usermodes, such as +r -- should/could this be implemented as a oper::require-modes option instead?

katsklaw

2011-02-20 06:54

reporter   ~0016599

Last edited: 2011-02-20 10:54

1> I personally already use certs and highly recommend it.

2> you are correct by stating the oper pass is exposed in non-ssl sessions, however, my counter argument is that in said scenario, if I require ssl sessions for my opers then if they don't use ssl, the oper's password is the only thing exposed in plaintext and not my users passwords and other sensitive data as well, such as the afore mentioned uncloaked hosts.

Not all opers understand the importance of ssl when used properly, as an admin I can't be around 24/7 to insure my opers use ssl. thus requiring it in the oper block I can.

This request isn't about securing the opers password per se as much as securing the exposure of user's sensitive data.

3> not all clients can support the use of sslcerts, even those that do support ssl connections. by atleast using umode +z the datastream is then encrypted even if the oper pass is not a cert.

4> oper::require-modes would indeed be a better feature :)

katsklaw

2011-07-13 05:22

reporter   ~0016683

Can we get a bump on this?

I like Binki's oper::require-modes solution. What's the difficulty level on this? I haven't really messed with adding things to config parser.

ohnobinki

2011-07-13 19:20

reporter   ~0016684

unreal-oper-require-modes.patch:

Adds oper::require-modes, a string of modes a user must have before /OPER will work for him.

You are not able to disallow users with certain modes from becoming oper -- i.e., this requirement is only `positive'.

I'm not sure how the error messages/numerics should look like for when the user doens't have the right modes. For now, I'm just using the `your host doesn't have an oper block' message.

ohnobinki

2011-07-13 19:22

reporter   ~0016686

Oh, also upon a user failing to meet the requirements, that patch has it so that the following gets logged to ircd.log under the `oper' category:

[Wed Jul 13 12:59:32 2011] - OPER MISSINGMODES (binki) by (binki!a@localhost), needs modes=+x

It would list only the modes which the user is missing which he needs to attain before being allowed to /oper.

katsklaw

2011-07-14 03:29

reporter   ~0016689

Last edited: 2011-07-14 03:29

I'll likely make it use an unused numeric and share the patch. I seriously dislike vague errors :/

Thanks! :)

ohnobinki

2011-07-14 04:40

reporter   ~0016690

unreal-4008-r2.patch:

Use random new numeric:
OPER binki <pass>
:irc.foonet.com 554 bonki :You are missing user modes required to OPER
MODE bonki +x
:bonki MODE bonki :+x
OPER binki <pass>
:bonki MODE bonki :+owghaAsN
:irc.foonet.com 008 bonki :Server notice mask (+kcfvGqso)
:irc.foonet.com 381 bonki :You are now an IRC Operator

Has docs changes.

warg

2011-07-14 04:43

reporter   ~0016691

Don't waste a mode for this.

oper name { options { ssl; }; };

Require an SSL connection to operup. Otherwise, return the standard error message, No o-lines for your host.

We don't use spiffy error messages on such things because it's not appropriate to tell a user _exactly_ why they couldn't operup, just as it wouldn't be appropriate to tell someone they can't login because their username doesn't exist, etc.

katsklaw

2011-07-14 04:58

reporter   ~0016692

wow, thanks binki! ;)

Warg, we'll just have to agree to disagree about the vagueness of error messages. "Sorry it didn't work" type messages are counter productive and cause admins to spend needless time figuring out the root cause. "you are missing usermodes" is specific enough to point the admin in the correct direction to trouble shoot without telling the oper-to-be exactly which mode is needed in the odd event someone is trying to compromise security.

syzop

2012-02-26 21:58

administrator   ~0016924

Hm, sounds good.

As for giving a hint or not. I'm not sure, there are good arguments in favor and against it. I tend to go with binki/katsklaw here though.. trying to imagine what others would prefer when they use this feature, they would probably want some kind of hint. Whatever we decide, the feature would be an improvement.

As for the numeric, use ERR_NOOPERHOST but with a different text hinting on the user mode. AFAICT that is allowed, it's just that our current code doesn't provide or make use of it.

syzop

2012-08-17 10:30

administrator   ~0017069

Patch applied. With changes as discussed in previous comment. Also moved the check down to after authentication, similar to maxlogins, so the error is only shown when you enter the right user name & password, good compromise I think.

http://hg.unrealircd.com/hg/unreal/rev/7d0e77e30087 and http://hg.unrealircd.com/hg/unreal/rev/a6525074d8e2

Issue History

Date Modified Username Field Change
2011-02-20 05:09 katsklaw New Issue
2011-02-20 05:38 ohnobinki Note Added: 0016598
2011-02-20 06:54 katsklaw Note Added: 0016599
2011-02-20 10:53 katsklaw Note Edited: 0016599
2011-02-20 10:54 katsklaw Note Edited: 0016599
2011-07-13 05:22 katsklaw Note Added: 0016683
2011-07-13 19:18 ohnobinki Summary Add oper::require-ssl => Add oper::require-modes (to enforce being registered with nickserv or having an SSL connection)
2011-07-13 19:18 ohnobinki File Added: unreal-oper-require-modes.patch
2011-07-13 19:20 ohnobinki Note Added: 0016684
2011-07-13 19:22 ohnobinki Note Added: 0016686
2011-07-14 03:29 katsklaw Note Added: 0016689
2011-07-14 03:29 katsklaw Note Edited: 0016689
2011-07-14 04:39 ohnobinki File Added: unreal-4008-r2.patch
2011-07-14 04:40 ohnobinki Note Added: 0016690
2011-07-14 04:41 ohnobinki Status new => has patch
2011-07-14 04:43 warg Note Added: 0016691
2011-07-14 04:58 katsklaw Note Added: 0016692
2012-02-26 21:58 syzop Note Added: 0016924
2012-08-17 10:30 syzop Note Added: 0017069
2012-08-17 10:30 syzop Status has patch => resolved
2012-08-17 10:30 syzop Fixed in Version => 3.2.10-rc1
2012-08-17 10:30 syzop Resolution open => fixed
2012-08-17 10:30 syzop Assigned To => syzop
2012-08-17 10:31 syzop Assigned To syzop => ohnobinki