Improving Crypto on All Platforms

Server Development
22

Thanks for giving me some time to mull over this folks. Apologies in advance, this is a long post.

Thanks for this Paul, it's very helpful, and gave me a lot to think about.

Part of me is beginning to wonder how much of this is less a matter of the actual structure of the repository, and more a matter of its emphasis.

I believe that there is meaningful value in having the current Crypto module exist, in at least some form. Having a clearly delineated space where the API is cross-platform with the best possible implementation for all supported platforms is useful for a wide range of development work, including some server work. But Swift Crypto is not a server library, it's a Swift library, and the goal is that we are serving the whole community, not just the Server piece of it.

So, given that it's clear we're not supporting the Server community well enough, I understand this feedback to be asking for an investigation into how the server community can get better support. But I don't think we should do it at the cost of hurting other communities that rely on Crypto.

This brings me to my question, and a suggestion. My question is: what problems have we encountered due to the slower rate of evolution of the Crypto module? I don't think answers to this can be "we couldn't get x algorithm in": in my view it's a non-goal to get non-x-platform algorithms into the x-platform piece, and the new plan is that CryptoExtras (or its equivalent) will be a fully-supported piece of the library. So the real aim would be adding API to the existing types there, or making changes to the implementation.

I can think of only two issues in recent memory where this has been relevant, Sendable and a proposal to change the layout of the digest types. For Sendable, I think I have a plan for how we could address that issue, and for the digest types this is mostly an issue of us acting as your representative internally. Changes in the cross-platform implementation can be made: they just can't be made fast, and they require that the Swift on Server team that maintain Swift Crypto are held to account to actually get them through. But they can be done.

As for my suggestion, here it is: what if we changed the name of CryptoExtras, and changed the emphasis? This would be a two-step process.

First, a new name. Paul's feedback makes it clear that an issue with CryptoExtras is that it feels secondary to Crypto, not complementary. That's a good piece of feedback. I am not in any way attached to the name, and if the community have a better one they'd like to use I'm all ears.

Second, a changed emphasis. What if we have CryptoExtras (or its replacement) @_exported import Crypto? This means that, for server use-cases, there is simply no reason to import Crypto at all. Users that wish to be sure that they are getting the CryptoKit interface can still do so, but users that want all the crypto support can just import CryptoExtras and be on their way.

This also allows a band-aid on the current Sendable approach: we can compiler-guard some @retroactive Sendable conformances on the types defined in Crypto. For server use-cases, we generally own those types. For client use-cases we don't, but as Sendable conformance is simply a marker protocol I don't believe there is any particular concern with the retroactive conformance (though I'd like to have @Joe_Groff fact-check that). We can then remove the conformance at such a time as it becomes available in CryptoKit.

If we combined this with the proposed ease of contribution, does this address the bulk of the problems? If not, what am I missing?

I think this is a really interesting sub-piece of feedback, and thanks for raising it Alexander. Rest assured I have not lost sight of the bitcoin curve.

A question we have here is: what is the point of Swift Crypto? This relates to the discussion Paul and I have had about philosophy: what are we trying to serve?

I am nervous about having Swift Crypto be effectively a "standard library" for cryptography. I don't think this serves a healthy community, as it forces this high barrier to contribution. This high barrier will always be there, because any contribution that others make must be maintained in perpituity by us. As you note, maintenance requires funding and time. Many algorithms are not worthy of that time, but importantly, many more can be just as effectively implemented by others, thereby spreading the work and the cost.

An example here is the recent proposal to add XSalsa20Poly1305 for compatibility with libsodium. As the author notes, BoringSSL does not have an implementation of XSalsa20, so this would require a greenfield implementation.

In my view, this is a perfect opportunity to take something out of Crypto. @0xTim has expressed his displeasure with the build times required for BoringSSL: having things outside of Crypto allows the opportunity for interested users to avoid that build time altogether, as well as take a reduced binary size, while not costing anything more in the case that they also need Crypto. It also gives implementers agency (they don't have to get permission from maintainers to make changes) and control over timelines (they don't have to wait for me to get a spare moment to conduct reviews).

In my view, it is not a good idea for us to try to produce a monolithic library with support for all cryptography. secp256k1 is a good example. As I noted in our discussion, BoringSSL lacks an optimized implementation of secp256k1:

Additionally, BoringSSL does not have an optimised implementation of secp256k1. We can have BoringSSL implement it using the custom curves functionality, but this will make the performance of secp256k1 substantially worse than the optimised NIST curves currently exposed via Swift Crypto. That’s an unfortunate and surprising performance cliff to hit for unsuspecting users who may reasonably assume that the performance of these curves could be expected to be broadly similar.

This remains true, and raises a question: if we were to add secp256k1, would we accept a slow implementation, or would we need to find an acceleration library? If the latter, this is another C library that must be built and linked in order for Crypto to provide it, further harming the build times that the community is already unhappy with.

This takes us to Alex's post on the philosophy of cryptographic libraries. Thus far, CryptoKit is very close to "one right answer". Swift Crypto started that way, but has intentionally moved in response to feedback from the OSS community towards something much closer to "all secure algorithms". For now, this is where I want us to be.

However, I want to call out an important part of Alex's post that may have been missed:

In some sense all of these libraries have a place in the world. In another sense, the more use cases (including the need to be interoperable) that can be served from “One Right Answer” libraries, the more secure outcomes we’re likely to get.

In many ways, the thing that makes Crypto valuable is its access to a copy of BoringSSL. That means there is a strong incentive to include anything that would require or benefit from a copy of BoringSSL (or similar) to achieve its goals. However, the moment we don't need BoringSSL to do something, we should as a community ask ourselves whether a "one right answer" library is a better fit. I continue to believe this is true for secp256k1, but I remain open to feedback that the wider community doesn't agree.

7 Likes
23

Thank you @lukasa for the excellent write-up. This exactly addresses the bits I've ran into without compromising core of the library.

I'd love to compile BoringSSL only once which - though not mandatory for my use cases - is certainly lowering the entry for newcomers.

Sendability is also an annoying one for me, and I'd love to see it addressed in this way. And I also agree that we shouldn't collect all possible algorithms in one place. However, given algorithms (such as RSA) are unfortunately necessary for various different libraries - I think it would be nice to discuss how we want to organise these efforts.

I'm not eager to see three different libraries wrapping, for example, BoringSSL's RSA APIs in their own type. That will be a guaranteed way to seed confusion and incompatibilities between libraries. I think it'd be wise to organise these efforts rather than dealing with (too much) fragmentation of Crypto APIs (again).

Perhaps a workgroup of sorts could be dedicated to these efforts given the incredible amount of interest in this topic.

24

I think this is a great point for defining where the library draws the line in accepting something rather than not. Crypto saves us the trouble of bundling BoringSSL and provides us most of the primitives we need; if one needs something extra they'll have to fork crypto or rely on a separate package. This also somewhat addresses your point about "standard crypto library" which Crypto wouldn't try to be at that point: it would be the library you go to for BoringSSL bundling.
This however also means that (again, judiciously) any request for a primitive which is provided by BoringSSL, should be accepted into Crypto. Is this the way to go? I think it makes sense and gives a bit more clarity on the "bar", however:

  1. I'm not sure if it aligns with the objective of the library, with it also providing Darwin support;
  2. as Tim said, BoringSSL should probably be just an implementation detail and people probably shouldn't have to care about checking if a BoringSSL implementation exists for the algorithm they need;
  3. what would happen if an actual Swift (with integrated assembly) rewrite were to happen and BoringSSL would be gone?

This definitely addresses my concern about CryptoExtras being secondary and actually gives it a reason to exist. With this approach we separate concerns and have a clear reason for both modules: one for the client and one for the server (CryptoServer?). While it's a good solution, I do keep asking myself why we can't drift away from CryptoKit and have just one module. This would simplify the package structure. Is the problem with this simply having different APIs when using CryptoKit and SwiftCrypto? I'm guessing there might be problems when calling CryptoKit from SwiftCrypto if the APIs were different? To be clear Cory, I think your solution is fine, these are actual questions to help me understand the dislike against the CryptoKit decouple.

1 Like
25

I think we want to be a bit more judicious than that. BoringSSL supports a number of things that are very old and rough to use for compatibility with OpenSSL. We don't have that need, and so we can be a bit more judicious.

However, we should be willing to add support for things that are needed to add compatibility with the rest of the world. Under this definition of the bar, I'd be willing to expose 3DES if we had a use-case that required it (under an Insecure namespace), but the bar for exposing (say) RC4 would be very high. 3DES is merely a bad choice, but RC4 is almost apocalyptic.

I don't forsee that outcome happening any time soon. The work required to produce it is high, and the value it produces is minimal compared to the status quo. I think we'd have to have a really clear picture of why we thought that was useful.

It's not that it's a problem, it's that we're drawing a clear demarcation line. On one side is things that are common: on the other side is things that are not. As @Joannis_Orlandos has noted upstream, this has been valuable to him. I'm aware of many other users of the library for whom this is also valuable.

I want us to retain that value, but I am more than happy to de-emphasise that function of the library.

At this stage I'm not aware of anyone who wants any of these APIs to be different. Lots of folks have asked for additions, but I'm not aware of a need for a difference at this time.

1 Like
26

Another suggestion: could we have swift-crypto and swift-nio-ssl use the same vendored copy of BoringSSL? Perhaps NIOSSL could depend on the BoringSSL C modules in swift-crypto.

1 Like
27

There's an issue tracking this - Migrate Crypto Dependencies to Swift Crypto ¡ Issue #382 ¡ apple/swift-nio-ssl ¡ GitHub

TL;DR - that's the end goal but NIOSSL uses some APIs that won't make it into Swift Crypto and should be implemented in Swift instead, either using Swift cerfticates or reimplementing in NIOSSL

28

This is not something I had considered and I think would alleviate a lot of the current frustration and go a long way to improving things (with the added ease of contribution, of which there's been a lot of movement recently).

In terms of naming, I don't think we should aim it at just the server ecosystem, they're definitely the one to benefit predominantly from it but that feels a little exclusionary. CryptoSSL would follow the pattern of OpenSSL, BoringSSL etc, but feels a bit redundant (and leaking implementation details). The best I can come up with is a fully qualified Cryptography but I'll let the bike shedding on that one begin :laughing:

I think this is an important point. From a "putting-my-server-hat-on" perspective (though realistically it applies to all use cases) BoringSSL is an important piece at the moment in the Swift world - it provides a solid, well-tested implementation of many of the algorithms used today, though without the stable public API, nor a Swift API. Swift Crypto provides that layer so whilst I don't think it should be exlusively about BoringSSL, I would say that the barrier for adding algorithms that don't exist in BoringSSL should be substantially higher, because as Cory says they can be implemented, evolved and tested without the burden of Swift Crypto. (And I say this as someone who would like secp256k1 purely to get all the green ticks on jwt.io for JWTKit). Adding stuff that exists in BoringSSL should be lower (though still justifiable) to avoid vending yet-another-copy of BoringSSL in a library, especially if that library is widely used (such as NIOSSL).

I don't fully agree with this. I think Swift as an ecosystem needs a standard cryptography library that has lots of eyes on it and is well battle tested. Being able to point to that does a lot for Swift as a language outside of the Apple ecosystem, even if it is just a perception. However, I think as things stand today, and with the proposed changes (with the accompanying documentation) we effectively have that and pushing that further at the moment is not a hill I'm particularly stuck to.

TL;DR Big +1 to the proposed changes, I think this would go a long way to improving things

1 Like
29

As a side note that deserves its own reply - I would like to call out the recent changes in Crypto Extras that have enabled us in JWTKit to rely solely on Swift Crypto and support a very wide range of algorithms. This has removed a copy of BoringSSL from a large proportion of the server ecosystem and meant that thousands of clones we see a week don't need to compile BoringSSL (at least) twice. This has resulted in some noticeable compile time improvements on lower-powered platforms and is much appreciated!

7 Likes
30

Ok, so I think the main thing we need to nail down to make this transition possible is to work out what name we want to use for The Artist Formerly Known as _CryptoExtras. Cryptography can work but it isn't ideal, and arguably worsens the interaction. I agree with you that CryptoSSL and ServerCryptography are not great.

Some vague suggestions that we might kick around:

  • ExtendedCryptography
  • CryptoToolbelt
  • CryptoToolbox
  • CryptoUtilities

I'd be delighted to hear other suggestions.

31

Honestly writing import Cryptography does look quite clean, however it does clash with Crypto I guess. If we go down the server road, CryptoServer or ServerCrypto might make sense. Otherwise maybe CryptoExtended or CryptoPlus? I do like ExtendedCryptography but I'd expect the other module to be called Cryptography instead of Crypto then

Native speakers might be better suited for this task :smile:

32

This might be a dumb question but where should go new algorithms for clients if the bar is high for Crypto and the module is named around server usage ?

33

I think the point is the module won't be named around server usage and will have a name that reflects all use cases

1 Like
34

In Foundation nomenclature, the smaller library would be CryptoEssentials and the bigger one just Crypto.

7 Likes
35

I'm not sure if there's scope for renaming Crypto but there is an upcoming major version so it's possible but disruptive.

36

On the assumption that the larger library re-exports the smaller one, it would be a strictly additive change for clients importing Crypto, isomorphic to adding new API, no?

1 Like
37

API wise it should be fine. The issue is compilation/binary size as if coming from Apple platforms you get all of BoringSSL whether you like it or now. Saying that, I actually like the idea of Crypto and CryptoCore and switching them around

38

Like Tim, I don't object to the renaming per-se, but I would prefer a solution where we prodded people to engage with the change. The downside of this making the change silently work is that it surprises people like @Joannis_Orlandos who have been expecting the current behaviour. So if we wanted to go this direction, I'd prefer a more substantial rename.

1 Like
39

I want to bring up one more solution to the naming problem here. Arguably Crypto is the right name for the module and I would like to see this being used forward. It would make the migration story easier. I also agree that it is important to retain the possibility to avoid the BoringSSL dependency on Apple platforms. It brings in a lot of binary size and we expect that a lot of our server libraries are used in client side applications as well.

The solution to this that I have in mind is using the currently in review package traits. This would allow us to make the BoringSSL dependency optional on a trait like this

targets: [
    .target(
        name: "Crypto",
        dependencies: [
            .target(
                name: "CCryptoBoringSSL",
                condition: .when(traits: ["BoringSSL"])
            ),
        ]
    )
]

once the BoringSSL trait is enabled by a consumer the additional APIs currently in CryptoExtras would become available from the Crytpo module.

On a slightly related note, we could also add a new trait UnsafeCrypto to the package which could gate some of the algorithms that we are nowadays considered problematic but are still used by legacy systems.

3 Likes
40

To bikeshed a bit, these wouldn't be 'unsafe' in the sense of memory safety, but rather the problem has to do with how secure they are (as I understand it—yes?), hence: InsecureCrypto.

And if that gives folks some pause when they reach for it—it's as it should be!

2 Likes
41

Oh yeah Insecure is way better! Thanks for the suggestion.

2 Likes