PointyCastle / pointycastle

Moved into the Bouncy Castle project: https://github.com/bcgit/pc-dart
MIT License
271 stars 75 forks source link

Bring back old Hard-Coded Registry #147

Closed yshrsmz closed 6 years ago

yshrsmz commented 6 years ago

131

brought back https://github.com/PointyCastle/pointycastle/commit/2da75e5a8d7bdbf95d08329add9f13b9070b75d4, which you need to call initCipher beforehand.

I'm currently working on creating a bit more dynamic registry as stated in here: https://github.com/PointyCastle/pointycastle/issues/131#issuecomment-412473515. I think it may take some time.

but anyway, here's the current status.

stevenroose commented 6 years ago

Hmm. This goes back a lot. There's also some changes that revert more recent fixes. I think we don't necessarily have to go back to the initCipher() situation, where all dynamic registries are centralized. The FACTORY_CONFIG variables work and can be used for that instead.

What we need is a registry that does not use reflection to receive all these FACTORY_CONFIG objects. It shouldn't be that hard. I think either September 1st or September 22nd I could try and implement it!

yshrsmz commented 6 years ago

Thanks for the quick review!

If I understand correctly, Registry class would take some function as a constructor parameter, which registers all related FACTORY_CONFIGs, right? And if so, since registry is there at a base class(such as Digest), does it cause circular dependency?

stevenroose commented 6 years ago

Hmm, a tiny bit. Logic-wise not really, just for the registration.

Registry needs a method void register<T>(FactoryConfig config). This is fine, doesn't need any dependency on the rest of the project.

But somewhere there should be a method that registers all the algos. Preferably this can be called lazily on the first usage of the registry. Something like Registry.ensureInit() which calls all the registry.register<Digest>(Md5Digest.FACTORY_CONFIG)...

yshrsmz commented 6 years ago

Ah, I've misunderstood the concept. sorry.

So there's one single(possibly multiple?) global Registry instance, not as a static field of Digest or KeyDerivator.

stevenroose commented 6 years ago

Hmm, it could be type-specific, but then you will have to cross different types. F.e. "SHA-256/HMAC" will ne in the Mac class registry, but will have to use the Digest registry to construct the SHA-256 digest etc.

I could be either, I think. No actual preference or arguments for either. Maybe an argument for a single global one is that it's easier to use different implementations. F.e. a mirrors-feeded one and a hard-coded one. Or even a disabled one.

Cretezy commented 6 years ago

Great work! This works perfectly in Flutter (except SecureRandom, which needs to be provided but that might actually be intentional).

What other work need to be done for this PR to be put into master? I'd like to contribute as much as I can since this is quite an important PR

yshrsmz commented 6 years ago

Thanks!

As @stevenroose said, this PR is too out of date. So I think I'm going to close this.

But I'm now trying to implement something more similar to the current master implementation, but without reflection.

Cretezy commented 6 years ago

It's out of date? Your branch (registry) is the only thing I could get working in Flutter. What should I be using?

yshrsmz commented 6 years ago

I've created another PR(#148), which I believe is more sophisticated. So closing this one.