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

Feedback on adopting Rust code #5381

Closed
alex opened this issue Aug 7, 2020 · 28 comments
Closed

Feedback on adopting Rust code #5381

alex opened this issue Aug 7, 2020 · 28 comments

Comments

@alex
Copy link
Member

alex commented Aug 7, 2020

We are looking into including Rust code in pyca/cryptography (see #5357). This will be done using PyO3, and thus it requires #5359 to be completed before it will happen. This means this will not happen until 2021 in all likelihood.

We are interested in feedback in things we can do to make this a smooth process, particularly from folks who are re-distributing cryptography. Please note: feedback of the form "don't use Rust" is not productive and will be disregarded.

Here are things we're currently planning to minimize pain:

  • Support the stable Rust channel.
  • Declare a Minimum Supported Rust Version (MSRV) and test and document it.
  • Continue building binary wheels for all the platforms we currently do.

Ideas we're considering:

  • Ensuring our MSRV is present in the Debian repositories
@alex
Copy link
Member Author

alex commented Aug 7, 2020

cc: @glyph @mattsb42-aws @bmw @ohemorange @Lukasa @bitprophet for our various high-profile downstreams

@Lukasa
Copy link
Member

Lukasa commented Aug 8, 2020

I don’t think I’m prominently using cryptography at this time, so...+1, go for it!

@glyph
Copy link
Contributor

glyph commented Aug 8, 2020

The only way I could be happier about this is if it meant you were also eliminating OpenSSL.

@glyph
Copy link
Contributor

glyph commented Aug 8, 2020

(I am slightly concerned about the build-dependencies side of this; particularly that pypy doesn't get binary wheels, and there are already enough ways that this can go wrong in confusing ways today.)

@alex
Copy link
Member Author

alex commented Aug 8, 2020

Is there some action we could be taking to make building PyPy wheels practical, or is the blocker there defining something akin to abi3 for PyPy?

@geofft
Copy link
Contributor

geofft commented Aug 8, 2020

Can I request that you include some optional Rust, enabled by default but disableable, for a bit of time before you depend on it?

At $work we're rebuilding cryptography from source in our monorepo using our own build system, which calls out to setup.py but manages dependencies on its own (to make sure we're not using system Python, system gcc, system rustc, etc.). We recently had a request for another PyO3 package, which we managed to put together somehow, but for cryptography I'd like to be very sure that it's building properly and using the right dependencies. It sounds from #5357 like you plan to start small - if you can keep the old implementation present and behind a setup.py flag, or something, that would let us both make sure that our build system works with setuptools-rust (or whatever you end up using) and also would give us a temporary option to keep upgrading if it doesn't work yet.

Also you probably want to ping your Debian and Fedora packagers if you haven't.

@alex
Copy link
Member Author

alex commented Aug 8, 2020

How about this:

  • The first release containing PyO3 will actually just build a no-op module. It won't export anything, and won't be used anywhere. But it will be built by setup.py, which should let folks properly test their build infra.
  • For the first release only, there'll be an env var you can set which will cause it to not build that module. In other words, you get one prod release to get your build system in order.
  • The release after that will contain non-optional Rust code which will need to be built.

@geofft
Copy link
Contributor

geofft commented Aug 8, 2020

Sounds good, and one release cycle sounds like the right amount of time.

I'd ask if you can actually use the module just to make sure we've got dynamic linking working right, but that might be catering too much to people with custom build systems :)

Oh, also - does Anaconda have any PyO3 stuff yet?

@glyph
Copy link
Contributor

glyph commented Aug 9, 2020

I'm honestly more concerned with adoption by new projects than build-system updates for existing ones. Whatever wacky gyrations you require of Twisted, we'll figure it out. But what is the experience of pip install cryptography trying out twisted on pypy3 just to see how it works, for someone with no rust toolchain installed? How do they find out what they're missing? How much configuration is required?

This situation already sucks for people missing fragments of the requisite C compiler toolchain and libraries, but at least there's a ton of folk knowledge well-diffused throughout the community to deal with it. This is going to be both horrible and novel, and it would be great to address that head-on before it ships.

@alex
Copy link
Member Author

alex commented Aug 9, 2020

The error is pretty clear: error: Can not find Rust compiler. Any of the standard instructions for installing Rust (whether that's the installer from rust-lang.org, brew, etc.) will install Rust in a way that will satisfy the error.

@h-vetinari
Copy link

@geofft: Oh, also - does Anaconda have any PyO3 stuff yet?

Not as far as I can tell, but I opened an issue to add it. The rust-compiler is already there, so it should hopefully not take too much effort.

@alex
Copy link
Member Author

alex commented Aug 9, 2020 via email

@h-vetinari
Copy link

It's already linked in the GH-UI, but I guess that's not visible via email. ;-)
conda-forge/staged-recipes#12342

@tiran
Copy link
Contributor

tiran commented Aug 11, 2020

From security perspective I welcome the introduction of Rust.

From the point of view of a downstream packager and consumer on two major Linux platforms at a $VENDOR I see one possible issue. Glyph said

The only way I could be happier about this is if it meant you were also eliminating OpenSSL.

We use and recommend PyCA cryptography as the preferred Python crypto library because PyCA cryptography does not implement its own crypto. Contrary to other Python libraries it is a wrapper of OpenSSL. We don't like self-written crypto implementations for obvious reasons. We also care about $CRYTPO_STANDARDS like a certain four-letter-acronym standard by the US government. If PyCA cryptography would start to implement its own crypto or use non-OpenSSL crypto code, then we would have to do additional work to disable this code under certain conditions. (This is totally our problem and we are getting paid to deal with it. You don't have to burden yourself with this problem.)

Other than that I don't have a major concern with Rust. It's available for Fedora. I don't plan to rebase and update python3-cryptography package in RHEL 8. A hard dependency on Rust may make life for other users of non-binary wheel platforms a bit harder. After all binary wheels are only available for Windows, macOS, and certain variants of Linux (AMD64, X86, glibc).

@bmw
Copy link
Contributor

bmw commented Aug 11, 2020

I'm excited to hear about this change!

I don't have any significant concerns on the Certbot side. Over the next few months we'll be deprecating our only distribution method that sometimes tries to compile cryptography on user's machines. For the distribution methods where we compile cryptography for our users, we should be able to pretty easily figure out getting whatever Rust version you require set up.

@alex
Copy link
Member Author

alex commented Aug 11, 2020

@tiran We do not plan to move any of the crypto under the scope of FIPS-140 from OpenSSL to cryptography. We do expect to move a) our own code that's written in C (e.g. unpadding), b) ASN.1 parsing. Neither of those are in scope for FIPS-140.

@mattsb42-aws
Copy link
Contributor

I love the idea of replacing C with Rust. :)

As others have mentioned, this will introduce build complexity for those who consume source dists for ${UNAVOIDABLE_REASONS}, but I think the cost is worth the benefit and the cadence that @alex laid out sounds like it will give plenty of time to deal with that.

The only other complication IMO is that this represents a clear cut-off point for Python 2 support. My personal opinion is that this is a good thing. As a maintainer of a downstream package, this has been a good lever to force the decision of how we will handle Python 2 support. I love the idea of being able to point to clear value that is being added that necessitates dropping Python 2 support.

@alex
Copy link
Member Author

alex commented Aug 11, 2020

What, making our CI 2x faster isn't the sort of thing your users care about :-P?

@reaperhulk
Copy link
Member

@tiran as @alex mentioned this Rust proposal does not cover the cryptographic code. That will remain OpenSSL. We are experimenting with also supporting BoringSSL, but even if we continue down that path maintaining OpenSSL compatibility remains important for a large number of our consumers.

@geofft
Copy link
Contributor

geofft commented Aug 13, 2020

@glyph Looks like https://github.com/antocuni/pypy-wheels provides manylinux2010 pypy wheels of cryptography - is it enough to make sure they're able to build Rust wheels?

I think that just requires them installing Rust in their Docker image. That image is based on the manylinux2010 one, and perhaps it'd be even better to get Rust into the manylinux2010 image itself.

@glyph
Copy link
Contributor

glyph commented Aug 14, 2020

@geofft Those definitely help, but they don't quite address my concern, which is more to do with naïve users encountering errors and giving up immediately, rather than determined engineers who have to solve a problem this way. The "negative air pressure", if you'll forgive the metaphor, which prevents people from ever trying PyPy in the first place.

But, it'd be pretty completely addressed if antocuni's wheels could be integrated into the cryptography release process and uploaded to PyPI. (What's the blocker? Is it PyPI support of some platform tag feature, or integration work on cryptography's CI?)

@simo5
Copy link
Contributor

simo5 commented Aug 17, 2020

@reaperhulk my only concern as downstream distributor is what version of Rust will be required.
Mostly in terms of building on older distributions. That said both RHEL and Fedora release updated version of Rust constantly, so we are probably well covered, as I do not think you'll follow bleeding edge, but will use stable, right ?
(The other FIPS concerns were already covered in replies to @tiran)

@alex
Copy link
Member Author

alex commented Aug 17, 2020

@simo5 yes, we'll use a stable release, and after we do our first release with rust code we'll document our MSRV and try to notify folks when it changes.

@simo5
Copy link
Contributor

simo5 commented Aug 17, 2020

@alex great, can't wait to see some sanity^Wrust in the code :-)

@fochoao

This comment has been minimized.

@fochoao

This comment has been minimized.

@fochoao

This comment was marked as off-topic.

@pyca pyca temporarily blocked fochoao Aug 23, 2020
@alex
Copy link
Member Author

alex commented Sep 5, 2020

Thanks to everyone who provided feedback here! I'm going to close this as I think we've reached a steady-state on our decision of how to proceed. #5410 is the PR where we'll track actually implementing, and you're of course welcome to leave a review of it.

@alex alex closed this as completed Sep 5, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests