Security Build Definition by ravimeda · Pull Request #20873 · dotnet/corefx

ravimeda · 2017-06-09T16:08:16Z

Introducing build definition that will run security tools required to comply with Security Development Lifecycle (SDL). The build definition puts together VSTS extensions that run the tools, analyze logs, and upload results to Trust Services Automation that creates workitems for identified security issues.
List of tools included in this definition are:

BinSkim - validates compiler/linker settings and other security-relevant binary characteristics. https://github.com/Microsoft/binskim
APIScan - determines whether or not the software complies with the API Usage Standard of the Interoperability Policy.
CredScan - index and scan for credentials or other sensitive content.
PoliCheck - scan code, code comments, and content for words that may be sensitive for legal, cultural, or geopolitical reasons.

Approach followed in this build is:

Get NuGet packages corresponding to the specified official build.
Extract packages to $(Build.SourcesDirectory)\security
Run BinSkim and APIScan on the extracted packages
Get sources corresponding to the official build. This means checkout at the SHA listed version.txt
Run CredScan and PoliCheck on the sources.

Running this build identified the following issues https://msazure.visualstudio.com/defaultcollection/One/_workitems?tempQueryId=f4c69d04-1a80-4fef-ad4b-8f0dd0a8af04

@morganbr @weshaggard @chcosta @dagood PTAL
Cc @gkhanna79

Edit 6/12: Described the approach for the build, and updated the query link.
Edit 6/12: Fixed type "Trust Services Automation"

chcosta · 2017-06-09T17:52:42Z

#Resolved

chcosta · 2017-06-09T17:54:17Z

+                "scriptName": "",
+                "arguments": "$(Build.SourcesDirectory) $(PB_CloudDropAccountName) $(PB_CloudDropAccessToken) $(PB_CloudDropContainer)",
+                "workingFolder": "$(Build.SourcesDirectory)",
+                "inlineScript": "param($SrcDir, $account, $token, $container)\n$syncCmd = Join-Path \"$SrcDir\" \"sync.cmd\"\n$p = Start-Process $syncCmd -ArgumentList \"-ab -- /p:CloudDropAccountName=$account /p:CloudDropAccessToken=$token /p:ContainerName=$container\" -PassThru -NoNewWindow -ErrorAction Stop -Wait -Verbose  \nWrite-Host \"Exit Code: $($p.ExitCode)\"\n\n\n",


There's a lot of inline scripts, any reason these can't be done with BuildTools? #Resolved

Please try this query - https://msazure.visualstudio.com/defaultcollection/One/_workitems?tempQueryId=f4c69d04-1a80-4fef-ad4b-8f0dd0a8af04 #Closed

There's a lot of inline scripts, any reason these can't be done with BuildTools?

This particular script is simply running sync.cmd, which is similar to how other build definitions do. But for other inline scripts such as extract packages, yes, having something in BuildTools such as an MSBuild target that can be used cross-plat is better choice. I can log an issue to track that work. What do you guys say? #Closed

Why do you need an inline script to run a command? Why not just run the command from a command window the same way a dev would? #Resolved

morganbr · 2017-06-09T22:05:55Z

@ravimeda this looks like a good start. Some of the BinSkim issues suggest you need to fix a couple things:

You need to ensure symbols for the product are on BinSkim's symbol path. Errors like bug 1229205 Missing Symbols for mscordaccore.dll prevent BinSkim from properly scanning those binaries.
Lots of dlls being scanned aren't part of the product. In particular, I see lots of errors for api-ms-*, which are part of the Windows SDK, not the product.

Once you have those fixed up, it'll be easier to see whether there are any other issues needing action.

What's your plan to suppress/baseline false positives, which there seem to be a lot of from CredScan and PoliCheck? #Resolved

weshaggard · 2017-06-12T20:24:37Z

+            "timeoutInMinutes": 0,
+            "condition": "succeeded()",
+            "task": {
+                "id": "3056813a-40e9-4b2f-8f6b-612d1bc4e045",


Do all these tools already have custom vsts tasks associated with them? #Resolved

Yes, each tool has a custom VSTS task referred to as Security Extension. Details at http://aka.ms/sdtvsts #Resolved

ravimeda · 2017-06-12T20:56:06Z

Thanks @morganbr Couple comments -

Updated the arguments for BinSkim and APIScan through commit 63cfeca
Verified that all references to api-ms-* are from the test CoreCLR run I did a few days ago. CoreFx packages do not contain any api-ms-*.
I'll do a couple CoreFx test builds to ensure there are no symbol-not-found errors.
I'm following up with TSA to clarify on how to suppress false positives.

@chcosta @dagood logged dotnet/buildtools#1565 to avoid custom PowerShell script to extract packages.
#Closed

morganbr · 2017-06-13T00:32:23Z

@ravimeda , sounds good. When you have new BinSkim results, let me know and I'll take a look. #Resolved

ravimeda · 2017-06-14T14:08:35Z

Cleared up the noise from test builds, and did a couple fresh builds. List of issues reported from these new builds is summarized in the table below:

Tool	Issues
BinSkim	0
APIScan	80
CredScan	24
PoliCheck	2972

Full list of issues is at https://msazure.visualstudio.com/defaultcollection/One/_workitems?tempQueryId=94620dd6-d32e-4f1e-b2bb-1cf748f7dd51

Next steps on how we want to proceed with security builds:

Build will be launched manually when required. There will not be any automatic triggers. This is because the input variable for the build is the name of the container from where the NuGet packages should be restored from.
Prepare security build for CoreCLR and Core-Setup using similar approach.
In near future, we will extend Maestro to determine the latest container name for a repo, and trigger the security build passing that container name.

Cc @dleeapho #Closed

* Remove folders other than security. * Update args for BinSkim. * Cleanup scripts. * Fix APIScan args. * Update task names. * Update PB variable name. * Do delete .txt files. Version.txt is needed. * Set symbolsFolder for APIScan. * Update git checkout task. * Add a test task that will be removed before merge. * Add PB_Git variable. * Update Git path. * Revert to PB_Git. * Remove test task.

gkhanna79 · 2017-06-14T14:27:15Z

Adding @wtgodbe to take a look as well since he has been involved in related security efforts.

* Create root and windows definition. * Include PB_ prefix. * Remove variables whose values will be in PB. * Remove unnecessary tasks. * Update path property. * Add OSX leg. * Remove triggers. * Remove PB variables. * Remove OSX leg since there is no TSA support. * Remove CodeMetrics. * Pass official build number as PB variable. * Invoke sync.cmd directly. * Update arguments passed to security tools. (dotnet/corefx#11) * Minor fixes to arguments passed, and variable names. (dotnet/corefx#12) * Remove folders other than security. * Update args for BinSkim. * Cleanup scripts. * Fix APIScan args. * Update task names. * Update PB variable name. * Do delete .txt files. Version.txt is needed. * Set symbolsFolder for APIScan. * Update git checkout task. * Add a test task that will be removed before merge. * Add PB_Git variable. * Update Git path. * Revert to PB_Git. * Remove test task. Commit migrated from dotnet/corefx@114dc02

ravimeda added 11 commits June 8, 2017 11:57

Create root and windows definition.

d6dc97a

Include PB_ prefix.

a5c0e8c

Remove variables whose values will be in PB.

7dfd7b9

Remove unnecessary tasks.

9e37df3

Update path property.

5af64ff

Add OSX leg.

178922b

Remove triggers.

b5f010b

Remove PB variables.

c0bce17

Remove OSX leg since there is no TSA support.

c27e391

Remove CodeMetrics.

20a66e9

Pass official build number as PB variable.

7aed957

dnfclas added the cla-already-signed label Jun 9, 2017

chcosta reviewed Jun 9, 2017

View reviewed changes

Invoke sync.cmd directly.

d3883d0

karelz added the area-Infrastructure-libraries label Jun 11, 2017

karelz assigned ravimeda Jun 11, 2017

karelz added this to the 2.1.0 milestone Jun 11, 2017

weshaggard reviewed Jun 12, 2017

View reviewed changes

Update arguments passed to security tools. (#11)

63cfeca

ravimeda mentioned this pull request Jun 14, 2017

Security Build Definition - CoreCLR dotnet/coreclr#12279

Merged

ravimeda merged commit 114dc02 into dotnet:master Jun 16, 2017

ravimeda mentioned this pull request Jun 22, 2017

Security Build Definition - Core-Setup dotnet/core-setup#2722

Merged

tannergooding mentioned this pull request Nov 9, 2018

Moving the Utf8Parser/Utf8Formatter to be shared #33348

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Security Build Definition#20873

Security Build Definition#20873
ravimeda merged 14 commits intodotnet:masterfrom
ravimeda:sec_ext

ravimeda commented Jun 9, 2017 •

edited

Loading

Uh oh!

chcosta commented Jun 9, 2017 •

edited by ravimeda

Loading

Uh oh!

chcosta Jun 9, 2017 •

edited by ravimeda

Loading

Uh oh!

ravimeda Jun 9, 2017 •

edited

Loading

Uh oh!

ravimeda Jun 9, 2017 •

edited

Loading

Uh oh!

chcosta Jun 9, 2017 •

edited by ravimeda

Loading

Uh oh!

morganbr commented Jun 9, 2017 •

edited by ravimeda

Loading

Uh oh!

weshaggard Jun 12, 2017 •

edited by ravimeda

Loading

Uh oh!

ravimeda Jun 12, 2017 •

edited

Loading

Uh oh!

ravimeda commented Jun 12, 2017 •

edited

Loading

Uh oh!

morganbr commented Jun 13, 2017 •

edited by ravimeda

Loading

Uh oh!

ravimeda commented Jun 14, 2017 •

edited

Loading

Uh oh!

gkhanna79 commented Jun 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

ravimeda commented Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chcosta commented Jun 9, 2017 • edited by ravimeda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chcosta Jun 9, 2017 • edited by ravimeda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravimeda Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravimeda Jun 9, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chcosta Jun 9, 2017 • edited by ravimeda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morganbr commented Jun 9, 2017 • edited by ravimeda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weshaggard Jun 12, 2017 • edited by ravimeda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravimeda Jun 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravimeda commented Jun 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

morganbr commented Jun 13, 2017 • edited by ravimeda Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ravimeda commented Jun 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gkhanna79 commented Jun 14, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ravimeda commented Jun 9, 2017 •

edited

Loading

chcosta commented Jun 9, 2017 •

edited by ravimeda

Loading

chcosta Jun 9, 2017 •

edited by ravimeda

Loading

ravimeda Jun 9, 2017 •

edited

Loading

ravimeda Jun 9, 2017 •

edited

Loading

chcosta Jun 9, 2017 •

edited by ravimeda

Loading

morganbr commented Jun 9, 2017 •

edited by ravimeda

Loading

weshaggard Jun 12, 2017 •

edited by ravimeda

Loading

ravimeda Jun 12, 2017 •

edited

Loading

ravimeda commented Jun 12, 2017 •

edited

Loading

morganbr commented Jun 13, 2017 •

edited by ravimeda

Loading

ravimeda commented Jun 14, 2017 •

edited

Loading