From 42a4b4474f639161ce1b55b52437ba405d910498 Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Sun, 29 Sep 2024 08:25:29 +0100 Subject: [PATCH 1/4] Add the local machine's hostname to the certificate alternative names. This will (probably) be useful for those people using pizauth on a remote machine accessed via ssh. --- Cargo.lock | 1 + Cargo.toml | 1 + src/server/http_server.rs | 9 +++++++-- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b4470c1..739fcd1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -651,6 +651,7 @@ dependencies = [ "chrono", "getopts", "getrandom", + "hostname", "libc", "log", "lrlex", diff --git a/Cargo.toml b/Cargo.toml index 2ba49f7..afdf002 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ chacha20poly1305 = "0.10" chrono = "0.4" getopts = "0.2" getrandom = "0.2" +hostname = "0.4" log = "0.4" lrlex = "0.13" lrpar = "0.13" diff --git a/src/server/http_server.rs b/src/server/http_server.rs index 2219c1e..e41b849 100644 --- a/src/server/http_server.rs +++ b/src/server/http_server.rs @@ -390,8 +390,13 @@ pub fn https_server_setup( let _ = rustls::crypto::ring::default_provider().install_default(); // Generate self-signed certificate - let cert = - generate_simple_self_signed(vec![String::from("localhost"), String::from("127.0.0.1")])?; + let mut names = vec![String::from("localhost"), String::from("127.0.0.1")]; + if let Ok(x) = hostname::get() { + if let Some(x) = x.to_str() { + names.push(String::from(x)); + } + } + let cert = generate_simple_self_signed(names)?; // Bind TCP port for HTTPS let listener = TcpListener::bind(&conf.https_listen)?; From 15828b49d2d57128a23b8aeedc44c6b5f7781d4b Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Sun, 29 Sep 2024 08:34:25 +0100 Subject: [PATCH 2/4] Document the HTTPS listen option. --- pizauth.conf.5 | 6 ++++++ src/config.rs | 1 + 2 files changed, 7 insertions(+) diff --git a/pizauth.conf.5 b/pizauth.conf.5 index 4346d82..be98f29 100644 --- a/pizauth.conf.5 +++ b/pizauth.conf.5 @@ -42,6 +42,12 @@ specifies the address for the HTTP server to listen on. Defaults to .Qq 127.0.0.1:0 . +.It Sy https_listen = Qo Em bind-name Qc ; +specifies the address for the +.Xr pizauth 1 +HTTPS server to listen on. +Defaults to +.Qq 127.0.0.1:0 . .It Sy transient_error_if_cmd = Qo Em shell-cmd Qc ; specifies a shell command to be run when pizauth repeatedly encounters errors when trying to refresh a token. diff --git a/src/config.rs b/src/config.rs index 4cccb5a..5c36d16 100644 --- a/src/config.rs +++ b/src/config.rs @@ -26,6 +26,7 @@ const REFRESH_RETRY_DEFAULT: Duration = Duration::from_secs(40); const AUTH_NOTIFY_INTERVAL_DEFAULT: u64 = 15 * 60; /// What is the default bind() address for the HTTP server? const HTTP_LISTEN_DEFAULT: &str = "127.0.0.1:0"; +/// What is the default bind() address for the HTTPS server? const HTTPS_LISTEN_DEFAULT: &str = "127.0.0.1:0"; #[derive(Debug)] From cd9e1b059ef4b370c9962777dd0aa44b061f3cc5 Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Sun, 29 Sep 2024 09:15:25 +0100 Subject: [PATCH 3/4] Make the HTTP and HTTPS servers optional. The main use case for this is someone who wants to ensure that only the HTTPS server is active (i.e. that they can't accidentally connect via HTTP). But making both of them optional keeps the system more consistent, and someone might have a use for running the HTTP but not the HTTPS server! --- pizauth.conf.5 | 12 ++- src/config.l | 3 +- src/config.rs | 206 ++++++++++++++++++++++++++++++++++---- src/config.y | 2 + src/config_ast.rs | 2 + src/main.rs | 1 + src/server/http_server.rs | 44 ++++---- src/server/mod.rs | 18 +++- src/server/state.rs | 20 ++-- 9 files changed, 252 insertions(+), 56 deletions(-) diff --git a/pizauth.conf.5 b/pizauth.conf.5 index be98f29..9df9dd6 100644 --- a/pizauth.conf.5 +++ b/pizauth.conf.5 @@ -36,16 +36,24 @@ is set to the error message. Defaults to logging via .Xr syslog 3 if not specified. -.It Sy http_listen = Qo Em bind-name Qc ; +.It Sy http_listen = Em none | Qo Em bind-name Qc ; specifies the address for the .Xr pizauth 1 HTTP server to listen on. +If +.Em none +is specified, the HTTP server is turned off entirely. +Note that at least one of the HTTP and HTTPS servers must be turned on. Defaults to .Qq 127.0.0.1:0 . -.It Sy https_listen = Qo Em bind-name Qc ; +.It Sy https_listen = Em none | Qo Em bind-name Qc ; specifies the address for the .Xr pizauth 1 HTTPS server to listen on. +If +.Em none +is specified, the HTTPS server is turned off entirely. +Note that at least one of the HTTP and HTTPS servers must be turned on. Defaults to .Qq 127.0.0.1:0 . .It Sy transient_error_if_cmd = Qo Em shell-cmd Qc ; diff --git a/src/config.l b/src/config.l index 5a9460a..35bf210 100644 --- a/src/config.l +++ b/src/config.l @@ -22,7 +22,7 @@ error_notify_cmd "ERROR_NOTIFY_CMD" http_listen "HTTP_LISTEN" https_listen "HTTPS_LISTEN" login_hint "LOGIN_HINT" -transient_error_if_cmd "TRANSIENT_ERROR_IF_CMD" +none "NONE" refresh_retry "REFRESH_RETRY" redirect_uri "REDIRECT_URI" refresh_before_expiry "REFRESH_BEFORE_EXPIRY" @@ -30,6 +30,7 @@ refresh_at_least "REFRESH_AT_LEAST" scopes "SCOPES" token_event_cmd "TOKEN_EVENT_CMD" token_uri "TOKEN_URI" +transient_error_if_cmd "TRANSIENT_ERROR_IF_CMD" //.*?$ ; [ \t\n\r]+ ; . "UNMATCHED" diff --git a/src/config.rs b/src/config.rs index 5c36d16..ac01eb8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -35,8 +35,8 @@ pub struct Config { pub auth_notify_cmd: Option, pub auth_notify_interval: Duration, pub error_notify_cmd: Option, - pub http_listen: String, - pub https_listen: String, + pub http_listen: Option, + pub https_listen: Option, pub transient_error_if_cmd: Option, refresh_at_least: Option, refresh_before_expiry: Option, @@ -127,20 +127,28 @@ impl Config { )?) } config_ast::TopLevel::HttpListen(span) => { - http_listen = Some(check_not_assigned_str( + http_listen = Some(Some(check_not_assigned_str( &lexer, "http_listen", span, http_listen, - )?) + )?)) + } + config_ast::TopLevel::HttpListenNone(span) => { + check_not_assigned(&lexer, "http_listen", span, http_listen)?; + http_listen = Some(None) } config_ast::TopLevel::HttpsListen(span) => { - https_listen = Some(check_not_assigned_str( + https_listen = Some(Some(check_not_assigned_str( &lexer, "https_listen", span, https_listen, - )?) + )?)) + } + config_ast::TopLevel::HttpsListenNone(span) => { + check_not_assigned(&lexer, "https_listen", span, https_listen)?; + https_listen = Some(None) } config_ast::TopLevel::TransientErrorIfCmd(span) => { transient_error_if_cmd = Some(check_not_assigned_str( @@ -189,18 +197,40 @@ impl Config { _ => unreachable!(), } + if let (&Some(None), &Some(None)) = (&http_listen, &https_listen) { + return Err("Cannot set both http_listen and https_listen to 'none'".into()); + } + if accounts.is_empty() { return Err("Must specify at least one account".into()); } + for (act_name, act) in &accounts { + if act.redirect_uri.starts_with("https") { + match https_listen { + Some(Some(_)) | None => (), + Some(None) => { + return Err(format!("Account {act_name} has an 'https' redirect but the HTTPS server is set to 'none'")); + } + } + } else if act.redirect_uri.starts_with("http") { + match http_listen { + Some(Some(_)) | None => (), + Some(None) => { + return Err(format!("Account {act_name} has an 'http' redirect but the HTTP server is set to 'none'")); + } + } + } + } + Ok(Config { accounts, auth_notify_cmd, auth_notify_interval: auth_notify_interval .unwrap_or_else(|| Duration::from_secs(AUTH_NOTIFY_INTERVAL_DEFAULT)), error_notify_cmd, - http_listen: http_listen.unwrap_or_else(|| HTTP_LISTEN_DEFAULT.to_owned()), - https_listen: https_listen.unwrap_or_else(|| HTTPS_LISTEN_DEFAULT.to_owned()), + http_listen: http_listen.unwrap_or_else(|| Some(HTTP_LISTEN_DEFAULT.to_owned())), + https_listen: https_listen.unwrap_or_else(|| Some(HTTPS_LISTEN_DEFAULT.to_owned())), transient_error_if_cmd, refresh_at_least, refresh_before_expiry, @@ -210,6 +240,22 @@ impl Config { } } +fn check_not_assigned( + lexer: &LRNonStreamingLexer>, + name: &str, + span: Span, + v: Option, +) -> Result<(), String> { + match v { + None => Ok(()), + Some(_) => Err(error_at_span( + lexer, + span, + &format!("Mustn't specify '{name:}' more than once"), + )), + } +} + fn check_not_assigned_str( lexer: &LRNonStreamingLexer>, name: &str, @@ -252,7 +298,13 @@ fn check_not_assigned_uri( None => { let s = unescape_str(lexer.span_str(span)); match Url::parse(&s) { - Ok(_) => Ok(s), + Ok(x) => { + if x.scheme() == "http" || x.scheme() == "https" { + Ok(s) + } else { + Err(error_at_span(lexer, span, "not a valid HTTP or HTTPS URI")) + } + } Err(e) => Err(error_at_span(lexer, span, &format!("Invalid URI: {e:}"))), } } @@ -369,12 +421,8 @@ impl Account { )?) } config_ast::AccountField::RedirectUri(span) => { - redirect_uri = Some(check_not_assigned_uri( - lexer, - "redirect_uri", - span, - redirect_uri, - )?) + let uri = check_not_assigned_uri(lexer, "redirect_uri", span, redirect_uri)?; + redirect_uri = Some(uri) } config_ast::AccountField::RefreshAtLeast(span) => { refresh_at_least = Some(time_str_to_duration(check_not_assigned_time( @@ -496,13 +544,18 @@ impl Account { && self.token_uri == act_dump.token_uri } - pub fn redirect_uri(&self, http_port: u16, https_port: u16) -> Result> { + pub fn redirect_uri( + &self, + http_port: Option, + https_port: Option, + ) -> Result> { + assert!(http_port.is_some() || https_port.is_some()); let mut url = Url::parse(&self.redirect_uri)?; - if self.redirect_uri.to_lowercase().starts_with("https") { - url.set_port(Some(https_port)) + if https_port.is_some() && self.redirect_uri.to_lowercase().starts_with("https") { + url.set_port(https_port) .map_err(|_| "Cannot set https port")?; } else { - url.set_port(Some(http_port)) + url.set_port(http_port) .map_err(|_| "Cannot set http port")?; } Ok(url) @@ -691,7 +744,7 @@ mod test { assert_eq!(c.error_notify_cmd, Some("j".to_owned())); assert_eq!(c.auth_notify_cmd, Some("g".to_owned())); assert_eq!(c.auth_notify_interval, Duration::from_secs(88 * 60)); - assert_eq!(c.http_listen, "127.0.0.1:56789".to_owned()); + assert_eq!(c.http_listen, Some("127.0.0.1:56789".to_owned())); assert_eq!(c.transient_error_if_cmd, Some("k".to_owned())); assert_eq!(c.token_event_cmd, Some("q".to_owned())); @@ -757,6 +810,18 @@ mod test { Err(s) if s.contains("Mustn't specify 'http_listen' more than once") => (), _ => panic!(), } + match Config::from_str(r#"http_listen = none; http_listen = "a";"#) { + Err(s) if s.contains("Mustn't specify 'http_listen' more than once") => (), + _ => panic!(), + } + match Config::from_str(r#"https_listen = "a"; https_listen = "b";"#) { + Err(s) if s.contains("Mustn't specify 'https_listen' more than once") => (), + _ => panic!(), + } + match Config::from_str(r#"https_listen = none; https_listen = "a";"#) { + Err(s) if s.contains("Mustn't specify 'https_listen' more than once") => (), + _ => panic!(), + } fn account_dup(field: &str, values: &[&str]) { let c = format!( @@ -789,6 +854,103 @@ mod test { account_dup("token_uri", &[r#""http://a.com/""#, r#""http://b.com/""#]); } + #[test] + fn one_of_http_or_https() { + match Config::from_str( + r#" + http_listen = none; + https_listen = none; + account "x" { + auth_uri = "http://a.com"; + client_id = "b"; + token_uri = "http://f.com"; + } + "#, + ) { + Err(e) if e.contains("Cannot set both http_listen and https_listen to 'none'") => (), + Err(e) => panic!("{e:?}"), + _ => panic!(), + } + } + + #[test] + fn http_or_https_redirect_uris_only() { + match Config::from_str( + r#" + account "x" { + auth_uri = "http://a.com"; + client_id = "b"; + redirect_uri = "httpx://"; + token_uri = "http://f.com"; + } + "#, + ) { + Err(e) if e.contains("not a valid HTTP or HTTPS URI") => (), + Err(e) => panic!("{e:?}"), + _ => panic!(), + } + + match Config::from_str( + r#" + account "x" { + auth_uri = "http://a.com"; + client_id = "b"; + redirect_uri = "ftp://blah/"; + token_uri = "http://f.com"; + } + "#, + ) { + Err(e) if e.contains("not a valid HTTP or HTTPS URI") => (), + Err(e) => panic!("{e:?}"), + _ => panic!(), + } + } + + #[test] + fn correct_listen_for_account() { + match Config::from_str( + r#" + http_listen = none; + account "x" { + auth_uri = "http://a.com"; + client_id = "b"; + token_uri = "http://f.com"; + } + "#, + ) { + Err(e) + if e.contains( + "Account x has an 'http' redirect but the HTTP server is set to 'none'", + ) => + { + () + } + Err(e) => panic!("{e:?}"), + _ => panic!(), + } + match Config::from_str( + r#" + https_listen = none; + account "x" { + auth_uri = "http://a.com"; + client_id = "b"; + redirect_uri = "https://c.com"; + token_uri = "http://f.com"; + } + "#, + ) { + Err(e) + if e.contains( + "Account x has an 'https' redirect but the HTTPS server is set to 'none'", + ) => + { + () + } + Err(e) => panic!("{e:?}"), + _ => panic!(), + } + } + #[test] fn invalid_uris() { fn invalid_uri(field: &str) { @@ -823,10 +985,10 @@ mod test { "#, ) .unwrap(); - assert_eq!(c.https_listen, "127.0.0.1:56789".to_owned()); + assert_eq!(c.https_listen, Some("127.0.0.1:56789".to_owned())); let act = &c.accounts["x"]; assert_eq!(act.redirect_uri, "https://e.com"); - let uri = act.redirect_uri(0, 56789).unwrap(); + let uri = act.redirect_uri(Some(0), Some(56789)).unwrap(); assert_eq!(uri.scheme(), "https"); assert_eq!(uri.port(), Some(56789)); assert_eq!(uri.host_str(), Some("e.com")); diff --git a/src/config.y b/src/config.y index ab44488..f09238a 100644 --- a/src/config.y +++ b/src/config.y @@ -16,7 +16,9 @@ TopLevel -> Result: | "AUTH_NOTIFY_CMD" "=" "STRING" ";" { Ok(TopLevel::AuthNotifyCmd(map_err($3)?)) } | "AUTH_NOTIFY_INTERVAL" "=" "TIME" ";" { Ok(TopLevel::AuthNotifyInterval(map_err($3)?)) } | "ERROR_NOTIFY_CMD" "=" "STRING" ";" { Ok(TopLevel::ErrorNotifyCmd(map_err($3)?)) } + | "HTTP_LISTEN" "=" "NONE" ";" { Ok(TopLevel::HttpListenNone(map_err($3)?)) } | "HTTP_LISTEN" "=" "STRING" ";" { Ok(TopLevel::HttpListen(map_err($3)?)) } + | "HTTPS_LISTEN" "=" "NONE" ";" { Ok(TopLevel::HttpsListenNone(map_err($3)?)) } | "HTTPS_LISTEN" "=" "STRING" ";" { Ok(TopLevel::HttpsListen(map_err($3)?)) } | "TRANSIENT_ERROR_IF_CMD" "=" "STRING" ";" { Ok(TopLevel::TransientErrorIfCmd(map_err($3)?)) } | "REFRESH_AT_LEAST" "=" "TIME" ";" { Ok(TopLevel::RefreshAtLeast(map_err($3)?)) } diff --git a/src/config_ast.rs b/src/config_ast.rs index 822b6a9..ce427eb 100644 --- a/src/config_ast.rs +++ b/src/config_ast.rs @@ -7,7 +7,9 @@ pub enum TopLevel { AuthNotifyInterval(Span), ErrorNotifyCmd(Span), HttpListen(Span), + HttpListenNone(Span), HttpsListen(Span), + HttpsListenNone(Span), TransientErrorIfCmd(Span), RefreshAtLeast(Span), RefreshBeforeExpiry(Span), diff --git a/src/main.rs b/src/main.rs index 35295a1..a49270d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,5 @@ #![allow(clippy::derive_partial_eq_without_eq)] +#![allow(clippy::type_complexity)] mod compat; mod config; diff --git a/src/server/http_server.rs b/src/server/http_server.rs index e41b849..93dfaba 100644 --- a/src/server/http_server.rs +++ b/src/server/http_server.rs @@ -360,10 +360,15 @@ fn http_400(mut stream: T) { stream.write_all(b"HTTP/1.1 400\r\n\r\n").ok(); } -pub fn http_server_setup(conf: &Config) -> Result<(u16, TcpListener), Box> { +pub fn http_server_setup(conf: &Config) -> Result, Box> { // Bind TCP port for HTTP - let listener = TcpListener::bind(&conf.http_listen)?; - Ok((listener.local_addr()?.port(), listener)) + match &conf.http_listen { + Some(http_listen) => { + let listener = TcpListener::bind(http_listen)?; + Ok(Some((listener.local_addr()?.port(), listener))) + } + None => Ok(None), + } } pub fn http_server( @@ -385,22 +390,27 @@ pub fn http_server( pub fn https_server_setup( conf: &Config, -) -> Result<(u16, TcpListener, CertifiedKey), Box> { - // Set a process wide default crypto provider. - let _ = rustls::crypto::ring::default_provider().install_default(); - - // Generate self-signed certificate - let mut names = vec![String::from("localhost"), String::from("127.0.0.1")]; - if let Ok(x) = hostname::get() { - if let Some(x) = x.to_str() { - names.push(String::from(x)); +) -> Result, Box> { + match &conf.https_listen { + Some(https_listen) => { + // Set a process wide default crypto provider. + let _ = rustls::crypto::ring::default_provider().install_default(); + + // Generate self-signed certificate + let mut names = vec![String::from("localhost"), String::from("127.0.0.1")]; + if let Ok(x) = hostname::get() { + if let Some(x) = x.to_str() { + names.push(String::from(x)); + } + } + let cert = generate_simple_self_signed(names)?; + + // Bind TCP port for HTTPS + let listener = TcpListener::bind(https_listen)?; + Ok(Some((listener.local_addr()?.port(), listener, cert))) } + None => Ok(None), } - let cert = generate_simple_self_signed(names)?; - - // Bind TCP port for HTTPS - let listener = TcpListener::bind(&conf.https_listen)?; - Ok((listener.local_addr()?.port(), listener, cert)) } pub fn https_server( diff --git a/src/server/mod.rs b/src/server/mod.rs index c0669d4..5e836a0 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -307,8 +307,14 @@ pub fn server(conf_path: PathBuf, conf: Config, cache_path: &Path) -> Result<(), #[cfg(target_os = "openbsd")] pledge("stdio rpath wpath inet fattr unix dns proc exec", None).unwrap(); - let (http_port, http_state) = http_server::http_server_setup(&conf)?; - let (https_port, https_state, certificate) = http_server::https_server_setup(&conf)?; + let (http_port, http_state) = match http_server::http_server_setup(&conf)? { + Some((x, y)) => (Some(x), Some(y)), + None => (None, None), + }; + let (https_port, https_state, certificate) = match http_server::https_server_setup(&conf)? { + Some((x, y, z)) => (Some(x), Some(y), Some(z)), + None => (None, None, None), + }; // TODO: Store certificate into trusted folder (OS dependent..)? let eventer = Arc::new(Eventer::new()?); @@ -325,8 +331,12 @@ pub fn server(conf_path: PathBuf, conf: Config, cache_path: &Path) -> Result<(), Arc::clone(&refresher), )); - http_server::http_server(Arc::clone(&pstate), http_state)?; - http_server::https_server(Arc::clone(&pstate), https_state, certificate)?; + if let Some(x) = http_state { + http_server::http_server(Arc::clone(&pstate), x)?; + } + if let (Some(x), Some(y)) = (https_state, certificate) { + http_server::https_server(Arc::clone(&pstate), x, y)?; + } eventer.eventer(Arc::clone(&pstate))?; refresher.refresher(Arc::clone(&pstate))?; notifier.notifier(Arc::clone(&pstate))?; diff --git a/src/server/state.rs b/src/server/state.rs index 7b58918..a25d18e 100644 --- a/src/server/state.rs +++ b/src/server/state.rs @@ -46,9 +46,9 @@ pub struct AuthenticatorState { /// [AuthenticatorState::ct_lock]. locked_state: Mutex, /// port of the HTTP server required by OAuth. - pub http_port: u16, + pub http_port: Option, /// port of the HTTPS server required by OAuth. - pub https_port: u16, + pub https_port: Option, pub eventer: Arc, pub notifier: Arc, pub refresher: Arc, @@ -58,8 +58,8 @@ impl AuthenticatorState { pub fn new( conf_path: PathBuf, conf: Config, - http_port: u16, - https_port: u16, + http_port: Option, + https_port: Option, eventer: Arc, notifier: Arc, refresher: Arc, @@ -592,8 +592,8 @@ mod test { let pstate = AuthenticatorState::new( PathBuf::new(), conf, - 0, - 0, + Some(0), + Some(0), eventer, notifier, Refresher::new(), @@ -725,8 +725,8 @@ mod test { let pstate = AuthenticatorState::new( PathBuf::new(), conf, - 0, - 0, + Some(0), + Some(0), eventer, notifier, Refresher::new(), @@ -806,8 +806,8 @@ mod test { let pstate = AuthenticatorState::new( PathBuf::new(), conf, - 0, - 0, + Some(0), + Some(0), eventer, notifier, Refresher::new(), From 516e69f3459ccc0bfdf25138cf867f2521cd6d6b Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Sat, 19 Oct 2024 08:55:19 +0100 Subject: [PATCH 4/4] Also include the IPv6 localhost synonym. --- src/server/http_server.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/server/http_server.rs b/src/server/http_server.rs index 93dfaba..948f786 100644 --- a/src/server/http_server.rs +++ b/src/server/http_server.rs @@ -397,7 +397,11 @@ pub fn https_server_setup( let _ = rustls::crypto::ring::default_provider().install_default(); // Generate self-signed certificate - let mut names = vec![String::from("localhost"), String::from("127.0.0.1")]; + let mut names = vec![ + String::from("localhost"), + String::from("127.0.0.1"), + String::from("::1"), + ]; if let Ok(x) = hostname::get() { if let Some(x) = x.to_str() { names.push(String::from(x));