From e143aaa83fd8a5e357de77305f7f2fe251b08eaa Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 18 Dec 2019 15:44:06 -0500 Subject: [PATCH 1/6] fix #696 --- irc/commands.go | 2 +- irc/config.go | 3 +++ irc/handlers.go | 10 +++++++--- irc/help.go | 2 +- irc/utils/crypto.go | 13 +++++++++++++ irc/utils/crypto_test.go | 18 ++++++++++++++++++ oragono.yaml | 5 ++++- 7 files changed, 47 insertions(+), 6 deletions(-) diff --git a/irc/commands.go b/irc/commands.go index 24f8e5b8..31aa6c18 100644 --- a/irc/commands.go +++ b/irc/commands.go @@ -206,7 +206,7 @@ func init() { }, "OPER": { handler: operHandler, - minParams: 2, + minParams: 1, }, "PART": { handler: partHandler, diff --git a/irc/config.go b/irc/config.go index 464bcb00..09e01f8e 100644 --- a/irc/config.go +++ b/irc/config.go @@ -211,6 +211,7 @@ type OperConfig struct { Vhost string WhoisLine string `yaml:"whois-line"` Password string + Certfp string Modes string } @@ -459,6 +460,7 @@ type Oper struct { WhoisLine string Vhost string Pass []byte + Certfp string Modes []modes.ModeChange } @@ -479,6 +481,7 @@ func (conf *Config) Operators(oc map[string]*OperClass) (map[string]*Oper, error if err != nil { return nil, err } + oper.Certfp = opConf.Certfp oper.Vhost = opConf.Vhost class, exists := oc[opConf.Class] diff --git a/irc/handlers.go b/irc/handlers.go index dbcc5cb7..74381015 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2170,7 +2170,7 @@ func npcaHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp return false } -// OPER +// OPER [password] func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *ResponseBuffer) bool { if client.HasMode(modes.Operator) { rb.Add(nil, server.name, ERR_UNKNOWNERROR, client.Nick(), "OPER", client.t("You're already opered-up!")) @@ -2180,8 +2180,12 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp authorized := false oper := server.GetOperator(msg.Params[0]) if oper != nil { - password := []byte(msg.Params[1]) - authorized = (bcrypt.CompareHashAndPassword(oper.Pass, password) == nil) + if utils.CertfpsMatch(oper.Certfp, client.certfp) { + authorized = true + } else if 1 < len(msg.Params) { + password := []byte(msg.Params[1]) + authorized = (bcrypt.CompareHashAndPassword(oper.Pass, password) == nil) + } } if !authorized { rb.Add(nil, server.name, ERR_PASSWDMISMATCH, client.Nick(), client.t("Password incorrect")) diff --git a/irc/help.go b/irc/help.go index 91d751be..c6a96963 100644 --- a/irc/help.go +++ b/irc/help.go @@ -349,7 +349,7 @@ The NPC command is used to send an action to the target as the source. Requires the roleplay mode (+E) to be set on the target.`, }, "oper": { - text: `OPER + text: `OPER [password] If the correct details are given, gives you IRCop privs.`, }, diff --git a/irc/utils/crypto.go b/irc/utils/crypto.go index a725b547..be4247ff 100644 --- a/irc/utils/crypto.go +++ b/irc/utils/crypto.go @@ -8,6 +8,7 @@ import ( "crypto/subtle" "encoding/base32" "encoding/base64" + "strings" ) var ( @@ -68,3 +69,15 @@ func GenerateSecretKey() string { rand.Read(buf[:]) return base64.RawURLEncoding.EncodeToString(buf[:]) } + +func normalizeCertfp(certfp string) string { + return strings.ToLower(strings.Replace(certfp, ":", "", -1)) +} + +// Convenience to compare certfps as returned by different tools, e.g., openssl vs. oragono +func CertfpsMatch(storedCertfp, suppliedCertfp string) bool { + if storedCertfp == "" { + return false + } + return normalizeCertfp(storedCertfp) == normalizeCertfp(suppliedCertfp) +} diff --git a/irc/utils/crypto_test.go b/irc/utils/crypto_test.go index c00ebad3..0b0b6967 100644 --- a/irc/utils/crypto_test.go +++ b/irc/utils/crypto_test.go @@ -81,3 +81,21 @@ func BenchmarkMungeSecretToken(b *testing.B) { t = MungeSecretToken(t) } } + +func TestCertfpComparisons(t *testing.T) { + opensslFP := "3D:6B:11:BF:B4:05:C3:F8:4B:38:CD:30:38:FB:EC:01:71:D5:03:54:79:04:07:88:4C:A5:5D:23:41:85:66:C9" + oragonoFP := "3d6b11bfb405c3f84b38cd3038fbec0171d50354790407884ca55d23418566c9" + badFP := "3d6b11bfb405c3f84b38cd3038fbec0171d50354790407884ca55d23418566c8" + if !CertfpsMatch(opensslFP, oragonoFP) { + t.Error("these certs should match") + } + if !CertfpsMatch(oragonoFP, opensslFP) { + t.Error("these certs should match") + } + if CertfpsMatch("", "") { + t.Error("empty stored certfp should not match empty provided certfp") + } + if CertfpsMatch(opensslFP, badFP) { + t.Error("these certs should not match") + } +} diff --git a/oragono.yaml b/oragono.yaml index 1f2d9ef4..f805bf06 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -121,7 +121,7 @@ server: # one webirc block -- should correspond to one set of gateways - # tls fingerprint the gateway must connect with to use this webirc block - fingerprint: 938dd33f4b76dcaf7ce5eb25c852369cb4b8fb47ba22fc235aa29c6623a5f182 + fingerprint: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" # password the gateway uses to connect, made with oragono genpasswd password: "$2a$04$sLEFDpIOyUp55e6gTMKbOeroT6tMXTjPFvA0eGvwvImVR9pkwv7ee" @@ -449,6 +449,9 @@ opers: # generated using "oragono genpasswd" password: "$2a$04$LiytCxaY0lI.guDj2pBN4eLRD5cdM2OLDwqmGAgB6M2OPirbF5Jcu" + # being logged in with this client cert will let you /OPER without a password + certfp: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + # logging, takes inspiration from Insp logging: - From 98e83b0a828dd6ee85629b052a982d79ac36497f Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 18 Dec 2019 17:53:12 -0500 Subject: [PATCH 2/6] add clarifying comments --- oragono.yaml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/oragono.yaml b/oragono.yaml index f805bf06..750ab112 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -120,10 +120,11 @@ server: webirc: # one webirc block -- should correspond to one set of gateways - - # tls fingerprint the gateway must connect with to use this webirc block + # SHA-256 fingerprint of the TLS certificate the gateway must use to connect + # (comment this out to use passwords only) fingerprint: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" - # password the gateway uses to connect, made with oragono genpasswd + # password the gateway uses to connect, made with oragono genpasswd password: "$2a$04$sLEFDpIOyUp55e6gTMKbOeroT6tMXTjPFvA0eGvwvImVR9pkwv7ee" # addresses/CIDRs that can use this webirc command @@ -449,7 +450,8 @@ opers: # generated using "oragono genpasswd" password: "$2a$04$LiytCxaY0lI.guDj2pBN4eLRD5cdM2OLDwqmGAgB6M2OPirbF5Jcu" - # being logged in with this client cert will let you /OPER without a password + # if you're logged in using the client cert with this SHA-256 fingerprint, + # you'll be able to /OPER without a password certfp: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" # logging, takes inspiration from Insp From 6033d9f56986bf2642d66c2476add86943f56ebf Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 18 Dec 2019 20:33:50 -0500 Subject: [PATCH 3/6] tweaks for consistency --- irc/config.go | 28 ++++++++++++++-------------- irc/handlers.go | 4 ++-- oragono.yaml | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/irc/config.go b/irc/config.go index 09e01f8e..e00a9bac 100644 --- a/irc/config.go +++ b/irc/config.go @@ -207,12 +207,12 @@ type OperClassConfig struct { // OperConfig defines a specific operator's configuration. type OperConfig struct { - Class string - Vhost string - WhoisLine string `yaml:"whois-line"` - Password string - Certfp string - Modes string + Class string + Vhost string + WhoisLine string `yaml:"whois-line"` + Password string + Fingerprint string + Modes string } // LineLenConfig controls line lengths. @@ -455,13 +455,13 @@ func (conf *Config) OperatorClasses() (map[string]*OperClass, error) { // Oper represents a single assembled operator's config. type Oper struct { - Name string - Class *OperClass - WhoisLine string - Vhost string - Pass []byte - Certfp string - Modes []modes.ModeChange + Name string + Class *OperClass + WhoisLine string + Vhost string + Pass []byte + Fingerprint string + Modes []modes.ModeChange } // Operators returns a map of operator configs from the given OperClass and config. @@ -481,7 +481,7 @@ func (conf *Config) Operators(oc map[string]*OperClass) (map[string]*Oper, error if err != nil { return nil, err } - oper.Certfp = opConf.Certfp + oper.Fingerprint = opConf.Fingerprint oper.Vhost = opConf.Vhost class, exists := oc[opConf.Class] diff --git a/irc/handlers.go b/irc/handlers.go index 74381015..62f509a2 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2180,7 +2180,7 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp authorized := false oper := server.GetOperator(msg.Params[0]) if oper != nil { - if utils.CertfpsMatch(oper.Certfp, client.certfp) { + if utils.CertfpsMatch(oper.Fingerprint, client.certfp) { authorized = true } else if 1 < len(msg.Params) { password := []byte(msg.Params[1]) @@ -2645,7 +2645,7 @@ func webircHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Re if 0 < len(info.Password) && bcrypt.CompareHashAndPassword(info.Password, givenPassword) != nil { continue } - if 0 < len(info.Fingerprint) && client.certfp != info.Fingerprint { + if 0 < len(info.Fingerprint) && !utils.CertfpsMatch(info.Fingerprint, client.certfp) { continue } diff --git a/oragono.yaml b/oragono.yaml index 750ab112..d5638f41 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -452,7 +452,7 @@ opers: # if you're logged in using the client cert with this SHA-256 fingerprint, # you'll be able to /OPER without a password - certfp: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + fingerprint: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" # logging, takes inspiration from Insp logging: From b717402b5e7cfccab1e6cd7e353d5ebb069cabf0 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 19 Dec 2019 06:33:43 -0500 Subject: [PATCH 4/6] implement review feedback 1. If both fingerprint and hash are specified, require both instead of either 2. Implement auto-oper on connect --- irc/client.go | 15 +++++++++++++++ irc/config.go | 15 ++++++++++++--- irc/handlers.go | 40 +++++++++++++++++++++++++--------------- irc/responsebuffer.go | 12 ++++++++++++ irc/server.go | 3 +++ oragono.yaml | 16 +++++++++++----- 6 files changed, 78 insertions(+), 23 deletions(-) diff --git a/irc/client.go b/irc/client.go index e3ef9034..9c95c3bf 100644 --- a/irc/client.go +++ b/irc/client.go @@ -1354,3 +1354,18 @@ func (client *Client) CheckInvited(casefoldedChannel string) (invited bool) { delete(client.invitedTo, casefoldedChannel) return } + +// Implements auto-oper by certfp (scans for an auto-eligible operator block that matches +// the client's cert, then applies it). +func (client *Client) attemptAutoOper(session *Session) { + if client.certfp == "" || client.HasMode(modes.Operator) { + return + } + for _, oper := range client.server.Config().operators { + if oper.Auto && oper.Pass == nil && utils.CertfpsMatch(oper.Fingerprint, client.certfp) { + rb := NewResponseBuffer(session) + applyOper(client, oper, rb) + rb.Send(true) + } + } +} diff --git a/irc/config.go b/irc/config.go index e00a9bac..3d0fe34d 100644 --- a/irc/config.go +++ b/irc/config.go @@ -212,6 +212,7 @@ type OperConfig struct { WhoisLine string `yaml:"whois-line"` Password string Fingerprint string + Auto bool Modes string } @@ -461,6 +462,7 @@ type Oper struct { Vhost string Pass []byte Fingerprint string + Auto bool Modes []modes.ModeChange } @@ -477,11 +479,18 @@ func (conf *Config) Operators(oc map[string]*OperClass) (map[string]*Oper, error } oper.Name = name - oper.Pass, err = decodeLegacyPasswordHash(opConf.Password) - if err != nil { - return nil, err + if opConf.Password != "" { + oper.Pass, err = decodeLegacyPasswordHash(opConf.Password) + if err != nil { + return nil, err + } } oper.Fingerprint = opConf.Fingerprint + oper.Auto = opConf.Auto + + if oper.Pass == nil && oper.Fingerprint == "" { + return nil, fmt.Errorf("Oper %s has neither a password nor a fingerprint", name) + } oper.Vhost = opConf.Vhost class, exists := oc[opConf.Class] diff --git a/irc/handlers.go b/irc/handlers.go index 62f509a2..2c3347c1 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2177,14 +2177,19 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp return false } - authorized := false + // must have a matching oper block and not fail any enabled checks + // (config validation ensures that there is at least one check) oper := server.GetOperator(msg.Params[0]) + authorized := oper != nil if oper != nil { - if utils.CertfpsMatch(oper.Fingerprint, client.certfp) { - authorized = true - } else if 1 < len(msg.Params) { - password := []byte(msg.Params[1]) - authorized = (bcrypt.CompareHashAndPassword(oper.Pass, password) == nil) + if oper.Fingerprint != "" && !utils.CertfpsMatch(oper.Fingerprint, client.certfp) { + authorized = false + } else if oper.Pass != nil { + if len(msg.Params) == 1 { + authorized = false + } else if bcrypt.CompareHashAndPassword(oper.Pass, []byte(msg.Params[1])) != nil { + authorized = false + } } } if !authorized { @@ -2193,9 +2198,17 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp return true } - oldNickmask := client.NickMaskString() + applyOper(client, oper, rb) + return false +} + +// applies operator status to a client, who MUST NOT already be an operator +func applyOper(client *Client, oper *Oper, rb *ResponseBuffer) { + details := client.Details() + oldNickmask := details.nickMask client.SetOper(oper) - if client.NickMaskString() != oldNickmask { + newNickmask := client.NickMaskString() + if newNickmask != oldNickmask { client.sendChghost(oldNickmask, oper.Vhost) } @@ -2208,17 +2221,14 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp copy(modeChanges[1:], oper.Modes) applied := ApplyUserModeChanges(client, modeChanges, true) - rb.Add(nil, server.name, RPL_YOUREOPER, client.nick, client.t("You are now an IRC operator")) - rb.Add(nil, server.name, "MODE", client.nick, applied.String()) + client.server.snomasks.Send(sno.LocalOpers, fmt.Sprintf(ircfmt.Unescape("Client opered up $c[grey][$r%s$c[grey], $r%s$c[grey]]"), client.nickMaskString, oper.Name)) - server.snomasks.Send(sno.LocalOpers, fmt.Sprintf(ircfmt.Unescape("Client opered up $c[grey][$r%s$c[grey], $r%s$c[grey]]"), client.nickMaskString, oper.Name)) - - // client may now be unthrottled by the fakelag system + rb.Broadcast(nil, client.server.name, RPL_YOUREOPER, details.nick, client.t("You are now an IRC operator")) + rb.Broadcast(nil, client.server.name, "MODE", details.nick, applied.String()) for _, session := range client.Sessions() { + // client may now be unthrottled by the fakelag system session.resetFakelag() } - - return false } // PART {,} [] diff --git a/irc/responsebuffer.go b/irc/responsebuffer.go index 9df090a0..7fcb1b8f 100644 --- a/irc/responsebuffer.go +++ b/irc/responsebuffer.go @@ -77,6 +77,18 @@ func (rb *ResponseBuffer) Add(tags map[string]string, prefix string, command str rb.AddMessage(ircmsg.MakeMessage(tags, prefix, command, params...)) } +// Broadcast adds a standard new message to our queue, then sends an unlabeled copy +// to all other sessions. +func (rb *ResponseBuffer) Broadcast(tags map[string]string, prefix string, command string, params ...string) { + // can't reuse the IrcMessage object because of tag pollution :-\ + rb.Add(tags, prefix, command, params...) + for _, session := range rb.session.client.Sessions() { + if session != rb.session { + session.Send(tags, prefix, command, params...) + } + } +} + // AddFromClient adds a new message from a specific client to our queue. func (rb *ResponseBuffer) AddFromClient(time time.Time, msgid string, fromNickMask string, fromAccount string, tags map[string]string, command string, params ...string) { msg := ircmsg.MakeMessage(nil, fromNickMask, command, params...) diff --git a/irc/server.go b/irc/server.go index 5c56ac4d..bb498139 100644 --- a/irc/server.go +++ b/irc/server.go @@ -440,6 +440,9 @@ func (server *Server) playRegistrationBurst(session *Session) { if modestring != "+" { session.Send(nil, d.nickMask, RPL_UMODEIS, d.nick, modestring) } + + c.attemptAutoOper(session) + if server.logger.IsLoggingRawIO() { session.Send(nil, c.server.name, "NOTICE", d.nick, c.t("This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.")) } diff --git a/oragono.yaml b/oragono.yaml index d5638f41..10640958 100644 --- a/oragono.yaml +++ b/oragono.yaml @@ -446,13 +446,19 @@ opers: # modes are the modes to auto-set upon opering-up modes: +is acjknoqtux - # password to login with /OPER command - # generated using "oragono genpasswd" + # operators can be authenticated either by password (with the /OPER command), + # or by certificate fingerprint, or both. if a password hash is set, then a + # password is required to oper up (e.g., /OPER dan mypassword). to generate + # the hash, use `oragono genpasswd`. password: "$2a$04$LiytCxaY0lI.guDj2pBN4eLRD5cdM2OLDwqmGAgB6M2OPirbF5Jcu" - # if you're logged in using the client cert with this SHA-256 fingerprint, - # you'll be able to /OPER without a password - fingerprint: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + # if a SHA-256 certificate fingerprint is configured here, then it will be + # required to /OPER. if you comment out the password hash above, then you can + # /OPER without a password. + #fingerprint: "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789" + # if 'auto' is set (and no password hash is set), operator permissions will be + # granted automatically as soon as you connect with the right fingerprint. + #auto: true # logging, takes inspiration from Insp logging: From 78da024b24cb514fe070ecb55217b2431fd70178 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 19 Dec 2019 09:30:49 -0500 Subject: [PATCH 5/6] improve an error message --- irc/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/irc/config.go b/irc/config.go index 3d0fe34d..58fa60da 100644 --- a/irc/config.go +++ b/irc/config.go @@ -482,7 +482,7 @@ func (conf *Config) Operators(oc map[string]*OperClass) (map[string]*Oper, error if opConf.Password != "" { oper.Pass, err = decodeLegacyPasswordHash(opConf.Password) if err != nil { - return nil, err + return nil, fmt.Errorf("Oper %s has an invalid password hash: %s", oper.Name, err.Error()) } } oper.Fingerprint = opConf.Fingerprint From 01488bfe2eba235a0af7c669c5ba2a81b9c67131 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Thu, 19 Dec 2019 18:30:19 -0500 Subject: [PATCH 6/6] slightly more defensive implementation of /OPER check --- irc/handlers.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/irc/handlers.go b/irc/handlers.go index 2c3347c1..79df2fdb 100644 --- a/irc/handlers.go +++ b/irc/handlers.go @@ -2177,22 +2177,27 @@ func operHandler(server *Server, client *Client, msg ircmsg.IrcMessage, rb *Resp return false } - // must have a matching oper block and not fail any enabled checks - // (config validation ensures that there is at least one check) + // must pass at least one check, and all enabled checks + var checkPassed, checkFailed bool oper := server.GetOperator(msg.Params[0]) - authorized := oper != nil if oper != nil { - if oper.Fingerprint != "" && !utils.CertfpsMatch(oper.Fingerprint, client.certfp) { - authorized = false - } else if oper.Pass != nil { - if len(msg.Params) == 1 { - authorized = false - } else if bcrypt.CompareHashAndPassword(oper.Pass, []byte(msg.Params[1])) != nil { - authorized = false + if oper.Fingerprint != "" { + if utils.CertfpsMatch(oper.Fingerprint, client.certfp) { + checkPassed = true + } else { + checkFailed = true + } + } + if !checkFailed && oper.Pass != nil { + if len(msg.Params) == 1 || bcrypt.CompareHashAndPassword(oper.Pass, []byte(msg.Params[1])) != nil { + checkFailed = true + } else { + checkPassed = true } } } - if !authorized { + + if !checkPassed || checkFailed { rb.Add(nil, server.name, ERR_PASSWDMISMATCH, client.Nick(), client.t("Password incorrect")) client.Quit(client.t("Password incorrect"), rb.session) return true