* [pbs-devel] [PATCH proxmox v2] login: use `ticket` if both it and `ticket_info` are provided
@ 2025-10-01 13:11 Shannon Sterz
2025-10-01 15:02 ` Christian Ebner
0 siblings, 1 reply; 4+ messages in thread
From: Shannon Sterz @ 2025-10-01 13:11 UTC (permalink / raw)
To: pbs-devel
previously the precense of `ticket_info` was assumed to indicate the
HTTPOnly authentication flow. the `ticket` field was ignore in that
case, because the client has no way of validating a ticket anyway.
this commit changes the behaviour to assume that the server is not
trying to "trick us" and that the presence of a `ticket` field
indicates that this value should be used for authentication. if the
`ticket_info` field is also present, it will be ignored.
this fixes an issue where authentication against later versions of
proxmox-backup-server 3.4 failed. including versions up to and
including version 3.4.6-1.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-login/src/lib.rs | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index 4482f2e4..fd4ca850 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -198,22 +198,24 @@ impl Login {
));
}
- // `ticket_info` is set when the server sets the ticket via an HttpOnly cookie. this also
- // means we do not have access to the cookie itself which happens for example in a browser.
- // assume that the cookie is handled properly by the context (browser) and don't worry
- // about handling it ourselves.
- if let Some(ref ticket) = response.ticket_info {
- let ticket = ticket.parse()?;
- return Ok(TicketResult::HttpOnly(
- self.authentication_for(ticket, response)?,
- ));
- }
-
// old authentication flow where we needed to handle the ticket ourselves even in the
// browser etc.
let ticket: TicketResponse = match response.ticket {
Some(ref ticket) => ticket.parse()?,
- None => return Err("no ticket information in response".into()),
+ None => {
+ // `ticket_info` is set when the server sets the ticket via a HttpOnly cookie. this
+ // also means we do not have access to the cookie itself which happens for example
+ // in a browser. assume that the cookie is handled properly by the context
+ // (browser) and don't worry about handling it ourselves.
+ if let Some(ref ticket) = response.ticket_info {
+ let ticket = ticket.parse()?;
+ return Ok(TicketResult::HttpOnly(
+ self.authentication_for(ticket, response)?,
+ ));
+ }
+
+ return Err("no ticket information in response".into());
+ }
};
Ok(match ticket {
--
2.47.3
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox v2] login: use `ticket` if both it and `ticket_info` are provided
2025-10-01 13:11 [pbs-devel] [PATCH proxmox v2] login: use `ticket` if both it and `ticket_info` are provided Shannon Sterz
@ 2025-10-01 15:02 ` Christian Ebner
2025-10-02 7:50 ` Shannon Sterz
0 siblings, 1 reply; 4+ messages in thread
From: Christian Ebner @ 2025-10-01 15:02 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Shannon Sterz
On 10/1/25 3:11 PM, Shannon Sterz wrote:
> previously the precense of `ticket_info` was assumed to indicate the
> HTTPOnly authentication flow. the `ticket` field was ignore in that
> case, because the client has no way of validating a ticket anyway.
>
> this commit changes the behaviour to assume that the server is not
> trying to "trick us" and that the presence of a `ticket` field
> indicates that this value should be used for authentication. if the
> `ticket_info` field is also present, it will be ignored.
>
> this fixes an issue where authentication against later versions of
> proxmox-backup-server 3.4 failed. including versions up to and
> including version 3.4.6-1.
>
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
Only one question: Should the `SecondFactorChallenge::response_bytes()`
also prioritize `ticket` over `ticket_info` to get the same flow behavior?
Other than that the changes look good to me, as they do follow the same
intend of parsing the full ticket I did try
to force via the compat mode in
https://lore.proxmox.com/pdm-devel/DD70FPP8GIBO.2K8CWBW5XPL0K@proxmox.com/T/
Tested ticket parsing and request authentication works with this patch
applied by using PDM remote add wizard (which uses the proxmox-login)
for the following version:
- PBS 3.4.0 (fresh install from ISO), 3.4.6, 4.0.11 (fresh install from
ISO), 4.0.15
- PVE 8.4.0 (fresh install from ISO), 8.4.14, 9.0.3 (fresh install from
ISO), 9.0.10
Please consider:
Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
Tested-by: Christian Ebner <c.ebner@proxmox.com>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox v2] login: use `ticket` if both it and `ticket_info` are provided
2025-10-01 15:02 ` Christian Ebner
@ 2025-10-02 7:50 ` Shannon Sterz
2025-10-02 7:57 ` [pbs-devel] Superseded: " Shannon Sterz
0 siblings, 1 reply; 4+ messages in thread
From: Shannon Sterz @ 2025-10-02 7:50 UTC (permalink / raw)
To: Christian Ebner, Proxmox Backup Server development discussion
On Wed Oct 1, 2025 at 5:02 PM CEST, Christian Ebner wrote:
> On 10/1/25 3:11 PM, Shannon Sterz wrote:
>> previously the precense of `ticket_info` was assumed to indicate the
>> HTTPOnly authentication flow. the `ticket` field was ignore in that
>> case, because the client has no way of validating a ticket anyway.
>>
>> this commit changes the behaviour to assume that the server is not
>> trying to "trick us" and that the presence of a `ticket` field
>> indicates that this value should be used for authentication. if the
>> `ticket_info` field is also present, it will be ignored.
>>
>> this fixes an issue where authentication against later versions of
>> proxmox-backup-server 3.4 failed. including versions up to and
>> including version 3.4.6-1.
>>
>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>> ---
>
> Only one question: Should the `SecondFactorChallenge::response_bytes()`
> also prioritize `ticket` over `ticket_info` to get the same flow behavior?
yes sorry that was an oversight on my end, will send a v3 in a minute.
> Other than that the changes look good to me, as they do follow the same
> intend of parsing the full ticket I did try
> to force via the compat mode in
> https://lore.proxmox.com/pdm-devel/DD70FPP8GIBO.2K8CWBW5XPL0K@proxmox.com/T/
>
> Tested ticket parsing and request authentication works with this patch
> applied by using PDM remote add wizard (which uses the proxmox-login)
> for the following version:
> - PBS 3.4.0 (fresh install from ISO), 3.4.6, 4.0.11 (fresh install from
> ISO), 4.0.15
> - PVE 8.4.0 (fresh install from ISO), 8.4.14, 9.0.3 (fresh install from
> ISO), 9.0.10
>
> Please consider:
>
> Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
> Tested-by: Christian Ebner <c.ebner@proxmox.com>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pbs-devel] Superseded: Re: [PATCH proxmox v2] login: use `ticket` if both it and `ticket_info` are provided
2025-10-02 7:50 ` Shannon Sterz
@ 2025-10-02 7:57 ` Shannon Sterz
0 siblings, 0 replies; 4+ messages in thread
From: Shannon Sterz @ 2025-10-02 7:57 UTC (permalink / raw)
To: Shannon Sterz, Christian Ebner,
Proxmox Backup Server development discussion
Superseded-by: https://lore.proxmox.com/all/DD7O7RLF50HZ.1NAU5VCE3AE7A@proxmox.com/T/#t
On Thu Oct 2, 2025 at 9:50 AM CEST, Shannon Sterz wrote:
> On Wed Oct 1, 2025 at 5:02 PM CEST, Christian Ebner wrote:
>> On 10/1/25 3:11 PM, Shannon Sterz wrote:
>>> previously the precense of `ticket_info` was assumed to indicate the
>>> HTTPOnly authentication flow. the `ticket` field was ignore in that
>>> case, because the client has no way of validating a ticket anyway.
>>>
>>> this commit changes the behaviour to assume that the server is not
>>> trying to "trick us" and that the presence of a `ticket` field
>>> indicates that this value should be used for authentication. if the
>>> `ticket_info` field is also present, it will be ignored.
>>>
>>> this fixes an issue where authentication against later versions of
>>> proxmox-backup-server 3.4 failed. including versions up to and
>>> including version 3.4.6-1.
>>>
>>> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
>>> ---
>>
>> Only one question: Should the `SecondFactorChallenge::response_bytes()`
>> also prioritize `ticket` over `ticket_info` to get the same flow behavior?
>
> yes sorry that was an oversight on my end, will send a v3 in a minute.
>
>> Other than that the changes look good to me, as they do follow the same
>> intend of parsing the full ticket I did try
>> to force via the compat mode in
>> https://lore.proxmox.com/pdm-devel/DD70FPP8GIBO.2K8CWBW5XPL0K@proxmox.com/T/
>>
>> Tested ticket parsing and request authentication works with this patch
>> applied by using PDM remote add wizard (which uses the proxmox-login)
>> for the following version:
>> - PBS 3.4.0 (fresh install from ISO), 3.4.6, 4.0.11 (fresh install from
>> ISO), 4.0.15
>> - PVE 8.4.0 (fresh install from ISO), 8.4.14, 9.0.3 (fresh install from
>> ISO), 9.0.10
>>
>> Please consider:
>>
>> Reviewed-by: Christian Ebner <c.ebner@proxmox.com>
>> Tested-by: Christian Ebner <c.ebner@proxmox.com>
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-02 7:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-10-01 13:11 [pbs-devel] [PATCH proxmox v2] login: use `ticket` if both it and `ticket_info` are provided Shannon Sterz
2025-10-01 15:02 ` Christian Ebner
2025-10-02 7:50 ` Shannon Sterz
2025-10-02 7:57 ` [pbs-devel] Superseded: " Shannon Sterz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox