Fix Python >= 3.7 deprecations, and Improve Server Resilience #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…) for Python 3.7 and later
It is currently a request handler based on BaseHTTPRequestHandler, which does not allow us to pass in the verbosity level because it gets instantiated by the HTTPServer class
- Use plain string instead of f'' strings where not needed - Remove semicolons where not needed
This is to prepare for adding verbosity level to RemoteStashServer request Handler & fixing the file push / POST capability
- Rename class RemoteStashServer(BaseHTTPRequestHandler) - to: RemoteStashServerRequestHandler(BaseHTTPRequestHandler) - Implement stderr print function - Implement sanitize_data() function for logging bytes / string
…ogging This should allow figuring out the issues with file upload / POST
…ng push & log more
…tput exceptions to client
…& get raw file data
This seems to have been the cause of **many** RemoteStash pull failures!
What was happening here:
- Client would call `GET /pull` when server has a binary file in stash
- The client would see HTTP/1.1 200 ...long potentially binary string here...
- Client would fail to parse the HTTP status line, and rightfully hang up
- Server would see EOF in the middle of sending the item content
- Exception would be thrown, and second 500 error response would trigger!
- Client never sees the second 500 error response... but at least it was logged!
Error was:
ssl.SSLEOFError: EOF occurred in violation of protocol (_ssl.c:2426)
Traceback (most recent call last):
File "/home/exampleuser/.local/bin/remotestash", line 919, in do_GET
getattr(self, method)()
File "/home/exampleuser/.local/bin/remotestash", line 879, in last
self.respond_item( item )
File "/home/exampleuser/.local/bin/remotestash", line 1092, in respond_item
self.respond( 200, headers, message, log_message=f'Request: {item}' )
File "/home/exampleuser/.local/bin/remotestash", line 1189, in respond
self.end_headers()
File "/home/exampleuser/.pyenv/versions/3.10.12/lib/python3.10/http/server.py", line 535, in end_headers
self.flush_headers()
File "/home/exampleuser/.pyenv/versions/3.10.12/lib/python3.10/http/server.py", line 539, in flush_headers
self.wfile.write(b"".join(self._headers_buffer))
File "/home/exampleuser/.pyenv/versions/3.10.12/lib/python3.10/socketserver.py", line 826, in write
self._sock.sendall(b)
File "/home/exampleuser/.pyenv/versions/3.10.12/lib/python3.10/ssl.py", line 1237, in sendall
v = self.send(byte_view[count:])
File "/home/exampleuser/.pyenv/versions/3.10.12/lib/python3.10/ssl.py", line 1206, in send
return self._sslobj.write(data)
ssl.SSLEOFError: EOF occurred in violation of protocol (_ssl.c:2426)
- Memory limit for form data processing only
applies as written if the `Content-Type` is
`"application/x-www-form-urlencoded"` or
`"application/x-url-encoded"`.
- The memory limmit is not applied to multipart
form data processing unless the disk limit is
less than the memory limit.
- In other words:
- If Content-Type is `"multipart/form-data"`,
the memory limit is set to the lesser of
memory limit or disk limit.
- Provide references to relevant RFCs and errata.
- Modify `parse_form_data()` function call to
include the `strict_mode` and `mem_limit`
parameters.
- `strict_mode` is set to `True`
- `mem_limit` is set to the value of
`DEFAULT_FORMDATA_CONTENT_LENGTH_MEM_LIMIT`,
which is 1 MiB (1 mebibyte = 1024*1024 bytes).
- `disk_limit` is set to the value of
`DEFAULT_FORMDATA_CONTENT_LENGTH_DISK_LIMIT`,
which is 1 GiB (1 gibibyte = 1024*1024*1024
bytes).
- Ensures the multipart code handles form data
processing correctly, throwing exceptions from
`parse_form_data()` and adheres to the
specified memory limit when dealing with
URL-encoded form data. Also ensures that the
specified disk limit is adhered to for
multipart form uploads.
- Set the `timeout` property of the
`RemoteStashServerRequestHandler` class to 60
seconds.
- If a request takes longer than 60 seconds to
process, a `TimeoutError` will be raised.
- Added exception handling blocks for
`TimeoutError`.
- Respond with status code 408 (Request Timeout)
if a `TimeoutError` is raised.
- Send `REQUEST_METHOD` to `parse_form_data()` to handle POST/GET requests properly. - Send appropriate response code: 405 Method Not Allowed with helpful `Allowed:` error header.
trinitronx
force-pushed
the
fix-all-the-things
branch
from
812a706 to
23514ae
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed that after a recent python upgrade (Arch Linux is now on
Python 3.10.12) that the server component stopped working due to the removal of some deprecated things in the Python language that were scheduled to be removed since Python 3.7. So this PR started out as just trying to fix those things.However, it soon grew larger because after fixing those, I found that it was still throwing some exceptions and stack traces when doing things like uploading large files, image mime types, and generally exercising the API in ways that a security researcher might try to do.
I was inspired after reading your blog post about this Father-Son project, which I had been using for many years as a way to use Airdrop clipboard functionality between iOS and Linux.
So, in the spirit of Linux & Open Source, I decided to try my hand at making the server component much more resilient & secure. For example, I tested trying to use
curlas the client and sending a large growing log file, but not sending theContent-Lengthheader, which caused a possible DoS-style deadlock condition where both server and client would wait for something to be send from the other which would never happen. This led me towards fixing a bunch of issues that I encountered in testing.This PR improves the Python-based server component in the following ways:
multipart/form-datauploads)contents.itemsdict keys before trying to access them (was causingKeyErrorexceptions in certain cases60 secondserver-side timeout to handle a request (Prevents indefinite deadlock DoS attack vector mentioned above)I hope these improvements are helpful to improve the server component. I haven't yet learned Swift, and am not familiar with iOS development yet, so unfortunately I couldn't improve the iOS client at the same time. So, some things I tested like image push only work in one direction (Linux -> iOS), but not yet the other way around. Using two Python-based server/clients should work, however.
P.S. I didn't see an OSI-approved
LICENSEfile in this repo. Would you be willing to choose a license and officially make this code truly Open-Source? I'd recommend the GPLv3 to help foster the ability of the community to share improvements.Anyway, Thanks for such a great and useful project!