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

rust: switch from OpenSSL to rustls #4931

Merged
merged 2 commits into from
May 5, 2021
Merged

rust: switch from OpenSSL to rustls #4931

merged 2 commits into from
May 5, 2021

Conversation

wchargin
Copy link
Contributor

@wchargin wchargin commented May 4, 2021

Summary:
The RustBoard data server now uses rustls as a TLS backend instead of
using OpenSSL. This culls the libcrypto.so.1.1 and libssl.so.1.1
shared library dependencies, fixing #4928.

Test Plan:
Run with --logdir gs://tensorboard-bench-logs/edge_cgan to verify that
RustBoard still works. Then, dump the ldd output:

$ cd tensorboard/data/server/
$ cargo build --release
$ ldd target/release/rustboard
	linux-vdso.so.1 (0x00007ffe8afbd000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f422533e000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f422531c000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f42251d8000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f42251d2000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f422500d000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f4225b66000)

…and verify that all the shared libraries are on the manylinux2010
whitelist
.

wchargin-branch: rust-rustls

Summary:
The RustBoard data server now uses `rustls` as a TLS backend instead of
using OpenSSL. This culls the `libcrypto.so.1.1` and `libssl.so.1.1`
shared library dependencies, fixing #4928.

Test Plan:
Run with `--logdir gs://tensorboard-bench-logdir/edge_cgan` to verify
that RustBoard still works. Then, dump the `ldd` output:

```
$ cd tensorboard/data/server/
$ cargo build --release
$ ldd target/release/rustboard
	linux-vdso.so.1 (0x00007ffe8afbd000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f422533e000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007f422531c000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f42251d8000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007f42251d2000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f422500d000)
	/lib64/ld-linux-x86-64.so.2 (0x00007f4225b66000)
```

…and verify that all the shared libraries are on the [manylinux2010
whitelist].

[manylinux2010 whitelist]: https://www.python.org/dev/peps/pep-0571/#the-manylinux2010-policy

wchargin-branch: rust-rustls
wchargin-source: 51e7f815217d9a1ce3252757b4de3672635424ca
@wchargin
Copy link
Contributor Author

wchargin commented May 4, 2021

@psybuzz: Would you mind giving this a sanity check on macOS? I think
that it should be pretty safe—this removes a system-level dependency
on OpenSSL, and rustls/ring don't link against system libraries as
far as I can tell. But it would be nice to double-check.

The following should suffice:

bazel run //tensorboard -- --load_fast true --logdir gs://tensorboard-bench-logs/edge_cgan --bind_all

Only the `rustls-tls` feature is required (specifying `rustls` just
pulled in the optional dependency but did not actually activate the
feature flag), and manually calling `use_rustls_tls()` does not appear
to be necessary since `rustls` is the only TLS backend.

wchargin-branch: rust-rustls
wchargin-source: 29cc87f4bbd38d2282bbdace546b275eddb1abff
Copy link
Contributor

@psybuzz psybuzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked that with this PR, running bazel run //tensorboard -- --load_fast true --logdir gs://tensorboard-bench-logs/edge_cgan --bind_all on MacOS 11.3 works properly; scalars and images load fine.

@wchargin
Copy link
Contributor Author

wchargin commented May 5, 2021

Thank you!

@wchargin wchargin merged commit e61a497 into master May 5, 2021
@wchargin wchargin deleted the wchargin-rust-rustls branch May 5, 2021 05:13
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.

2 participants