Add System.Security.Cryptography.Hashing.Algorithms support

[janhenke]

Adding Interop.HMAC.cs (identical to Linux, since both use the same openssl libraries). This enables building the System.Security.Cryptography.Hashing.Algorithms.dll assembly.

/cc @josteink @stephentoub

[josteink]

LGTM

[stephentoub]

Adding Interop.HMAC.cs (identical to Linux, since both use the same openssl libraries)

Rather than duplicating the identical file, is there a location we could move the existing file to that would make sense to be consumed by both?

For example, if OSX is the odd duck here with regards to using an outdated openssl API, could we just move the Linux openssl file to the Unix\libcrypto folder, and use that from both Linux and FreeBSD? This slightly breaks from the pattern we've used thus far, where if there's any difference between platforms, each platform gets its own copy of the interop file. But I'd be ok changing that slightly to say that a platform can have its own file if it needs to effectively override the default file that's used more broadly.

@ellismg, other thoughts?

[josteink]

@stephentoub This is the primary subject in this issue, also created by @janhenke

https://github.com/dotnet/corefx/issues/2042

[stephentoub]

Thanks, I saw that issue, but I'd like to address it before creating duplication, rather than trying to clean up duplication later.

[ghost]

Would it make sense to have abstract classes under commons with most of the functionality (implemented virtual methods) and then concrete classes under interop for FreeBSD, Linux, OSX and Windows with required overrides? This way we will still have specialized classes (for future hacks / platform-specific stuff) but the code redundancy shall be minimized which is the main objective.

[stephentoub]

@jasonwilliams200OK, can you elaborate on your suggestion? It's not clear to me how that would help, but I may just not be "seeing it". With my suggestion, we'd just have two files, an Interop.HMAC.cs in Interop\Unix and an Interop.HMAC.cs in Interop\OSX, with no duplication. With your suggestion, it seems like we'd need a file for the Unix base class, one for the overridden OSX class, a factory type for each that would be used so that the consuming code wouldn't need to change, and we'd still need the interop files with the signatures wrapped by the base and derived types. Have I misunderstood?

[ghost]

@stephentoub, you are absolutely right. I think I have drank a little too much of the OOP coolaid, feel free to rip that idea out! 😄

Elaboration:

• Commons\HMAC.cs: a base abstract class for all OSes
• Unix\HMAC.cs: an abstract class inheriting Commons\HMAC, it will contain majority of common code between Linux, FreeBSD and OSX
• Windows\HMAC.cs: a concrete implementation for Windows inheriting Commons\HMAC
• OSX\HMAC.cs: a concrete implementation for OSX inheriting Unix\HMAC overriding methods necessary for OSX
• Linux\HMAC.cs: a concrete implementation for Linux inheriting Unix\HMAC, probably an empty class since Unix/HMAC contains all the required logic
• FreeBSD\HMAC.cs: a concrete implementation for FreeBSD inheriting Unix\HMAC, same as Linux, preferably just an empty class with concrete signature

The idea for having those empty concrete derived types is for possible future platform-specific overrides.

[stephentoub]

Thanks, @jasonwilliams200OK. At least for now, I would prefer to stick on the path we're on and not introduction extra class hierarchies like that; if we find a need in the future, we can always choose to do so then. For now and in this case, I think it's purely a matter of finding a properly named location for the file that's currently Common\src\Interop\Linux\libcrypto\Interop.HMAC.cs. The FreeBSD project could certainly reference it in its current location, but that's a bit strange, so we should either move it to Common\src\Interop\Unix\libcrypto\Interop.HMAC.cs, or we should find some other common location for it.

[janhenke]

How about naming the file by the library used? It is not necessary bound to the platform, but more a question whether the platform uses OpenSSL (as FreeBSD and most LInuxes do).

[stephentoub]

How about naming the file by the library used?

It is; that's what the libcrypto in the folder name is. But OSX also uses libcrypto, it just has slightly different signatures for the same functions(void instead of int return type).

[janhenke]

Well I fine with moving the file to the Unix folder and override the behaviour on OS X with a separate file. Shall I update this PR accordingly?

[stephentoub]

and override the behaviour on OS X with a separate file

Nothing should need to change for OSX. I think this is just a matter of moving the one file from the Linux directory to the Unix directory and updating the .csproj to find out in the new location.

@ellismg, are you ok with this? Or do you have a better idea in mind?

[janhenke]

I was referring to the fact that OS X is a Unix system, so in a sense the OS X file is overwriting the common Unix file then. But yes, I mean moving the current Linux file to the Unix folder and updating the reference.

[stephentoub]

I was referring to the fact...

Oh, sorry, I completely misread what you wrote and thought you were suggesting needing a separate PR. We're on the same page then.

[josteink]

LGTM.

Looks even better than last time :)

[ghost]

👍
Perhaps same strategy can be used for System.Diagnostics.Process. 😄

[stephentoub]

LGTM. I do want to wait for Matt to weigh in before merging.

[janhenke]

I can see no reason for the PR test failure (only Windows in release mode).

[janhenke]

@dotnet-bot test this please

[ellismg]

I like the approach that's taken here, it makes sense to me to have a generic "Unix" version, specialized (to either add additional functionality or make things more specific) for an individual platform when there are differences with the common API.

LGTM.

[stephentoub]

Great. Thanks, Matt.