Skip to content

Conversation

@cmb69
Copy link
Member

@cmb69 cmb69 commented Aug 12, 2021

ftp_ssl_shutdown() attempts a bidirectional shutdown handshake, i.e.
we're sending a close_notify alert, and then wait for the peer to send
its close_notify alert. If the peer doesn't sent it, typically because
it already closed the connection, we should not raise a warning, but
rather ignore this condition.


Note that the test case is somewhat bogus as it wouldn't fail without the changes to ftp.c; I wouldn't know how to accomplish that. Still, the test case makes some sense on its own, since it tests uploading via FTPS.

`ftp_ssl_shutdown()` attempts a bidirectional shutdown handshake, i.e.
we're sending a close_notify alert, and then wait for the peer to send
its close_notify alert.  If the peer doesn't sent it, typically because
it already closed the connection, we should not raise a warning, but
rather ignore this condition.
@cmb69 cmb69 added the Bug label Aug 12, 2021
@nikic
Copy link
Member

nikic commented Aug 12, 2021

New test fails on azure.

The latter might not be supported by the OpenSSL library.
@cmb69
Copy link
Member Author

cmb69 commented Aug 12, 2021

Thanks! I've pushed a potential fix.

@cmb69
Copy link
Member Author

cmb69 commented Aug 12, 2021

Nope, that wasn't the issue. This may get funny.

@cmb69 cmb69 marked this pull request as draft August 12, 2021 14:13
@cmb69
Copy link
Member Author

cmb69 commented Aug 31, 2021

Apparently, the test fails only with OpenSSL 1.1, but passes with OpenSSL 1.0. I have no idea why, though. Any hints appreciated!

@cmb69 cmb69 closed this Sep 7, 2021
@markus-angst
Copy link

From the comments above, I cannot conclude what the findings are with regard to the problem. I still get the warning. Apache/2.4.57 (Win64) OpenSSL/1.1.1t PHP/8.3.17. OpenSSL Library Version: OpenSSL 3.0.15 3 Sep 2024. Is this pull request closed, because there is no solution (yet)? Or is there a newer PHP version with a fix? I can't find anything in the PHP version history. Any help would be appreciated. Thanks!

@cmb69
Copy link
Member Author

cmb69 commented Aug 13, 2025

Indeed, this bug has still not been fixed; I had given up on it.

Apache/2.4.57 (Win64) OpenSSL/1.1.1t PHP/8.3.17. OpenSSL Library Version: OpenSSL 3.0.15 3 Sep 2024.

Which OpenSSL version is it, 1.1.1t or 3.0.15?

@markus-angst
Copy link

markus-angst commented Aug 13, 2025

Sorry for being ambiguous.
In phpinfo() the "apache2handler" section reports: Apache Version | Apache/2.4.57 (Win64) OpenSSL/1.1.1t PHP/8.3.17.
And the "openssl" extension reports: OpenSSL Library Version | OpenSSL 3.0.15 3 Sep 2024.
I am ready to find out more, if needed (and possible).

@markus-angst
Copy link

markus-angst commented Aug 13, 2025

In C:\Program Files\Apache Software Foundation\Apache_2.4.57_VS16\bin I have libssl-1_1-x64.dll. (Version 1.1.1t). This file came with Apache.
In C:\Program Files\php_8.3.17_VS16 I have libssl-3-x64.dll. (Version 3.0.15)This file came with PHP. (The PHP FTP extension php_ftp.dll depends on libssl-3-x64.dll.)
C:\Program Files\php_8.3.17_VS16 is in the path; C:\Program Files\Apache Software Foundation\Apache_2.4.57_VS16\bin is not, but that's where httpd.exe is located.

(All my FTP functions work so far, except for this warning.)

@bukka
Copy link
Member

bukka commented Oct 23, 2025

I have been checking and thinking about this one.

I think that waiting for the peer to close the connection and error otherwise is actually the right thing to do in general and this is also what OpenSSL does by default. The reason for that is that it might allow the truncation attack.

However in the FTP context, the truncation attack should be less likely because it awaits the responses and the data that is read is discarded anyway. I guess we could just call SSL_shutdown twice which should actually handle all of this. It would still error so we would need to set SSL_OP_IGNORE_UNEXPECTED_EOF which is something that this should do anyway because it could in some case have some data to read (e.g. session ticket record) but then close the connection. The only potentially issue with second SSL_shutdown would be that if server kept connection open, it wouldn't timed out so maybe this approach is safer.

I think those changes makes sense. As mentioned it should also set SSL_OP_IGNORE_UNEXPECTED_EOF to ctx options when created but that's more edge case.

The thing that needs to improve is the test though. I don't think this is going to close the connection so it seems not like the test for this. I'm just working on custom TLS 1.3 PHP implementation (as part of other work to catch some other cases) that would allow to recreate this so will try to look into this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants