search

bcgit / pc-dart

Pointy Castle - Dart Derived Bouncy Castle APIs
MIT License
237 stars 122 forks source link

Update to Null-Safety (and add 2 AES modes) #77

Closed AKushWarrior closed 3 years ago

AKushWarrior commented 3 years ago

This PR contains a version of pointycastle fully migrated to non-nullability. It updates the version of this library to 3.0.0, as per this recommendation: https://dart.dev/null-safety/migration-guide#package-version

All checks are passing on my computer (Linux - Ubuntu 20.10). I have not tested on any other computers, but also do not foresee any reason that the checks would not pass on a different platform.

This is a solid starting point, but there are likely nullable variables and parameters which don't need to be nullable. I tried to root out as many as possible, but I may have missed a few.

However, I recommend this PR be published, so that downstream users may begin the migration process.

AKushWarrior commented 3 years ago

We'll have to fix the Travis CI. It failed because the non-nullable-by-default feature needs the Dart SDK version 2.12 (currently in beta) to work. Instead of installing the stable branch of Dart, we should temporarily install the beta one.

mwcw commented 3 years ago

Hi,

I'll set this up on a release branch "3.0.0-nullsafety.0" so that it is away from the non null safe branch until the nullsafe dart leaves beta.

On OSX Dart SDK version: 2.12.0-133.7.beta (beta) (Tue Jan 12 09:25:38 2021 +0100) on "macos_x64", I got the following from pub run test:

00:37 +249 -1: test/key_derivators/hkdf_test.dart: HKDF tests derivator tests SHA-256/HMAC/HKDF: deriveKey: Lorem ipsum dolor sit amet[...] [E]                                                                                   
  type 'Null' is not a subtype of type 'int' in type cast
  dart:typed_data                                              _IntListMixin.+
  package:pointycastle/key_derivators/hkdf.dart 88:41          HKDFKeyDerivator.deriveKey
  package:pointycastle/src/impl/base_key_derivator.dart 14:15  BaseKeyDerivator.process
  test/test/key_derivators_tests.dart 33:26                    _runKeyDerivatorTest
  test/test/key_derivators_tests.dart 21:19                    runKeyDerivatorTests.<fn>.<fn>.<fn>

00:37 +252 -2: test/key_derivators/hkdf_test.dart: HKDF tests derivator tests SHA-256/HMAC/HKDF: deriveKey: En un lugar de La Mancha, [...] [E]                                                                                   
  type 'Null' is not a subtype of type 'int' in type cast
  dart:typed_data                                              _IntListMixin.+
  package:pointycastle/key_derivators/hkdf.dart 88:41          HKDFKeyDerivator.deriveKey
  package:pointycastle/src/impl/base_key_derivator.dart 14:15  BaseKeyDerivator.process
  test/test/key_derivators_tests.dart 33:26                    _runKeyDerivatorTest
  test/test/key_derivators_tests.dart 21:19                    runKeyDerivatorTests.<fn>.<fn>.<fn>

however dart run test/all_tests_web.dart all tests pass.

Off topic we need to have one way of running all tests, that rules them all so to speak.

Once again thanks your effort here, we really appreciate it.

MW

AKushWarrior commented 3 years ago

I've been running dart test test/all_tests_web.dart under the assumption that that file linked to all other tests. Not sure why pub run test is different. It's likely that the implementer of HKDF forgot to link to the HKDF test from all_tests_web.

Regardless, I'll take a look at HKDF. This will probably be a pretty easy fix.

AKushWarrior commented 3 years ago

@mwcw HKDF is fixed, and I tried to find the tests that were running in pub run test and not dart test test/all_tests_web.dart. I added most of them (the ones I could find, like HKDF), but there's still 60 or so "phantom tests" that run only when the command pub run test is called. Not sure why that's happening.

AKushWarrior commented 3 years ago

I added CCM and IGE (block cipher modes) with a few test cases. Give me a minute, I'll put them in the README and changelog.

mwcw commented 3 years ago

Hi,

I have been merging this into a new branch called "nullsafety".

I have published it as 3.0.0-nullsafety.2 as I had already pushed the .0 and .1 tags to the public repo.

Let me know how it goes.

I need to sort out Travis.

MW

AKushWarrior commented 3 years ago

Looks fine to me. I'll close this PR.