Re: [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Shannon Sterz <s.sterz@proxmox.com> writes:

> # Summary
>
> this series adds an authentication flow based on http only cookies for
> proxmox backup server. at the moment this authentication flow is opt-in
> in order to not break older clients that may still rely on the previous
> authentication flow.
>
> this series is split into three parts:
>
> 1. prepare proxmox-auth-api to be used with extjs and implements a new
>    ticket endpoint for pbs. this endpoint requires clients to provide a
>    boolean parameter `http-only` as `true` for it to switch to the new
>    http-only based authentication flow.
> 2. adapt proxmox backup server's ui components to always use the http
>    only based authentication flow. this should make cookies
>    inaccessible to any javascript-based attack in the browser,
>    providing an extra layer of security.
> 3. prepare pbs-client for potential servers that may no longer provide
>    the previous authentication flow. the point of already adding this
>    now, is to be prepare update-hesitant users for a future without the
>    old authentication flow. if the old authentication flow is dropped in
>    the future, more users will already have a client version that can
>    adapt to the new flow.
>
> # Why not opt the `pbs-client` into the new flow?
>
> the client is deliberatelly not opted into the new authentication flow
> for the following reasons:
>
> - http only cookies are only an effective security mechanism within a
>   browser context. the client simply does not benefit from this extra
>   layer of protection. the attacks http only cookies protect against,
>   simply don't exist here.
> - opting the client in unconditionally isn't possible. older pbs servers
>   would complain about the additional api parameters provided by the
>   client. this means:
>
>     + the client would either need to try the http-only flow and once
>       that fails, fall back to the older authentication flow.
>     + or query (and possibly cache) the server version and check if the
>       version is new enough to support the new authentication flow.
>
> both approaches are more error-prone and produce additional network
> overhead than simply not opting the client into the new flow. causing
> api errors may also produce a lot of false warnings when monitoring pbs.
> so there are no benefits, but potential downsides. hence, the client
> will not yet be opted into the new authentication flow.
>
> proxmox:
>
> Shannon Sterz (1):
>   auth-api: include meta information required by extjs in api endpoints
>
>  proxmox-auth-api/src/api/access.rs | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
>
> proxmox-backup:
>
> Shannon Sterz (3):
>   api: access: add opt-in http only ticket authentication flow
>   ui: opt into the new http-only ticket authentication flow
>   client: adapt pbs client to also handle http-only flows correctly
>
>  pbs-client/src/http_client.rs | 70 ++++++++++++++++++++++++++++---
>  src/api2/access/mod.rs        | 77 +++++++++++++++++++++++++++++++++--
>  www/Application.js            | 12 +++++-
>  www/LoginView.js              |  4 +-
>  www/MainView.js               |  1 +
>  www/Utils.js                  |  6 +++
>  6 files changed, 159 insertions(+), 11 deletions(-)
>
>
> Summary over all repositories:
>   7 files changed, 169 insertions(+), 14 deletions(-)

Tested via modifying the client:

```diff
modified   pbs-client/src/http_client.rs
@@ -506,11 +506,20 @@ impl HttpClient {
             }
         };
 
+        let http_only = std::env::var("PROXMOX_BACKUP_CLIENT_HTTP_ONLY")
+            .ok()
+            .and_then(|s| s.parse::<i32>().ok())
+            .is_some_and(|res| res > 0);
+
+        if http_only {
+            info!("Using http-only authentication");
+        }
+
         let auth = Arc::new(RwLock::new(AuthInfo {
             auth_id: auth_id.clone(),
             ticket: password.clone(),
             token: "".to_string(),
-            http_only: false,
+            http_only,
         }));
 
         let server2 = server.to_string();
```

then `proxmox-backup-client login` works against a pbs running this
patch when then env variable is set.

When login via a web browser both cookies are set and cleared correctly
on logout.

The only thing I am missing is a bit of logging on the server side as
described in my reply, but this is not a blocker.

Consider this.

Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

-- 
Maximiliano


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel