[pve-devel] [PATCH access-control] fix #4074: increase API OpenID code size limit to 2048

[pve-devel] [PATCH access-control] fix #4074: increase API OpenID code size limit to 2048

[Mira Limbeck]

Azure AD seems to have a variable authorization code size, depending on
the browser state according to one report in bug #4074 [0].

Sometimes the size is greater than our current limit of 1024, so
increase it to 2048.

The RFC [1] mentions that there is no limit to the code size, but based on
current experience, a size limit of 2048 might be enough for every
current OpenID Connect provider.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=4074
[1] https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2

Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
---
The PBS implementation doesn't seem to be using any code size limit, do
we want to remove it entirely in PVE as well?

 src/PVE/API2/OpenId.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
index c838b2c..8653d61 100644
--- a/src/PVE/API2/OpenId.pm
+++ b/src/PVE/API2/OpenId.pm
@@ -134,7 +134,7 @@ __PACKAGE__->register_method ({
 	    code => {
 		description => "OpenId authorization code.",
 		type => 'string',
-		maxLength => 1024,
+		maxLength => 2048,
             },
 	    'redirect-url' => {
 		description => "Redirection Url. The client should set this to the used server url (location.origin).",
-- 
2.30.2

Re: [pve-devel] [PATCH access-control] fix #4074: increase API OpenID code size limit to 2048

[Wolfgang Bumiller]

On Wed, Jun 15, 2022 at 04:09:50PM +0200, Mira Limbeck wrote:
> Azure AD seems to have a variable authorization code size, depending on
> the browser state according to one report in bug #4074 [0].
> 
> Sometimes the size is greater than our current limit of 1024, so
> increase it to 2048.

Would there be any downside to just going straight to 4 or 8K here?

> 
> The RFC [1] mentions that there is no limit to the code size, but based on
> current experience, a size limit of 2048 might be enough for every
> current OpenID Connect provider.
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=4074
> [1] https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2
> 
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
> The PBS implementation doesn't seem to be using any code size limit, do
> we want to remove it entirely in PVE as well?
> 
>  src/PVE/API2/OpenId.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
> index c838b2c..8653d61 100644
> --- a/src/PVE/API2/OpenId.pm
> +++ b/src/PVE/API2/OpenId.pm
> @@ -134,7 +134,7 @@ __PACKAGE__->register_method ({
>  	    code => {
>  		description => "OpenId authorization code.",
>  		type => 'string',
> -		maxLength => 1024,
> +		maxLength => 2048,
>              },
>  	    'redirect-url' => {
>  		description => "Redirection Url. The client should set this to the used server url (location.origin).",
> -- 
> 2.30.2

Re: [pve-devel] [PATCH access-control] fix #4074: increase API OpenID code size limit to 2048

[Mira Limbeck]

On 6/17/22 10:42, Wolfgang Bumiller wrote:
> On Wed, Jun 15, 2022 at 04:09:50PM +0200, Mira Limbeck wrote:
>> Azure AD seems to have a variable authorization code size, depending on
>> the browser state according to one report in bug #4074 [0].
>>
>> Sometimes the size is greater than our current limit of 1024, so
>> increase it to 2048.
> Would there be any downside to just going straight to 4 or 8K here?
>
I don't think so. I just went with the usual `increase it to handle the 
current requirements` approach here.

If you prefer a higher limit then sure, I'll send a v2 with 4k or 8k.

The mentions in the bug tracker/forum post were of code sizes about 
1100-1300 in length.

[pve-devel] applied: [PATCH access-control] fix #4074: increase API OpenID code size limit to 2048

[Wolfgang Bumiller]

applied, but bumped to 4k, while I think that's *massive*, it'll be even
less likely to be hit, I hope :-)

On Wed, Jun 15, 2022 at 04:09:50PM +0200, Mira Limbeck wrote:
> Azure AD seems to have a variable authorization code size, depending on
> the browser state according to one report in bug #4074 [0].
> 
> Sometimes the size is greater than our current limit of 1024, so
> increase it to 2048.
> 
> The RFC [1] mentions that there is no limit to the code size, but based on
> current experience, a size limit of 2048 might be enough for every
> current OpenID Connect provider.
> 
> [0] https://bugzilla.proxmox.com/show_bug.cgi?id=4074
> [1] https://datatracker.ietf.org/doc/html/rfc6749#section-4.1.2
> 
> Signed-off-by: Mira Limbeck <m.limbeck@proxmox.com>
> ---
> The PBS implementation doesn't seem to be using any code size limit, do
> we want to remove it entirely in PVE as well?
> 
>  src/PVE/API2/OpenId.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/API2/OpenId.pm b/src/PVE/API2/OpenId.pm
> index c838b2c..8653d61 100644
> --- a/src/PVE/API2/OpenId.pm
> +++ b/src/PVE/API2/OpenId.pm
> @@ -134,7 +134,7 @@ __PACKAGE__->register_method ({
>  	    code => {
>  		description => "OpenId authorization code.",
>  		type => 'string',
> -		maxLength => 1024,
> +		maxLength => 2048,
>              },
>  	    'redirect-url' => {
>  		description => "Redirection Url. The client should set this to the used server url (location.origin).",
> -- 
> 2.30.2