New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.NET support #121

Merged
merged 46 commits into from Dec 29, 2017

Conversation

Projects
None yet
2 participants
@Metalnem
Contributor

Metalnem commented Dec 21, 2017

This is the C# version of AES-SIV, targeting .NET Standard 2.0. That means it's cross-platform, working on both .NET Framework and .NET Core. This version contains AES-SIV implementation that is using CMAC (I plan to add PMAC very soon). It can currently be built from scratch, but I plan to build the Nuget package for it. The full test suite is present, but I didn't write the CI script yet (I never used Travis with .NET, so I have to learn how to do that first).

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Dec 21, 2017

Contributor

Thanks for the contribution! Looks like you have a merge conflict in README.md. Otherwise this looks good upon cursory inspection. I'll try to do a more detailed review soon.

I plan to add PMAC very soon

That'd be great. I'd really like for all of the implementations in the repo to be at parity feature-wise.

Speaking of which, I'm in the process of implementing STREAM (#32) for all languages currently in-tree. Are you interested in implementing that too? It's relatively simple and straightforward.

Contributor

tarcieri commented Dec 21, 2017

Thanks for the contribution! Looks like you have a merge conflict in README.md. Otherwise this looks good upon cursory inspection. I'll try to do a more detailed review soon.

I plan to add PMAC very soon

That'd be great. I'd really like for all of the implementations in the repo to be at parity feature-wise.

Speaking of which, I'm in the process of implementing STREAM (#32) for all languages currently in-tree. Are you interested in implementing that too? It's relatively simple and straightforward.

@Metalnem

This comment has been minimized.

Show comment
Hide comment
@Metalnem

Metalnem Dec 21, 2017

Contributor

Yes, my plan was to implement STREAM right after I finish the PMAC implementation.

Contributor

Metalnem commented Dec 21, 2017

Yes, my plan was to implement STREAM right after I finish the PMAC implementation.

@tarcieri tarcieri added the security label Dec 21, 2017

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Dec 21, 2017

Contributor

I didn't write the CI script yet (I never used Travis with .NET, so I have to learn how to do that first).

I use Appveyor on several other projects for Windows builds. Does that work better for you?

Contributor

tarcieri commented Dec 21, 2017

I didn't write the CI script yet (I never used Travis with .NET, so I have to learn how to do that first).

I use Appveyor on several other projects for Windows builds. Does that work better for you?

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Dec 22, 2017

Contributor

For what it's worth, I added an Appveyor integration and have it checking the Ruby code on Windows now at least

Contributor

tarcieri commented Dec 22, 2017

For what it's worth, I added an Appveyor integration and have it checking the Ruby code on Windows now at least

Metalnem added some commits Dec 26, 2017

@Metalnem

This comment has been minimized.

Show comment
Hide comment
@Metalnem

Metalnem Dec 26, 2017

Contributor

PMAC, AEAD and STREAM are now all implemented. The only thing left is to create the Nuget package, which I will do when after you review the code.

Contributor

Metalnem commented Dec 26, 2017

PMAC, AEAD and STREAM are now all implemented. The only thing left is to create the Nuget package, which I will do when after you review the code.

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Dec 26, 2017

Contributor

Excellent! I'll try to take a look later today

Contributor

tarcieri commented Dec 26, 2017

Excellent! I'll try to take a look later today

@tarcieri tarcieri self-requested a review Dec 27, 2017

}
aes = Aes.Create();
aes.Mode = CipherMode.ECB;

This comment has been minimized.

@tarcieri

tarcieri Dec 27, 2017

Contributor

It's unfortunate you have to implement it this way, but I double checked and .NET apparently does not provide an AES-CTR implementation?

@tarcieri

tarcieri Dec 27, 2017

Contributor

It's unfortunate you have to implement it this way, but I double checked and .NET apparently does not provide an AES-CTR implementation?

This comment has been minimized.

@Metalnem

Metalnem Dec 27, 2017

Contributor

I wasn't happy about this either, but this was the only way - .NET really doesn't provide an AES-CTR implementation.

@Metalnem

Metalnem Dec 27, 2017

Contributor

I wasn't happy about this either, but this was the only way - .NET really doesn't provide an AES-CTR implementation.

Show outdated Hide outdated dotnet/README.md
@tarcieri

Well, as far as I can tell this is mostly fine. Left a few line notes, but I see nothing glaringly wrong with it.

/// <summary>
/// Defines the basic operations of message authentication code.
/// </summary>
internal interface IMac : IDisposable

This comment has been minimized.

@tarcieri

tarcieri Dec 27, 2017

Contributor

It looks like there's a KeyedHashAlgorithm interface you can implement:

https://msdn.microsoft.com/en-us/library/system.security.cryptography.keyedhashalgorithm(v=vs.110).aspx

@tarcieri

tarcieri Dec 27, 2017

Contributor

It looks like there's a KeyedHashAlgorithm interface you can implement:

https://msdn.microsoft.com/en-us/library/system.security.cryptography.keyedhashalgorithm(v=vs.110).aspx

This comment has been minimized.

@Metalnem

Metalnem Dec 27, 2017

Contributor

I actually implemented the KeyedHashAlgorithm initially, but after performance testing I decided that it was not the best idea (see the commit 9b768d5). The problem with KeyedHashAlgorithm is that it's not actually an interface, but an abstract class that does a whole lot unnecessary things behind the scenes. Also, given that the crypto part of the .NET standard library is stuck in the past, I though that we should't try too hard to be compatible with it.

@Metalnem

Metalnem Dec 27, 2017

Contributor

I actually implemented the KeyedHashAlgorithm initially, but after performance testing I decided that it was not the best idea (see the commit 9b768d5). The problem with KeyedHashAlgorithm is that it's not actually an interface, but an abstract class that does a whole lot unnecessary things behind the scenes. Also, given that the crypto part of the .NET standard library is stuck in the past, I though that we should't try too hard to be compatible with it.

Show outdated Hide outdated .travis.yml
@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Dec 28, 2017

Contributor

Make sure to add yourself here as well:

https://github.com/miscreant/miscreant/blob/master/AUTHORS.md

Contributor

tarcieri commented Dec 28, 2017

Make sure to add yourself here as well:

https://github.com/miscreant/miscreant/blob/master/AUTHORS.md

@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Dec 28, 2017

Contributor

Also note that in Miscreant 0.4 I plan on adding a key wrap API (#106) which generates a random encryption key and then seals it under a key-encrypting-key (KEK)

Contributor

tarcieri commented Dec 28, 2017

Also note that in Miscreant 0.4 I plan on adding a key wrap API (#106) which generates a random encryption key and then seals it under a key-encrypting-key (KEK)

@Metalnem

This comment has been minimized.

Show comment
Hide comment
@Metalnem

Metalnem Dec 29, 2017

Contributor

No problem, I'll implement all the new features that you add to the other libraries.

Contributor

Metalnem commented Dec 29, 2017

No problem, I'll implement all the new features that you add to the other libraries.

@tarcieri tarcieri merged commit 2d46674 into miscreant:master Dec 29, 2017

4 checks passed

codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - js/package.json No new issues
Details
@tarcieri

This comment has been minimized.

Show comment
Hide comment
@tarcieri

tarcieri Dec 29, 2017

Contributor

Merged! Thank you for the contribution!

Contributor

tarcieri commented Dec 29, 2017

Merged! Thank you for the contribution!

@tarcieri tarcieri added the C# label Dec 29, 2017

@Metalnem Metalnem deleted the Metalnem:dotnet branch Dec 29, 2017

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