Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POP3, questionable handling of '-ERR...' response on 'STLS' #12063

Closed
icing opened this issue Oct 8, 2023 · 2 comments
Closed

POP3, questionable handling of '-ERR...' response on 'STLS' #12063

icing opened this issue Oct 8, 2023 · 2 comments

Comments

@icing
Copy link
Contributor

icing commented Oct 8, 2023

I did this

CURL_DBG_SOCK_RMAX=4 ./runtests.pl -v 982

I expected the following

The test to succeed.

The difference is that pop3_state_starttls_resp() in pop3.c:789 behaves differently depending on the pp.cache_size. With small recv(), cache_size is 0, where on "normal" test runs, additional response data is in the cache.

Since CURLUSESSL_TRY is active, test982 proceeds with user authentication where in "normal" run, it does not.

curl/libcurl version

curl 8.4.0-DEV

operating system

macOS

@bagder bagder added the POP3 label Oct 8, 2023
@bagder
Copy link
Member

bagder commented Oct 8, 2023

I believe the test case is broken. It looks like it assumes that the full sequence is read and handled by curl in a single read, which of course it should not.

Any idea on how to polish this @monnerat ?

@monnerat
Copy link
Contributor

monnerat commented Oct 8, 2023

Any idea on how to polish this @monnerat ?

Maybe also reject some available data from the stream in libcurl? The tls handshake is always initiated by the client so it should be safe, although useless for normal operations. It may still fail the test however if the data is not yet present (delayed) when the rejection check occurs.

Currently, if pipelined data is not read, it is taken as tls handshake data and then makes the latter failing, which is what we want (although it would have another error code if tls is required).

But there's no support for upgrade to tls in the test server, so I had to use --ssl instead of --ssl-reqd as a workaround.

This test checks pipelining is rejected, but in the target case, curl does not even see it as such and data is processed by the TLS handshake, thus the test is meaningless. The real problem for testing is we can't force curl to read this garbage before the tls handshake.

We surely have the same problem with tests 980, 981 and 983.

The ideal solution would probably to have the buffered data fed into the tls handshake (to "unshuffle" stream data) and the cache check removed: tests for this could then be deleted.

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

No branches or pull requests

3 participants