Incorporate PR Feedback - Code Cleanup in NegotiateStream

[kapilash]

The following comments were made in dotnet/corefx#6469 and need to be incorporated into negotiatestream

1. One to one mapping with native calls and Interop.*.cs files
This is a lot of code being added to the Interop.*.cs files. Typically we've kept these as close to 1:1 with the underlying operation being wrapped as possible, typiclaly just the P/Invoke signature or maybe a 1-2 line wrapper if necessary, and then all of this additional logic ends up living in the consuming code.

2. Split up safefreecredentials
As post-merge cleanup this file should probably be split up (now that this base class applies to two different libraries' interop). And since libssl is the last lib* directory left in Interop, that could be cleaned up at the same time.

3. Fix up boiler plate code in KerberosTest.cs
I honestly can't figure out how each of these tests is different... it'd be great to factor out boilerplate and leave each test showing the kernel of what it's testing. (fixed in dotnet/corefx#7317)

4. Non-SafeHandle SafeHandles to be refactored out
Some of the safehandles in PAL are not really safe handles. These need to reworked.

Not today, but one day it would give me a warm and fuzzy feeling if the PAL was reworked to not require non-SafeHandle SafeHandles be created.

5. A compile time constant for the size of OID lengths

The following code has /magic/ value of 6.

  gss_OID_desc gss_mech_spnego_OID_desc = {.length = 6, .elements = static_cast<void*>(gss_spnego_oid_value)};

6. Debug only constructor with an assert for size (fixed in dotnet/corefx#7317)

private const int StatusDictionarySize = 39;

#if DEBUG
static SecurityStatusAdapterPal()
{
   Debug.Assert(s_statusDictionary.Count == StatusDictionarySize, $"Expected size       {StatusDictionarySize}, got size {s_statusDictionary.Count}");
 }
  #endif

   private static readonly ... = new BidirectionDictionary<...>(StatusDictionarySize);

[kapilash]

cc @stephentoub @bartonjs

[kapilash]

Adding some more points:

7. Why manual skip instead of compilation flag
Why are all of these tests using a manual skip instead of [ConditionalFact]?

8. Task array initialization without sizes dotnet/corefx#7317 fixed this
Task arrays (in multiple tests) can be initialized via
Task[] auth =
{
client.AuthenticateAsClientAsync(credential, target),
server.AuthenticateAsServerAsync(),
};

9 Need for overwriting values
We just called an interop method that built outputBuffer; why do we need to overwrite values here?

10 Possible Missing asserts on the parameters
We need to look into the case where some of parameters' values might not be null

11 Unncessary blank lines (fixed in dotnet/corefx#7317)

12 [Possible fix CMake error for message (https://github.com/dotnet/corefx/pull/6469#discussion_r56658286) (fixed in dotnet/corefx#7317)

13 Look into Asserts for NTLM case

14 Move the gss_mech_spnego_OID_desc to beside other constant (fixed in dotnet/corefx#7317)

15 Is it /bin/sh or bash
When invoking the script from managed code, do we need to use bin/sh or bash

[kapilash]

dotnet/corefx#7317 fixes the following:

3. Fix up boiler plate code in KerberosTest.cs
6. Debug only constructor with an assert for size
8. Task array initialization without sizes
11. Unncessary blank lines
12. Possible fix CMake error for message
14 Move the gss_mech_spnego_OID_desc to beside other constant

[kapilash]

Moving 2 and 4 to separate issue: dotnet/corefx#8115

[kapilash]

No 13, Look into Asserts for NTLM case was fixed via dotnet/corefx#7647

[kapilash]

Closing 15 Is it /bin/sh or bash.

It should be /bin/bash`` and not/bin/sh. The latter is , usually, a symbolic link pointing to whatever is the system shell. Using/bin/bash``` is the safer choice allowing us to use bash specific features in the script.

[kapilash]

Moving Why manual skip instead of compilation flag to dotnet/corefx#8117

[kapilash]

dotnet/corefx#8118 fixed the open items in this issue - namely:

1. One to one mapping with native calls and Interop.*.cs files

5. A compile time constant for the size of OID lengths

Closing this issue.