dependency-check / azuredevops

Dependency Check Azure DevOps Extension
Apache License 2.0
44 stars 26 forks source link

Add the NVD API Key #147

Closed rfennell closed 4 months ago

rfennell commented 7 months ago

Fixes #146

tlogik commented 7 months ago

@rfennell and @Saturate this is already supported if you use the addition argument. All you have to do is to check up on the CLI documentation for finegraiend control https://jeremylong.github.io/DependencyCheck/dependency-check-cli/arguments.html

ex. --nvdApiKey YOUR_API_KEY

I personally do not think exposing all fields thru the extension is a good idea, however the documentation could be more clear on how you can utilize the underlying CLI and pass params to it.

Personally i dont see this PR as proving much benefit but a PR with improved documentation would be awesome i think.

dependecycheck_apikey

rfennell commented 7 months ago

I agree in general not to add all CLI arguments as AZDO Task parameters.

However, given that --nvdApiKey is becoming in effect essential when using the 9.x.x version of the CLI tool

NVD API Key Highly Recommended With 9.0.0 dependency-check has moved from using the NVD data-feed to the NVD API. Users of dependency-check are highly encouraged to obtain an NVD API Key; see https://nvd.nist.gov/developers/request-an-api-key Without an NVD API Key dependency-check's updates will be extremely slow. Please see the documentation for the cli, maven, gradle, or ant integrations on how to set the NVD API key.

It should be exposed at the task parameter level.

pippolino commented 7 months ago

I agree in general not to add all CLI arguments as AZDO Task parameters.

However, given that --nvdApiKey is becoming in effect essential when using the 9.x.x version of the CLI tool

Hi @rfennell,

it doesn't mean that the --nvdApiKey parameter is always used. In my case I have a dedicated pipeline for updating the NVD-CVE database and I use the --noupdate parameter in this task.

I agree with @tlogik for a PR for improving the documentation on this.

Saturate commented 7 months ago

I agree in general not to add all CLI arguments as AZDO Task parameters. However, given that --nvdApiKey is becoming in effect essential when using the 9.x.x version of the CLI tool

Hi @rfennell,

it doesn't mean that the --nvdApiKey parameter is always used. In my case I have a dedicated pipeline for updating the NVD-CVE database and I use the --noupdate parameter in this task.

I agree with @tlogik for a PR for improving the documentation on this.

With the changes to the NVD, using the API KEY will be the main use case for this extension right? So I think adding this parameter would be pretty useful for the majority of users - as it mostly won't work without one.

I'm all for not adding everything, but the most used ones should be there.

jepp-igus commented 7 months ago

@tlogik I'm also using currently the "addtional parameter" field for adding the NVD API-Key. The API Key itself is stored as a secret, but it gets exposed in cleartext during the pipeline run... So I prefer an enhancement in exposing this parameter as a separate field and obfuscating the key in the logs.

@rfennell I'm new to Azure DevOps, do secrets needs to be obfuscated manually during the development?

rfennell commented 7 months ago

The way to obfuscate the API key in Azure DevOps is to save the key as a secret variable and then reference it by name in the pipeline in the from $(nvdapikey). This could be passed in via the existing additionalArguments or the proposed nvdapikey parameters.

- task: dependency-check-build-task@6
  displayName: "Vunerability Scan Exploited Vulnerabilities update check"
  inputs:
    projectName: 'Maintainance'
    scanPath: '.'
    format: 'HTML'
    additionalArguments: '--nvdApiKey $(nvdapikey)' 

If you pass in the key this way, Azure DevOps will automatically obfuscate the secret in it's logs as shown below

Invoking Dependency Check...
Path: D:\a\_tasks\dependency-check-build-task_47ea1f4a-57ba-414a-b12e-c44f42765e72\6.1.3\dependency-check\bin\dependency-check.bat
Arguments: --project "Maintainance" --out "D:\a\1\TestResults\dependency-check" --scan "D:\a\1\s" --format HTML --nvdApiKey ***
C:\Windows\system32\cmd.exe /D /S /C "D:\a\_tasks\dependency-check-build-task_47ea1f4a-57ba-414a-b12e-c44f42765e72\6.1.3\dependency-check\bin\dependency-check.bat --version"
Dependency-Check Core version 9.0.3

Searching for left over lock files...
found no left over lock files, continuing...
C:\Windows\system32\cmd.exe /D /S /C "D:\a\_tasks\dependency-check-build-task_47ea1f4a-57ba-414a-b12e-c44f42765e72\6.1.3\dependency-check\bin\dependency-check.bat --project Maintainance --out D:\a\1\TestResults\dependency-check --scan D:\a\1\s --format HTML --nvdApiKey ***"
jepp-igus commented 7 months ago

@rfennell Thanks for your quick response, but thats exactly what I'm doing and the api key is exposed in cleartext in the pipeline run.

The only difference is, that the secret is stored in an Azure keyvault and referenced as a secret in Azure DevOps -> Pipeline Library.

image

image

image

rfennell commented 7 months ago

Ah, seen that before, it looks to be difference in behavior between running on Windows (a .bat file) or Linux (a .sh file). Somewhere in the chains of execution the obfuscation gets lost when string are combined.

The problem line is dependency-check-build-task.ts#L158

We need to edit console.log(Arguments: ${args}); to do some string replacement of the --nvdapikey 1234 with --nvdapikey ****. I have to do similar on one of my other Azure DevOps tasks as it also shells out to command line tool and secret tracking gets lost.

I will add something to this PR

Saturate commented 4 months ago

Closing this as it will be solved with https://github.com/dependency-check/azuredevops/pull/155

Thanks for the PR!