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

Unexpected exception while using Python HTTP server with SSL #1694

Closed
grydz opened this issue Jan 2, 2024 · 11 comments · Fixed by #1696
Closed

Unexpected exception while using Python HTTP server with SSL #1694

grydz opened this issue Jan 2, 2024 · 11 comments · Fixed by #1696

Comments

@grydz
Copy link

grydz commented Jan 2, 2024

Description of the problem

A simple Python code for running an HTTP server with SSL (no 3rd party library) suddenly fails with gramine-sgx after an update in a Docker image using Ubuntu 22.04. The diff between Docker images shows a different version of libssl3: 3.0.2-0ubuntu1.12 vs. 3.0.2-0ubuntu1.10. Version of Python (3.10.6-1~22.04) and Gramine (1.5) are the same.

Steps to reproduce

Build and run the docker image from this repository https://github.com/Cosmian/gramine-py-bug or use the following Python code in Gramine: https://github.com/Cosmian/gramine-py-bug/blob/main/python/scripts/main.py.

Expected results

HTTPS server should run forever and accept HTTP GET and POST requests.

Actual results

OSError and PermissionError with the following traceback:

Traceback (most recent call last):
  File "/usr/lib/python3.10/ssl.py", line 1050, in _create
    self.getpeername()
OSError: [Errno 107] Transport endpoint is not connected

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "//scripts/main.py", line 42, in <module>
    httpd.socket = ctx.wrap_socket(httpd.socket, server_side=True)
  File "/usr/lib/python3.10/ssl.py", line 513, in wrap_socket
    return self.sslsocket_class._create(
  File "/usr/lib/python3.10/ssl.py", line 1062, in _create
    notconn_pre_handshake_data = self.recv(1)
  File "/usr/lib/python3.10/ssl.py", line 1290, in recv
    return super().recv(buflen, flags)
PermissionError: [Errno 13] Permission denied

Note that I'm not able to reproduce the bug outside of Gramine.

gramine.log

Gramine commit hash

1.5

@grydz
Copy link
Author

grydz commented Jan 2, 2024

After some investigation, it looks like the problem comes from this fix: python/cpython#108318.

The former image was using libpython3.10-stdlib/now 3.10.12-1~22.04.2 and the latter libpython3.10-stdlib/now 3.10.12-1~22.04.3 (22.04.2 -> 22.04.3).

I can see that the file /usr/lib/python3.10/ssl.py from the last version contains the patch.

Do you have an idea why this modification introduced the bug with Gramine?

@dimakuv
Copy link
Contributor

dimakuv commented Jan 2, 2024

Thanks @grydz for a detailed bug description!

I am looking at it. I can reproduce it even on my Ubuntu 20.04 and Python 3.8. Apparently the security bug fix that you mention was backported to Python 3.8 too (I looked at my local /usr/lib/python3.10/ssl.py).

Here's the relevant Gramine snippet:

(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- socket(INET, SOCK_CLOEXEC|STREAM, 0) = 0x3
(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- setsockopt(3, 1, 2, 0xedc818c, 4) = 0x0
(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- bind(3, {family=IPv4,ip=127.0.0.1,port=4433}, 16) = 0x0

(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- listen(3, 5) = 0x0

(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- getsockopt(3, 1, 3, 0xedc82a8, 0xedc82a4) = 0x0
(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- getsockname(3, 0xedc7e00, 0xedc7dec) = 0x0
(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- getpeername(3, 0xedc8220, 0xedc821c) = -107
(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- ioctl(3, FIONBIO, 0xedc828c) ...
(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- return from ioctl(...) = 0x0
(libos_parser.c:1658:buf_write_all) [P1:T1:python3.8] trace: ---- recvfrom(3, 0xde3ad10, 0x1, 0, 0, 0) = -13

After that, Python raises an error. Note that recvfrom() returns -13 which is -EACCES.

However, Python's ssl.py expects ENOTCONN and EINVAL:
https://github.com/python/cpython/blob/f637b44dd279a7e42d34dc3a00959315b1778072/Lib/ssl.py#L1024-L1033

@dimakuv
Copy link
Contributor

dimakuv commented Jan 2, 2024

Similar snippet of a native execution (collected via strace):

socket(AF_INET, SOCK_STREAM|SOCK_CLOEXEC, IPPROTO_IP) = 3
setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
bind(3, {sa_family=AF_INET, sin_port=htons(4433), sin_addr=inet_addr("127.0.0.1")}, 16) = 0
getsockname(3, {sa_family=AF_INET, sin_port=htons(4433), sin_addr=inet_addr("127.0.0.1")}, [16]) = 0

listen(3, 5)                            = 0

getsockopt(3, SOL_SOCKET, SO_TYPE, [1], [4]) = 0
getsockname(3, {sa_family=AF_INET, sin_port=htons(4433), sin_addr=inet_addr("127.0.0.1")}, [128->16]) = 0
getpeername(3, 0x7ffd596b6ed0, [16])    = -1 ENOTCONN (Transport endpoint is not connected)
ioctl(3, FIONBIO, [1])                  = 0
recvfrom(3, 0x7fbe18b63a40, 1, 0, NULL, NULL) = -1 ENOTCONN (Transport endpoint is not connected)

So clearly, Gramine is different from native execution in a wrong error code that it returns on a listening socket during recvfrom().

@dimakuv
Copy link
Contributor

dimakuv commented Jan 2, 2024

@grydz Will you be able to test my fix #1696 on your workload?

@grydz
Copy link
Author

grydz commented Jan 3, 2024

This is great, thank you very much for the quick fix!

It works perfectly well: https://github.com/Cosmian/gramine-py-bug/tree/gramine-from-source

@grydz
Copy link
Author

grydz commented Jan 3, 2024

@dimakuv is it feasible to release v1.6.1 (or v1.5.1) with the patch?

@dimakuv
Copy link
Contributor

dimakuv commented Jan 3, 2024

@grydz We typically don't do such point releases.

What we can do is to create a tag when this patch will be merged into Gramine. Then you'll have an immutable version of Gramine that you can git-clone, build and install. Unfortunately this means that you'll have to build Gramine yourself, instead of using a pre-built package. Is this something that will work for you?

I believe the next Gramine release (v1.7) will happen around March-April... It's probably too large of a time delay for you?

@grydz
Copy link
Author

grydz commented Jan 3, 2024

@grydz We typically don't do such point releases.

What we can do is to create a tag when this patch will be merged into Gramine. Then you'll have an immutable version of Gramine that you can git-clone, build and install. Unfortunately this means that you'll have to build Gramine yourself, instead of using a pre-built package. Is this something that will work for you?

I believe the next Gramine release (v1.7) will happen around March-April... It's probably too large of a time delay for you?

Sure, I was aware of Gramine's release life cycle but still, I asked :)

A git tag would be great, thanks!

grydz added a commit to Cosmian/mse-docker-base that referenced this issue Jan 5, 2024
@dimakuv
Copy link
Contributor

dimakuv commented Jan 12, 2024

A git tag would be great, thanks!

@grydz Other maintainers are wary of adding a separate git tag, see discussions in #1696. I hope you can live with a commit hash (it's guaranteed to be immutable)? I.e., instead of git checkout <tag name>, you'll do git checkout <commit hash>.

@dimakuv
Copy link
Contributor

dimakuv commented Jan 12, 2024

@grydz The commit is: 35f5d4b8b89bf713668bb1cfb5bae86c5d9ee387. You can use it, we merged the fix.

@grydz
Copy link
Author

grydz commented Jan 15, 2024

Thank you very much!

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

Successfully merging a pull request may close this issue.

2 participants