Rewrite convertfrom sddlstring in csharp #3936

zacateras · 2017-06-05T18:17:53Z

zacateras · 2017-06-05T18:22:21Z

src/System.Management.Automation/System.Management.Automation.csproj

@@ -25,6 +25,7 @@
    <PackageReference Include="System.Security.AccessControl" Version="4.4.0-preview1-25302-01" />
    <PackageReference Include="System.Security.Cryptography.Pkcs" Version="4.4.0-preview1-25302-01" />
    <PackageReference Include="System.Text.Encoding.CodePages" Version="4.4.0-preview1-25302-01" />
+    <PackageReference Include="System.Threading.AccessControl" Version="4.4.0-preview1-25302-01" />


this is required by types moved from System.Security.AccessControl to System.Threading.AccessControl (System.Security.AccessControl.MutexRights), am I allowed to add new references here ?

It looks to me the package System.Threading.AccessControl is required by Microsoft.PowerShell.Commands.Utility instead of System.Management.Automation. If that's the case, this PackageReference entry should be added to Microosft.PowerShell.Commands.Utility instead. And please remove the same entry from Microsoft.PowerShell.SDK.csproj.

daxian-dbw · 2017-06-06T05:17:29Z

@iSazonov if you have time, could you please review this PR?

iSazonov

Leave a comment

iSazonov · 2017-06-06T09:13:37Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+        [ValidateSet(
+            "FileSystemRights", "RegistryRights", "ActiveDirectoryRights",
+            "MutexRights", "SemaphoreRights", "CryptoKeyRights",
+            "EventWaitHandleRights")]


Please create C# constants for this strings because they're using in other places.

used custom enum instead

iSazonov · 2017-06-06T09:25:28Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+        protected override void BeginProcessing()
+        {
+#if CORECLR
+            if (Type == "CryptoKeyRights" || Type == "ActiveDirectoryRights")


Now ActiveDirectoryRights is in .Net Core 2.0 https://apisof.net/catalog/System.DirectoryServices.ActiveDirectoryRights

thanks, done

iSazonov · 2017-06-06T09:40:42Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+            {
+                return sid.Translate(typeof(NTAccount)).ToString();
+            }
+            catch {}


I believe we should write an error for user.

iSazonov · 2017-06-06T09:41:30Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+                { "FileSystemRights", typeof(System.Security.AccessControl.FileSystemRights) },
+                { "RegistryRights", typeof(System.Security.AccessControl.RegistryRights) },
+#if !CORECLR
+                { "ActiveDirectoryRights", typeof(System.DirectoryServices.ActiveDirectoryRights) },


See my comment above about "ActiveDirectoryRights".

iSazonov · 2017-06-06T10:41:03Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+        /// </summary>
+        private List<string> GetAccessRights(long accessMask, string type)
+        {
+            IDictionary<string, Type> rightTypes = new Dictionary<string, Type>


We should use OrderedDictionary.
rightTypes is good candidate for static cache.

added static cache field

iSazonov · 2017-06-06T11:03:12Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+            {
+                foreach (var accessFlag in Enum.GetNames(rightType))
+                {
+                    long longKeyValue = (long)Enum.Parse(rightType, accessFlag);


Oh, we re-evaluate Enum.GetNames and Enum.Parse again and again. We should refactor "GetAccessRights" to use cache(s).

We could write a script to generate C# and avoid reflection entirely.

@lzybkr what kind of code generation script do you mean? t4 template?

No, I just meant a PowerShell script that generates switch statements or something like that.

added ps1 generator

@lzybkr Will this require recompilation if there will are changes in CoreFX? We could initilize this in static cache on Begin step.
It seems so rare that the use of generation seems to be an unnecessary complication.

iSazonov · 2017-06-06T11:09:46Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+
+                if (ace is QualifiedAce qualifiedAce)
+                {
+                    aceString += qualifiedAce.AceQualifier;


We should use StringBuilder because aceString is changed multiple times.

iSazonov · 2017-06-06T11:10:04Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+            return aceStrings;
+        }
+    }
+}


Please add Newline.

iSazonov · 2017-06-06T11:15:20Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/UtilityCommon.cs

+        /// <summary>
+        /// '{0}' is not supported in this system.
+        /// </summary>
+        public static string TypeNotSupported { get { return UtilityCommonStrings.TypeNotSupported; } }


It seems we can directly use UtilityCommonStrings.TypeNotSupported

removed obsolete property

iSazonov · 2017-06-06T11:53:48Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+
+            // Go through the entries in the different right types, and see if they apply to the
+            // provided access mask. If they do, then add that to the result.
+            foreach (var rightType in typesToExamine)


@SteveL-MSFT @daxian-dbw We should make a conclusion about the algorithm. SDDL is always "object-specific" but we go through all "types". I believe that's a mistake. An user always know the object type (where he did get the SDDL), should set it explicit and the cmdlet must decode only against this type.

I don't understand this domain enough to be confident here - but in principle I fully agree with @iSazonov - the current algorithm is strange.

@lzybkr Could you please ping an expert in this domain?

The documentation for SDDL looks pretty good, I think if we have concerns about this algorithm, we can answer them ourselves, but I don't have time for that now.

I re-read the document and made any tests - My conclusion is full discard the algorithm:

From the docs:

Each type of securable object defines its own set of specific access rights and its own mapping of generic access rights. For information about the specific and generic access rights for each type of securable object, see the overview for that type of object.

the conclusion is an SDDL has no meaning without object entity - the cmdlet should get the object type as argument.

The cmdlet calls [Security.AccessControl.CommonSecurityDescriptor]::new($false,$false,$Sddl)
The constructor in Core FX

I tested the cmdlet with (1) [Security.AccessControl.CommonSecurityDescriptor]::new($false,$false,$Sddl) and (2) [Security.AccessControl.CommonSecurityDescriptor]::new($true,$false,$Sddl) for path "c:\test" (it is container) and got different results.

the conclusion is the cmdlet should take into account isContainer (folders/containers) and isDS (directory objects and containers).

CoreFX can throw exceptions (ArgumentException) - maybe the cmdlet should catch them and re-throw (put CoreFX exception in InnerException)

The cmdlet should have strongly typed output. In current PSCustomObject we should add "Type".

"Type" - we can define it as Enum ( remove unsupported member by #if !Unix ) and exclude ValidateSet at all.

We should add tests for all object types.

@iSazonov I also have doubts about current method of converting ACE AccessMask into access flag enum based strings. There are situations when flag cannot be converted without additional logic i.e. FileSystemRights contains entries with duplicate flag values - some applies only for files, other only for containers. I guess that this won't be solved by requiring Type parameter.

@zacateras Please see (2) point from my conclusion.

•the conclusion is the cmdlet should take into account isContainer (folders/containers) and isDS (directory objects and containers).

@iSazonov Apart from fixing CommonSecurityDescriptor constructor call (by requiring isContainer and isDS flags) we should also take care about correct parsing of AccessMasks to enum names. FileSystemRights and ActiveDirectoryRights contain several names that use the same value, eg. FileSystemRights contain AppendData & CreateDirectories which have the same value (4). In consequence Directories with access flag (4) can get AppendData right, what is incorrect.

As a solution we can add custom enums (or at least custom mapping dictionaries) containing distinct subsets of allowed values - one for containers (ContainerSystemRights) and one for other objects (FileSystemRights). Alternatively we can discard CommonSecurityDescriptor/Right-Enum based parsing algorithm entirely and try to implement text parser using sddl docs. What do you think ?

iSazonov · 2017-06-06T12:04:26Z

@zacateras Thanks for your work! I have question about the algorithm that I addressed to PowerShell team - please wait the conclusion before continue.

Also we haven't tests for the cmdlet - you should add its and pass CI before we can merge.

lzybkr

Thanks for the contribution - I'll be very happy to see this one merged.

lzybkr · 2017-06-07T23:41:55Z

src/Modules/Shared/Microsoft.PowerShell.Utility/Microsoft.PowerShell.Utility.psm1

@@ -1,174 +0,0 @@
-


I think you can git rm this file now.

this file name is referenced 3 times:

/src/Modules/map.json (remove entry?)

/src/System.Management.Automation/engine/InitialSessionState.cs (replace with some other build result file?)

/src/System.Management.Automation/security/Authenticode.cs (replace with some other build result file?)

Thanks for catching that.
Remove the entry in map.json - that's perfectly safe.
I think you can change the other two references to Microsoft.PowerShell.Utility.psd1.

lzybkr · 2017-06-07T23:47:42Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+        protected override void BeginProcessing()
+        {
+#if CORECLR
+            if (Type == "CryptoKeyRights" || Type == "ActiveDirectoryRights")


ValidateSet is case insensitive, so we would need a case insensitive comparison here too.

That said, maybe it's better to change the parameter type to an enum.

I should add - we don't need this code at all - the ValidateSet could simply be different under CORECLR.

The PowerShell version of this command necessarily had runtime code to cover this case, but when we are building different binaries, we can do better.

removed ValidateSet, used custom enum instead

lzybkr · 2017-06-07T23:52:15Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+
+            PSObject result = new PSObject();
+
+            result.Properties.Add(new PSNoteProperty("Owner", owner));


Do we want strongly typed output instead?

Following up on my own question and after playing around with this cmdlet a little, we definitely want strongly typed output.

lzybkr · 2017-06-08T00:10:00Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+            {
+                foreach (var accessFlag in Enum.GetNames(rightType))
+                {
+                    long longKeyValue = (long)Enum.Parse(rightType, accessFlag);


We could write a script to generate C# and avoid reflection entirely.

lzybkr · 2017-06-08T00:11:35Z

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ConvertFrom-SddlString.cs

+
+            // Go through the entries in the different right types, and see if they apply to the
+            // provided access mask. If they do, then add that to the result.
+            foreach (var rightType in typesToExamine)


I don't understand this domain enough to be confident here - but in principle I fully agree with @iSazonov - the current algorithm is strange.

SteveL-MSFT

Can you add some tests? I just cloned this branch and a basic attempt to use this cmdlet failed:

PS C:\users\slee\repos\zacateras> $sddl = "O:BAG:BAD:(A;CI;CCDCLCSWRPWPRCWD;;;BA)(A;CI;CCDCRP;;;NS)(A;CI;CCDCRP;;;LS)(A;
CI;CCDCRP;;;AU)"
PS C:\users\slee\repos\zacateras> ConvertFrom-SddlString -Sddl $sddl
ConvertFrom-SddlString : Unable to cast object of type 'System.Security.AccessControl.FileSystemRights' to type
'System.Int64'.
At line:1 char:1
+ ConvertFrom-SddlString -Sddl $sddl
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : NotSpecified: (:) [ConvertFrom-SddlString], InvalidCastException
    + FullyQualifiedErrorId : System.InvalidCastException,Microsoft.PowerShell.Commands.ConvertFromSddlStringCommand

Which worked with the old script cmdlet:

PS C:\users\slee\repos\PowerShell> $sddl = "O:BAG:BAD:(A;CI;CCDCLCSWRPWPRCWD;;;BA)(A;CI;CCDCRP;;;NS)(A;CI;CCDCRP;;;LS)(A
;CI;CCDCRP;;;AU)"
PS C:\users\slee\repos\PowerShell> ConvertFrom-SddlString -Sddl $sddl


Owner            : BUILTIN\Administrators
Group            : BUILTIN\Administrators
DiscretionaryAcl : {NT AUTHORITY\Authenticated Users: AccessAllowed (ListDirectory, WriteData,
                   WriteExtendedAttributes), NT AUTHORITY\LOCAL SERVICE: AccessAllowed (ListDirectory, WriteData,
                   WriteExtendedAttributes), NT AUTHORITY\NETWORK SERVICE: AccessAllowed (ListDirectory, WriteData,
                   WriteExtendedAttributes), BUILTIN\Administrators: AccessAllowed (ChangePermissions,
                   CreateDirectories, ExecuteKey, ListDirectory, ReadExtendedAttributes, ReadPermissions, Traverse,
                   WriteData, WriteExtendedAttributes, WriteKey)}
SystemAcl        : {}
RawDescriptor    : System.Security.AccessControl.CommonSecurityDescriptor

iSazonov · 2017-07-19T17:49:27Z

@SteveL-MSFT The algorithm is broken! Please see my comment above #3936 (comment) I believe we cannot merge the PR.

SteveL-MSFT

Reviewing algorithm

iSazonov · 2017-07-31T14:26:24Z

@daxian-dbw Can we use System.DirectoryService or before we should move to 4.5.0 version ?

iSazonov · 2017-08-03T12:11:48Z

@zacateras Do you plan to continue?

zacateras · 2017-08-03T23:33:09Z

@iSazonov yep. I thought that we are waiting for some external review.

SteveL-MSFT · 2017-08-04T01:19:40Z

Now that Beta.5 is out, I'll spend time on this tomorrow

iSazonov · 2017-08-04T03:35:50Z

We should follow MSDN docs - see my comment above.

iSazonov · 2017-08-24T10:19:33Z

@SteveL-MSFT Could you please continue?

SteveL-MSFT · 2017-08-29T00:15:33Z

...rShell.Commands.Utility/commands/utility/ConvertFromSddlStringCommand.RightType.Generate.ps1

@@ -0,0 +1,119 @@
+$RightTypes = @(


Was this file meant to be checked in? It's not part of the build to run and the resulting source doesn't indicate it shouldn't be edited.

SteveL-MSFT · 2017-08-29T00:16:22Z

...osoft.PowerShell.Commands.Utility/commands/utility/ConvertFromSddlStringCommand.RightType.cs

+            /// </summary>
+            SemaphoreRights,
+
+#if !CORECLR


since we're officially removing fullclr support, we should not include this

SteveL-MSFT · 2017-08-29T00:16:38Z

...osoft.PowerShell.Commands.Utility/commands/utility/ConvertFromSddlStringCommand.RightType.cs

+                        { 2031619, "FullControl" },
+                    }
+                },
+#if !CORECLR


since we're officially removing fullclr support, we should not include this

SteveL-MSFT · 2017-08-29T00:27:13Z

test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-SddlString.Tests.ps1

@@ -0,0 +1,21 @@
+Describe "ConvertFrom-SddlString" -Tags "CI" {
+    It "Should convert without type" -Skip:($IsLinux -Or $IsOSX) {


Since this cmdlet is not supported on non-Windows, you should skip them in bulk using this pattern https://github.com/PowerShell/PowerShell/blob/master/docs/testing-guidelines/WritingPesterTests.md#skipping-tests-in-bulk (that way if additional tests get added they get auto-skipped)

SteveL-MSFT · 2017-08-29T00:28:03Z

test/powershell/Modules/Microsoft.PowerShell.Utility/ConvertFrom-SddlString.Tests.ps1

+        $SddlStringInfo.Group | Should Be "BUILTIN\Administrators"
+        $SddlStringInfo.DiscretionaryAcl.Length | Should BeGreaterThan 0
+        $SddlStringInfo.SystemAcl.Length | Should Be $null
+        $SddlStringInfo.RawDescriptor | Should BeOfType [System.Security.AccessControl.CommonSecurityDescriptor]


Should be some tests for -Type parameter

SteveL-MSFT · 2017-08-29T00:40:06Z

src/Microsoft.PowerShell.Commands.Utility/Microsoft.PowerShell.Commands.Utility.csproj

@@ -80,6 +80,8 @@

  <ItemGroup>
    <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="2.2.0" />
+    <PackageReference Include="System.DirectoryServices" Version="4.0.0-preview3-25423-02" />


This doesn't appear to be a supported package, perhaps we should not support AD rights for now.

Official package is System.DirectoryService. It is 4.5.0 version. @daxian-dbw said that we should review the API before use.

SteveL-MSFT · 2017-08-29T00:51:44Z

Sorry this took so long, got randomized by other things. I also compared the result with

ConvertFrom-SddlString "D:(A;;FA;;;SY)(A;;FA;;;BA)"

# old script version
DiscretionaryAcl : {NT AUTHORITY\SYSTEM: AccessAllowed (ChangePermissions, CreateDirectories, Delete, DeleteSubdirectoriesAndFiles, ExecuteKey, FullControl,
                   FullControl, FullControl, FullControl, ListDirectory, Modify, Read, ReadAndExecute, ReadAttributes, ReadExtendedAttributes,
                   ReadPermissions, Synchronize, TakeOwnership, Traverse, Write, WriteAttributes, WriteData, WriteExtendedAttributes, WriteKey),
                   BUILTIN\Administrators: AccessAllowed (ChangePermissions, CreateDirectories, Delete, DeleteSubdirectoriesAndFiles, ExecuteKey, FullControl,
                   FullControl, FullControl, FullControl, ListDirectory, Modify, Read, ReadAndExecute, ReadAttributes, ReadExtendedAttributes,
                   ReadPermissions, Synchronize, TakeOwnership, Traverse, Write, WriteAttributes, WriteData, WriteExtendedAttributes, WriteKey)}

# c# version
DiscretionaryAcl : {NT AUTHORITY\SYSTEM: AccessAllowed (AppendData, ChangePermissions, CreateFiles, Delete, DeleteSubdirectoriesAndFiles, ExecuteFile,
                   FullControl, FullControl, FullControl, FullControl, Modify, Read, ReadAndExecute, ReadAttributes, ReadData, ReadExtendedAttributes,
                   ReadKey, ReadPermissions, Synchronize, TakeOwnership, Write, WriteAttributes, WriteExtendedAttributes, WriteKey), BUILTIN\Administrators:
                   AccessAllowed (AppendData, ChangePermissions, CreateFiles, Delete, DeleteSubdirectoriesAndFiles, ExecuteFile, FullControl, FullControl,
                   FullControl, FullControl, Modify, Read, ReadAndExecute, ReadAttributes, ReadData, ReadExtendedAttributes, ReadKey, ReadPermissions,
                   Synchronize, TakeOwnership, Write, WriteAttributes, WriteExtendedAttributes, WriteKey)}

And the newer version has more permissions (although not sure if the old one was wrong).

iSazonov · 2017-08-29T06:02:59Z

@SteveL-MSFT Did you look my comment above? Main is "SDDL has no meaning without object entity - the cmdlet should get the object type as argument."

SteveL-MSFT · 2017-08-29T15:57:30Z

@iSazonov Seems like if you have the object, you would be using Get-Acl. ConvertFrom-SDDLString is more likely used when you have just the SDDL and just want to understand (perhaps roughly) what it represents. Not sure how useful this is if you require the object.

iSazonov · 2017-08-29T17:28:53Z

@SteveL-MSFT My idea is that we should make -Type mandatory.

zacateras · 2017-08-29T21:24:20Z

@iSazonov If AccessMask is presented using well known aliases then it can be parsed entirely without the knowledge of securable object type (Type parameter).

In case of numerical AccessMasks (eg. 0x02142512) we can translate only higher 16-32bits if the Type is not provided (lower bits can remain unparsed as a reminder). If the Type parameter is present then specific rights scheme could be used - according to constant definitions in winnt.h. Also more securable object types can be considered - according to the list in docs.

I started an implementation of SDDL parser using the documentation. The code can be found here (with examples here). If we want to use parts of the implemented approach then I can integrate its simplified form into the command.

What do you think about such solution?

iSazonov · 2017-09-04T12:46:41Z

@zacateras I still don't understand the need for this heuristic. For which scenarios is it addressed? As for me it looks like a trasformation enum1 -> int -> enum2 - what's confusing.

daxian-dbw · 2018-03-12T17:39:59Z

This PR has been left unattended for a long time. I will close this PR for now.
@zacateras Please feel free to reopen it if you decide to continue on it at a later time.

msftclas added the cla-required label Jun 5, 2017

zacateras commented Jun 5, 2017

View reviewed changes

msftclas added cla-signed and removed cla-required labels Jun 5, 2017

daxian-dbw requested a review from iSazonov June 6, 2017 05:12

iSazonov reviewed Jun 6, 2017

View reviewed changes

mirichmo assigned lzybkr Jun 7, 2017

lzybkr reviewed Jun 8, 2017

View reviewed changes

SteveL-MSFT requested changes Jun 8, 2017

View reviewed changes

SteveL-MSFT requested a review from LeeHolmes June 8, 2017 17:43

zacateras force-pushed the rewrite-convertfrom-sddlstring-in-csharp branch 4 times, most recently from 76608a2 to 51f3466 Compare June 16, 2017 10:03

SteveL-MSFT approved these changes Jul 19, 2017

View reviewed changes

SteveL-MSFT requested changes Jul 20, 2017

View reviewed changes

zacateras force-pushed the rewrite-convertfrom-sddlstring-in-csharp branch 2 times, most recently from cbb1c14 to 0fb99ce Compare August 3, 2017 23:07

Add ConvertFrom-SddlString cmdlet

3a8d3de

zacateras force-pushed the rewrite-convertfrom-sddlstring-in-csharp branch from 0fb99ce to 3a8d3de Compare August 3, 2017 23:23

SteveL-MSFT requested changes Aug 29, 2017

View reviewed changes

msftgits removed the cla-signed label Sep 26, 2017

PowerShell deleted a comment from msftclas Sep 26, 2017

lzybkr removed their assignment Nov 13, 2017

daxian-dbw closed this Mar 12, 2018

iSazonov mentioned this pull request Nov 27, 2018

Improve powershell startup time #8341

Merged

11 tasks

daxian-dbw mentioned this pull request Nov 30, 2018

Revisit the algorithm used in 'ConvertFrom-SddlString' #8365

Closed


		PSObject result = new PSObject();

		result.Properties.Add(new PSNoteProperty("Owner", owner));

		@@ -0,0 +1,21 @@
		Describe "ConvertFrom-SddlString" -Tags "CI" {
		It "Should convert without type" -Skip:($IsLinux -Or $IsOSX) {

Rewrite convertfrom sddlstring in csharp #3936

Rewrite convertfrom sddlstring in csharp #3936

Conversation

zacateras commented Jun 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daxian-dbw commented Jun 6, 2017

iSazonov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iSazonov Jun 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iSazonov commented Jun 6, 2017

lzybkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zacateras Jun 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zacateras Jun 16, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveL-MSFT left a comment • edited

Choose a reason for hiding this comment

iSazonov commented Jul 19, 2017

SteveL-MSFT left a comment

Choose a reason for hiding this comment

iSazonov commented Jul 31, 2017

iSazonov commented Aug 3, 2017

zacateras commented Aug 3, 2017

SteveL-MSFT commented Aug 4, 2017

iSazonov commented Aug 4, 2017

iSazonov commented Aug 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SteveL-MSFT commented Aug 29, 2017 • edited

iSazonov commented Aug 29, 2017

SteveL-MSFT commented Aug 29, 2017

iSazonov commented Aug 29, 2017

zacateras commented Aug 29, 2017 • edited

iSazonov commented Sep 4, 2017

iSazonov Jun 16, 2017 •

edited

zacateras Jun 8, 2017 •

edited

zacateras Jun 16, 2017 •

edited

SteveL-MSFT left a comment •

edited

SteveL-MSFT commented Aug 29, 2017 •

edited

zacateras commented Aug 29, 2017 •

edited