1st shims for the LibSsl library.

[eerhardt]

I added shims the first 8 libssl functions and did a little clean up.

• Put all Interop files under "Common\Interop" in System.Net.Security.
• Removed some unused fields from StreamSizes.
• Changed some unnecessary IntPtr usages to byte[] or byte*, where appropriate.

@bartonjs @stephentoub @vijaykota @rajansingh10 @shrutigarg
(optionally, @nguerrera, @sokket if you want to take a look)

[eerhardt]

I'm fixing the OSX compile errors... :(. I guess this is a good reason we have shims, we know about these incompatibilities at build time, and not run time.

[bartonjs]

Is SafeSslContextHandle moving now just causing too much of a headache?

[eerhardt]

Yes, I wanted to leave where it is for now and move it with a separate change.
BTW - where do you think it should live? Some SafeHandles live under Microsoft.Win32.SafeHandles, which feels weird to me for Unix code.

[bartonjs]

For internal ones, I've been leaving them in Microsoft.Win32.SafeHandles.

[bartonjs]

LGTM in principle, modulo not compiling :)

We probably want a cmake functionality test for for OpenSSL compatibility levels, rather than using APPLE. If you end up making one, chime in on https://github.com/dotnet/corefx/pull/3916/files#r42436681 :)

[vijaykota]

Nit: Instead of making the method unsafe, can't you use an unsafe block?

[vijaykota]

LGTM

[nguerrera]

LGTM, but waiting to see where it lands to get things working on OS X.

[eerhardt]

Updated for PR feedback and getting it to build on OSX.

@nguerrera - PTAL at how I changed the CMake file to support OSX.

[eerhardt]

@bartonjs, @vijaykota, @shrutigarg - The System.Net.Security.Tests caught an error where it was possible that we tried invoking SSL_CTX_new without first initializing libssl properly. I pushed another commit that fixes this and ensures libssl is initialized.

[nguerrera]

I know it was already like this, but it's weird that we have a Interop/Unix/System.Net.Security/ rather than Interop/Unix/libssl. Since this will go away at some point soon with the shimming going quickly, it probably doesn't matter...

Beyond that, would it be better to have all of the SSL shims in an Interop.Ssl class? That would defer libssl initialization until it's definitely needed and also make it easier if we decide to split System.Security.Cryptography.Native along libcrypto/libssl boundaries. I'm finding this extra subtlety of initialization a little confusing to sort out.

[eerhardt]

Beyond that, would it be better to have all of the SSL shims in an Interop.Ssl class?

I have contemplated that, but wasn't sure it fit in with the Interop Guidelines.

•Declarations shouldn't be in Interop directly, but rather within a partial, static, internal nested type named for a given library or set of libraries, e.g.

I wasn't sure, so I kept it 'safe' and reused the Crypto class. I think I would prefer splitting the SSL methods out in a separate Interop.Ssl class, and keeping the native shims for interop.Ssl and Interop.Crypto contained in the System.Security.Cryptography.Native library. If people agree on that, I will make the change.

[stephentoub]

Preexisting your change, but especially now that we're using unsafe code, we should consider adding some Debug.Asserts here to validate that we're getting expected inputs for buffer/offset/count.

[eerhardt]

Do you mean:
Debug.Assert(offset + count <= buffer.Length);

Or are there more asserts you had in mind?

[stephentoub]

Yup, that's the core of what I had in mind, basically the same things that get validated for buffer/offset/count in Stream.Read/Write methods, e.g. https://github.com/dotnet/corefx/blob/master/src/System.IO.UnmanagedMemoryStream/src/System/IO/UnmanagedMemoryStream.cs#L434-L441

[stephentoub]

LGTM

[vijaykota]

LGTM for the additional changes