[pve-devel] [PATCH proxmox-acme 0/1] Close the acme standalone connection after sending a response

[pve-devel] [PATCH proxmox-acme 0/1] Close the acme standalone connection after sending a response

[Daniel Berteaud]

Close connection of the standalone ACME listener after sending a response
so next requests can be handled

Fixes #3048

Daniel Berteaud (1):
  Close the acme standalone connection after sending a response

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

-- 
2.26.2

[pve-devel] [PATCH proxmox-acme 1/1] Close the acme standalone connection after sending a response

[Daniel Berteaud]

Without this, the first req get a response, but not the next ones as the listeners stays busy
Fixes #3048

Signed-off-by: Daniel Berteaud <daniel@firewall-services.com>
---
 src/PVE/ACME/StandAlone.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/ACME/StandAlone.pm b/src/PVE/ACME/StandAlone.pm
index 0e2ece6..552c35c 100644
--- a/src/PVE/ACME/StandAlone.pm
+++ b/src/PVE/ACME/StandAlone.pm
@@ -55,8 +55,8 @@ sub setup {
 		} else {
 		    $c->send_error(404, 'Not found.')
 		}
+		$c->close();
 	    }
-	    $c->close();
 	    $c = undef;
 	}
     }
-- 
2.26.2

Re: [pve-devel] [PATCH proxmox-acme 1/1] Close the acme standalone connection after sending a response

[Fabian Grünbichler]

On September 30, 2020 4:09 pm, Daniel Berteaud wrote:
> Without this, the first req get a response, but not the next ones as the listeners stays busy
> Fixes #3048
> 
> Signed-off-by: Daniel Berteaud <daniel@firewall-services.com>
> ---
>  src/PVE/ACME/StandAlone.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/ACME/StandAlone.pm b/src/PVE/ACME/StandAlone.pm
> index 0e2ece6..552c35c 100644
> --- a/src/PVE/ACME/StandAlone.pm
> +++ b/src/PVE/ACME/StandAlone.pm
> @@ -55,8 +55,8 @@ sub setup {
>  		} else {
>  		    $c->send_error(404, 'Not found.')
>  		}
> +		$c->close();

I think this is not right - we only end up looping/blocking on 
get_request if the client requested keep alive, in which case the server 
should obviously not close the connection..

I guess we have to fork (up to some limit) on accept()? it's obviously 
not ideal that anybody can race with the LE validation attempts and 
block the single request handler ;)

maybe you can change something in your apache config to close the 
connection (or rather, to propagate the connection closing from the 
actual client)? it looks like this can only affect you if
- your apache proxy keeps the connection open
- your apache proxy does not re-use the open connection

the multi-perspective validation by LE is not that new, and it works 
when LE talks directly to the standalone plugin...

>  	    }
> -	    $c->close();
>  	    $c = undef;
>  	}
>      }
> -- 
> 2.26.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH proxmox-acme 1/1] Close the acme standalone connection after sending a response

[Daniel Berteaud]

----- Le 1 Oct 20, à 11:15, Fabian Grünbichler f.gruenbichler@proxmox.com a écrit :

> On September 30, 2020 4:09 pm, Daniel Berteaud wrote:
>> Without this, the first req get a response, but not the next ones as the
>> listeners stays busy
>> Fixes #3048
>> 
>> Signed-off-by: Daniel Berteaud <daniel@firewall-services.com>
>> ---
>>  src/PVE/ACME/StandAlone.pm | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/ACME/StandAlone.pm b/src/PVE/ACME/StandAlone.pm
>> index 0e2ece6..552c35c 100644
>> --- a/src/PVE/ACME/StandAlone.pm
>> +++ b/src/PVE/ACME/StandAlone.pm
>> @@ -55,8 +55,8 @@ sub setup {
>>  		} else {
>>  		    $c->send_error(404, 'Not found.')
>>  		}
>> +		$c->close();
> 
> I think this is not right - we only end up looping/blocking on
> get_request if the client requested keep alive, in which case the server
> should obviously not close the connection..
> 
> I guess we have to fork (up to some limit) on accept()? it's obviously
> not ideal that anybody can race with the LE validation attempts and
> block the single request handler ;)

Indeed, having a few more handlers could limit the risk of this happening.

> 
> maybe you can change something in your apache config to close the
> connection (or rather, to propagate the connection closing from the
> actual client)? it looks like this can only affect you if
> - your apache proxy keeps the connection open
> - your apache proxy does not re-use the open connection

You're right, the issue was on my rev proxy, which didn't re-used keep-alived connexions as it should (it was an old httpd 2.2.3 on a CentOS 5 box, on which I had no control).
Switching my setup so it now runs behind a nginx proxypass works normaly without any modification

Sorry for not having looked at this more closely before posting ;-)

Cheers,
Daniel



-- 
[ https://www.firewall-services.com/ ] 	
Daniel Berteaud 
FIREWALL-SERVICES SAS, La sécurité des réseaux 
Société de Services en Logiciels Libres 
Tél : +33.5 56 64 15 32 
Matrix: @dani:fws.fr 
[ https://www.firewall-services.com/ | https://www.firewall-services.com ]

Re: [pve-devel] [PATCH proxmox-acme 1/1] Close the acme standalone connection after sending a response

[Fabian Grünbichler]

On October 1, 2020 12:55 pm, Daniel Berteaud wrote:
> ----- Le 1 Oct 20, à 11:15, Fabian Grünbichler f.gruenbichler@proxmox.com a écrit :
> 
>> On September 30, 2020 4:09 pm, Daniel Berteaud wrote:
>>> Without this, the first req get a response, but not the next ones as the
>>> listeners stays busy
>>> Fixes #3048
>>> 
>>> Signed-off-by: Daniel Berteaud <daniel@firewall-services.com>
>>> ---
>>>  src/PVE/ACME/StandAlone.pm | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/src/PVE/ACME/StandAlone.pm b/src/PVE/ACME/StandAlone.pm
>>> index 0e2ece6..552c35c 100644
>>> --- a/src/PVE/ACME/StandAlone.pm
>>> +++ b/src/PVE/ACME/StandAlone.pm
>>> @@ -55,8 +55,8 @@ sub setup {
>>>  		} else {
>>>  		    $c->send_error(404, 'Not found.')
>>>  		}
>>> +		$c->close();
>> 
>> I think this is not right - we only end up looping/blocking on
>> get_request if the client requested keep alive, in which case the server
>> should obviously not close the connection..
>> 
>> I guess we have to fork (up to some limit) on accept()? it's obviously
>> not ideal that anybody can race with the LE validation attempts and
>> block the single request handler ;)
> 
> Indeed, having a few more handlers could limit the risk of this happening.
> 
>> 
>> maybe you can change something in your apache config to close the
>> connection (or rather, to propagate the connection closing from the
>> actual client)? it looks like this can only affect you if
>> - your apache proxy keeps the connection open
>> - your apache proxy does not re-use the open connection
> 
> You're right, the issue was on my rev proxy, which didn't re-used keep-alived connexions as it should (it was an old httpd 2.2.3 on a CentOS 5 box, on which I had no control).
> Switching my setup so it now runs behind a nginx proxypass works normaly without any modification
> 
> Sorry for not having looked at this more closely before posting ;-)

no worries. I retitled the bug you filed to track the actual issue - 
feel free to write a patch for it anyway ;)