Skip to content

Move off BouncyCastle onto .NET's own crypto, keep jose-jwt #647

Description

@NiclasOlofsson

Portable.BouncyCastle 1.9.0 is a dependency of the core MiNET package
(MiNET.csproj:66). Everything we use it for is now supported natively by .NET on
every platform we target, so the dependency can go. jose-jwt stays: JWT encoding
is a separate concern and there is no in-box equivalent.

This was not true when the crypto was written. .NET had no way to export a DER
SubjectPublicKeyInfo from an EC key, which is why the code reaches for
BouncyCastle in the first place. ExportSubjectPublicKeyInfo and
ImportSubjectPublicKeyInfo arrived in .NET Core 3.0 and close that gap.

The mapping

Current Native replacement
SubjectPublicKeyInfoFactory.CreateSubjectPublicKeyInfo(k).GetEncoded() key.ExportSubjectPublicKeyInfo()
PublicKeyFactory.CreateKey(der) ECDiffieHellman.ImportSubjectPublicKeyInfo / ECDsa.ImportSubjectPublicKeyInfo
ECPublicKeyParameters / ECPrivateKeyParameters ECParameters
CipherUtilities.GetCipher("AES/CFB8/NoPadding"), IBufferedCipher Aes with CipherMode.CFB and FeedbackSize = 8
SecureRandom.GetInstance("SHA256PRNG") RandomNumberGenerator
ECKeyPairGenerator + ECKeyGenerationParameters ECDiffieHellman.Create(ECCurve.NamedCurves.nistP384)

Per Microsoft's cross-platform cryptography table,
AES-CFB8 and ECDH on NIST P-384 are supported on Windows, Linux, macOS, iOS and
Android. Those were the two that could have blocked this.

Call sites

Server:

  • MiNET/LoginMessageHandler.cs:399 PublicKeyFactory.CreateKey on the client x5u
  • MiNET/LoginMessageHandler.cs:473-485 identity key, remote key encoding, key pair generation
  • MiNET/LoginMessageHandler.cs:542 the handshake x5u the server sends
  • MiNET/Utils/Cryptography/CryptoUtils.cs:142 key pair generation
  • MiNET/Utils/Cryptography/CryptoUtils.cs:176, 207, 333 public key encoding
  • MiNET/Utils/Cryptography/CryptoUtils.cs:343 private key parameters
  • MiNET/Utils/Cryptography/CryptoContext.cs:36, 40 IBufferedCipher on the session
  • MiNET/Utils/Cryptography/CryptoUtils.cs:119, 130 the encrypt and decrypt path

Client (test tooling, same treatment):

  • MiNET.Client/McpeClientMessageHandlerBase.cs:90
  • MiNET.Client/MiNetClient.cs:228

Dead code to delete rather than port

CryptoUtils.ToDerEncoded builds the DER encoding by hand: it takes the raw
Windows CNG blob from ECDiffieHellmanPublicKey.ToByteArray(), skips 8 bytes, and
prepends a hardcoded 24 byte ASN.1 header. ToByteArray() is declared on the
shared base class but only the CNG implementation can produce a CNG blob, so it
throws PlatformNotSupportedException on OpenSSL. Its only caller is
CryptoTests.TestMethod1, which is [Ignore]d for exactly that reason and is the
tripwire for this issue: un-ignoring it is the last step.

CryptoUtils.ImportECDsaCngKeyFromCngKey and the commented
ECDiffieHellmanCngPublicKey.FromByteArray line nearby are the same era and go
with it.

Acceptance

  • Portable.BouncyCastle removed from MiNET.csproj.
  • A real Bedrock client completes the encrypted handshake and reaches spawn.
  • MiNET.Client still logs in against BDS, since it signs its own login chain.
  • CryptoTests.TestMethod1 un-ignored and passing on the Linux CI runner.

Why it is worth doing

One less dependency in a NuGet package, one less thing to track for security
advisories, and the crypto stops being expressed in two libraries at once. The
handshake is also the code most likely to break on a non-Windows deployment, and
this removes the class of bug the ignored test represents.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions