Skip to content

SASL EXTERNAL mechanisms #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
reynir wants to merge 2 commits into
base: main
Choose a base branch
from
Open

SASL EXTERNAL mechanisms #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a78c84
more SASL mechanisms
reynir Feb 18, 2022
0184d5b
Apply suggestions from code review
reynir Mar 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions src/core/irc_client.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module Log = Irc_helpers.Log

type sasl = [`Plain | `External]

module type CLIENT = sig
module Io : sig
type 'a t
Expand Down Expand Up @@ -42,15 +44,15 @@ module type CLIENT = sig

val connect :
?username:string -> ?mode:int -> ?realname:string -> ?password:string ->
?sasl:bool -> ?config:Io.config ->
?sasl:sasl option -> ?config:Io.config ->
addr:Io.inet_addr -> port:int -> nick:string -> unit ->
connection_t Io.t
(** Connect to an IRC server at address [addr]. The PASS command will be
sent if [password] is not None. *)

val connect_by_name :
?username:string -> ?mode:int -> ?realname:string -> ?password:string ->
?sasl:bool -> ?config:Io.config ->
?sasl:sasl option -> ?config:Io.config ->
server:string -> port:int -> nick:string -> unit ->
connection_t option Io.t
(** Try to resolve the [server] name using DNS, otherwise behaves like
Expand Down Expand Up @@ -142,8 +144,8 @@ module Make(Io: Irc_transport.IO) = struct
let send_nick ~connection ~nick =
send ~connection (M.nick nick)

let send_auth_sasl ~connection ~user ~password =
Log.debug (fun k->k"login using SASL with user=%S" user);
let send_auth_sasl_plain ~connection ~user ~password =
Log.debug (fun k->k"login using SASL/plain with user=%S" user);
send_raw ~connection ~data:"CAP REQ :sasl" >>= fun () ->
send_raw ~connection ~data:"AUTHENTICATE PLAIN" >>= fun () ->
let b64_login =
Expand All @@ -153,6 +155,13 @@ module Make(Io: Irc_transport.IO) = struct
let data = Printf.sprintf "AUTHENTICATE %s" b64_login in
send_raw ~connection ~data

let send_auth_sasl_external ~connection ~user =
Log.debug (fun k -> k "login using SASL/external with user=%S" user);
send_raw ~connection ~data:"CAP REQ :sasl" >>= fun () ->
send_raw ~connection ~data:"AUTHENTICATE EXTERNAL" >>= fun () ->
let data = "AUTHENTICATE " ^ Base64.encode_string user in
send_raw ~connection ~data

let send_pass ~connection ~password =
send ~connection (M.pass password)

Expand Down Expand Up @@ -265,17 +274,20 @@ module Make(Io: Irc_transport.IO) = struct

let connect
?username ?(mode=0) ?(realname="irc-client")
?password ?(sasl=true) ?config ~addr ~port ~nick () =
?password ?(sasl=Some `Plain) ?config ~addr ~port ~nick () =
Io.open_socket ?config addr port >>= (fun sock ->
let connection = mk_connection_ sock in

let cap_end = ref false in
begin
match username, password with
| Some user, Some password when sasl ->
match username, password, sasl with
| Some user, Some password, Some `Plain ->
cap_end := true;
send_auth_sasl_plain ~connection ~user ~password
| Some user, _, Some `External ->
cap_end := true;
send_auth_sasl ~connection ~user ~password
| _, Some password -> send_pass ~connection ~password
send_auth_sasl_external ~connection ~user
| _, Some password, None -> send_pass ~connection ~password
| _ -> return ()

@reynir reynir Mar 4, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should match on etc None, _, Some Plain | _, None, Some Plain and issue an error instead of silently skipping SASL authentication. What do you think, @johnelse ?

@johnelse johnelse Mar 4, 2022

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is going to be a breaking change anyway, we could go even further and do something like

type auth = {
    password : string;
    sasl : [`Plain | `External] option
}

and have connect take an auth option. That way, there's no way of requesting any kind of SASL without also supplying a password.

@reynir reynir Mar 4, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that makes sense. Can I suggest that auth also carries a username field? It is not always the case that you want to authenticate with the same username as the IRC user.

The SASL external mechanism does not use the password for anything. Maybe the type should have a different shape.

end
>>= fun () ->
Expand Down
4 changes: 2 additions & 2 deletions src/core/irc_client.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module type CLIENT = sig

val connect :
?username:string -> ?mode:int -> ?realname:string -> ?password:string ->
?sasl:bool -> ?config:Io.config ->
?sasl:[`Plain | `External] option -> ?config:Io.config ->
addr:Io.inet_addr -> port:int -> nick:string -> unit ->
connection_t Io.t
(** Connect to an IRC server at address [addr]. The PASS command will be
Expand All @@ -56,7 +56,7 @@ module type CLIENT = sig

val connect_by_name :
?username:string -> ?mode:int -> ?realname:string -> ?password:string ->
?sasl:bool -> ?config:Io.config ->
?sasl:[`Plain | `External] option -> ?config:Io.config ->
server:string -> port:int -> nick:string -> unit ->
connection_t option Io.t
(** Try to resolve the [server] name using DNS, otherwise behaves like
Expand Down