Support BoringSSL

[nmittler]

This is important for projects that require FIPS compliance. This would add an optional feature that would provide a BoringSSL implementation of the crypto APIs.

Can potentially leverage Cloudflare's boring, which already has FIPS as a feature.

[nmittler]

FYI I've actually already started work on this in my own branch.

[djc]

I'm not convinced that we'll want to maintain a boring backend in this repository, but obviously if the crypto::Session trait needs to be adapted to implement this we'd be open to any improvements that need to be made!

[nmittler]

@djc understood, thanks. Would you be open to having a separate repository for this under quinn-rs?

[djc]

Hmm, not sure. If we put it in quinn-rs, I wonder if that gives off an implicit endorsement from the maintainers of the org, which I'm not sure makes sense for this case. @Ralith what do you think?

BTW, I think we'd be happy to link it from the docs and/or README.

You might also be interested in https://github.com/rustls/rustls/issues/521, and maybe in some of the work moving towards getting a FIPS-enabled Rust crypto primitive backend for rustls?

[Ralith]

I'm not opposed to having it under the quinn-rs org so long as maintenance expectations/endorsement are clearly documented, since it'd ensure we can at least facilitate changes of maintainership down the line if needed. We also have other unpolished repos in the org already.

+1 to FIPSifying rustls potentially being a lower-maintenance option for everyone, though.

[nmittler]

@Ralith @djc I'm getting close to having BoringSSL passing all of the integration tests ... just need to implement 0-RTT. If we split this out into a separate repo, however, testing becomes trickier. Ideally, I'd want to run all of the quinn integration tests against my provider. The first thought that comes to mind is to refactor the quinn tests into a "conformance test suite" crate that can be used in both repos. Thoughts?

[djc]

Seems reasonable to me...

[stevenctl]

Would a branch on the main repo simplify piggy-backing on your test infra?

[nmittler]

Would a branch on the main repo simplify piggy-backing on your test infra?

Good question regarding test infra. I certainly don't want to reinvent CI for this effort. I'm not sure a branch is really sustainable, however, since we'd want it to be a versioned entity. Regardless, having the code separate (branch or repo) also raises the concern of keeping it in sync with the latest quinn. Lots of details to sort out.

[djc]

I agree that we shouldn't have anything live on a branch forever.

[Ralith]

A test suite crate that's parameterized on crypto stack sounds like a graceful solution, though it might take some effort. Graceful integration with the built-in unit testing harness seems unlikely, for one. Care would be needed to avoid significantly degrading the testing experience, though maybe there's also opportunity here to make things cleaner than the status quo.

[nmittler]

@Ralith

A test suite crate that's parameterized on crypto stack sounds like a graceful solution, though it might take some effort.

I've effectively already done this work in my test branch. I needed it in order to make sure that everything actually works.

I'll scrub it before turning into a separate crate. We don't want this to become a burden for test authors within quinn.

[nmittler]

@djc @Ralith I threw together a WIP PR for the conformance test suite: https://github.com/quinn-rs/quinn/pull/1494. PTAL when you have a chance.

[nmittler]

@djc @Ralith IIUC your motivation for a separate BoringSSL repo is to clarify that ownership is distinct from the quinn repo, which makes sense. My main concern is the cost of multiiple repositories WRT setting up CI as well as keeping it up-to-date with the main quinn repo.

As an alternative, would you be open to a BoringSSL crate within the quinn repo? It could be clearly marked as experimental or as-is, with me listed as the only author. In the description we can state that the purpose is to fill the FIPS gap until rustls is FIPS certified.

[djc]

I wonder if the right solution here is to invert the dependency: the BoringSSL-based crypto::Session impl stays in a different crate, but we make that crate a dev-dependency in quinn-proto, and offer a way to switch the tests (maybe a Cargo feature, or an environment variable?) to that impl instead of the rustls-based one.

[nmittler]

@djc that sounds right to me. It would also eliminate the need for a testing crate altogether. Shall I send a PR?

[djc]

Yeah, please try it! I think the ideal scenario here is that the boringssl-based tests run in that repo's CI, cloning the quinn repo and running it with whatever switch it needs.

[nmittler]

@djc Oh sorry, I'm think I'm confused now ... Based on https://github.com/quinn-rs/quinn/issues/1488#issuecomment-1434883862, I thought you were behind the idea of a BoringSSL crate, rather than a separate repository? Can you clarify?

[djc]

Sorry -- no, I'd still prefer for it to be a separate repo. I don't think it's necessary that me and @Ralith and contributors to this repo are responsible for keeping the tests passing on the BoringSSL crate, keeping any other dependencies up to date, etc, which is the implicit contract for keeping it in this repo.

Setting up CI should be fairly straightforward, just copy-paste the Actions workflow and adapt it slightly. These are usually pretty cookie cutter across Rust workspaces.

[nmittler]

Got it ... thanks for clarifying. In that case, I'm inclined to continue down the path of the testing crate. Let me see if I can find a reasonable way to address your visibility concerns on https://github.com/quinn-rs/quinn/pull/1494. I think even with cloning we'd still run into the same visibility issues.

[djc]

Yeah, that's why I suggested a way outside of the testing crate approach. I don't think there is a good way out of the visibility issue.

[nmittler]

@djc just to follow up, I think I found a way of fixing the visibilty problem in https://github.com/quinn-rs/quinn/pull/1494. Take a look and let me know what you think.

[nmittler]

@djc regarding the separate repository, would you be able to create it under the quinn-rs org and grant me permission?

[djc]

I've created https://github.com/quinn-rs/quinn-boring, you should have admin access.

[nmittler]

Thanks, @djc! Now I'm just trying to sort out how to push the first commit, since the UI doesn't let me do anything to an empty repository (e.g. forking). I tried just pushing directly to the repository with git push, but it failed with a permissions issue (not sure if github repos allow direct pushing by default, anyway). Looks like gh repo create will let me push local code, but it seems to only work if I'm a member of the organization (quinn-rs). It also doesn't seem like the right command, since the repo already exists.

I'm probably missing something simple ... any suggestions?

[djc]

You should be able to push a branch, along the lines of git push -u origin main? (After setting the origin using git remote add origin <repo-url>, I suppose.)

[nmittler]

Ah yeah so I guess it's just a permissions issue:

$ git push -u upstream main
ERROR: Permission to quinn-rs/quinn-boring.git denied to nmittler.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

I manually set the upstream remote:

$ git remote -v
upstream    git@github.com:quinn-rs/quinn-boring.git (fetch)
upstream    git@github.com:quinn-rs/quinn-boring.git (push)

[djc]

It looks like you haven't accepted your GitHub invite to become a repo admin yet, did it end up in your spam?

https://github.com/quinn-rs/quinn-boring/invitations

[nmittler]

Ah I missed the invitation. All set now, thanks

[nmittler]

@djc I've been going down the path of having quinn-boring in a separate repository and quinn-proto taking a dev-dependency on it, so that we can run the integration tests with boring vs rustls. However, it looks like I'm running into a compiler bug that's preventing this from working.

Moving the testing code into a separate crate (as attempted in #1494) would fix the problem, although I know you had concerns about the number of things that needed to be public. Any other ideas on how we might proceed?

[nmittler]

@djc I've moved away from trying to re-use the tests under quinn-proto in favor of just writing my own integration tests. For 0-RTT, I was looking at the zero_rtt test. On my macbook, this test is successful, but takes over 10s ... I suspect it's running into a timeout. Do you see the same? Perhaps an issue with the test, itself?

[nmittler]

Resolved by #1496. Will follow up shortly with the boring-ssl provider in https://github.com/quinn-rs/quinn-boring/pull/2