Re: [pbs-devel] [PATCH v3 proxmox-backup 3/3] proxy: redirect HTTP requests to HTTPS

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Tue, Oct 31, 2023 at 07:47:05PM +0100, Max Carrara wrote:
> Signed-off-by: Max Carrara <m.carrara@proxmox.com>
> ---
>  Changes v1 --> v2:
>   * Incorporate changes of the previous two patches correspondingly
> 
>  Changes v2 --> v3:
>   * None
> 
>  src/bin/proxmox-backup-proxy.rs | 46 ++++++++++++++++++++++++++++-----
>  1 file changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index f38a02bd..f69f5bfc 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -23,8 +23,8 @@ use proxmox_sys::{task_log, task_warn};
>  use pbs_datastore::DataStore;
>  
>  use proxmox_rest_server::{
> -    cleanup_old_tasks, cookie_from_header, rotate_task_log_archive, ApiConfig, RestEnvironment,
> -    RestServer, WorkerTask,
> +    cleanup_old_tasks, cookie_from_header, rotate_task_log_archive, ApiConfig, Redirector,
> +    RestEnvironment, RestServer, WorkerTask,
>  };
>  
>  use proxmox_backup::rrd_cache::{
> @@ -253,6 +253,7 @@ async fn run() -> Result<(), Error> {
>          )?;
>  
>      let rest_server = RestServer::new(config);
> +    let redirector = Redirector::new();
>      proxmox_rest_server::init_worker_tasks(
>          pbs_buildcfg::PROXMOX_BACKUP_LOG_DIR_M!().into(),
>          file_opts.clone(),
> @@ -288,23 +289,54 @@ async fn run() -> Result<(), Error> {
>          Ok(Value::Null)
>      })?;
>  
> -    let connections = proxmox_rest_server::connection::AcceptBuilder::with_acceptor(acceptor)
> +    let connections = proxmox_rest_server::connection::AcceptBuilder::new()
>          .debug(debug)
>          .rate_limiter_lookup(Arc::new(lookup_rate_limiter))
>          .tcp_keepalive_time(PROXMOX_BACKUP_TCP_KEEPALIVE_TIME);
> +
>      let server = daemon::create_daemon(
>          ([0, 0, 0, 0, 0, 0, 0, 0], 8007).into(),
>          move |listener| {
> -            let connections = connections.accept(listener);
> +            let (secure_connections, insecure_connections) =
> +                connections.accept_tls_optional(listener, acceptor);
>  
>              Ok(async {
>                  daemon::systemd_notify(daemon::SystemdNotify::Ready)?;
>  
> -                hyper::Server::builder(connections)
> +                let secure_server = hyper::Server::builder(secure_connections)
>                      .serve(rest_server)
>                      .with_graceful_shutdown(proxmox_rest_server::shutdown_future())
> -                    .map_err(Error::from)
> -                    .await
> +                    .map_err(Error::from);
> +
> +                let insecure_server = hyper::Server::builder(insecure_connections)
> +                    .serve(redirector)
> +                    .with_graceful_shutdown(proxmox_rest_server::shutdown_future())
> +                    .map_err(Error::from);
> +
> +                let handles = vec![tokio::spawn(secure_server), tokio::spawn(insecure_server)];

Maybe we should just detach the redirection-handler and potentially give
it a retry logic and finally fail it with a log message.

Otherwise, this shouldn't need to be a Vec, a regular array should work,
skips the extra allocation.

> +
> +                let mut results: Vec<Result<(), Error>> = vec![];
> +
> +                for res_handle in futures::future::join_all(handles).await.into_iter() {
> +                    let flattened_res = match res_handle {
> +                        Ok(inner) => inner,
> +                        Err(err) => Err(format_err!(err)),
> +                    };
> +
> +                    results.push(flattened_res);
> +                }
> +
> +                if results.iter().any(Result::is_err) {
> +                    let cat_errors = results
> +                        .into_iter()
> +                        .filter_map(|res| res.err().map(|err| err.to_string()))
> +                        .collect::<Vec<_>>()
> +                        .join("\n");
> +
> +                    return Err(format_err!(cat_errors));
> +                }
> +
> +                Ok(())
>              })
>          },
>          Some(pbs_buildcfg::PROXMOX_BACKUP_PROXY_PID_FN),
> -- 
> 2.39.2