Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include latest v4 vulnerability #1400

Merged
merged 6 commits into from
Feb 21, 2024
Merged

Include latest v4 vulnerability #1400

merged 6 commits into from
Feb 21, 2024

Conversation

daynewlee
Copy link
Contributor

@daynewlee daynewlee commented Feb 7, 2024

Generate scanner-vuln-update including v4 vulnerability of latest version
Currently I can only verify scanner v2 work with new scanner-vuln-updates.zip, more V4 related tests will be done in ROX-21981

Test

  1. Make sure the zip is containing scanner v4 stuff:
ls -lh scanner-vuln-updates.zip 
-rw-r--r-- 1 yili staff 272M Feb 14 15:04 scanner-vuln-updates.zip
unzip -l scanner-vuln-updates.zip
Archive:  scanner-vuln-updates.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
140154404  02-14-2024 11:41   scanner-defs.zip
   248088  02-14-2024 11:41   k8s-istio.zip
144211510  02-14-2024 11:41   scanner-v4-defs-4.3.zip
---------                     -------
284614002                     3 files
  1. Upload to Central:
yli3-mac:stackrox yili$ curl --insecure -u admin:[PWD] -F "[email protected]" https://localhost:8000/api/extensions/scannerdefinitions -v
*   Trying [::1]:8000...
* connect to ::1 port 8000 failed: Connection refused
*   Trying 127.0.0.1:8000...
* Connected to localhost (127.0.0.1) port 8000
* ALPN: curl offers h2,http/1.1
* (304) (OUT), TLS handshake, Client hello (1):
* (304) (IN), TLS handshake, Server hello (2):
* (304) (IN), TLS handshake, Unknown (8):
* (304) (IN), TLS handshake, Request CERT (13):
* (304) (IN), TLS handshake, Certificate (11):
* (304) (IN), TLS handshake, CERT verify (15):
* (304) (IN), TLS handshake, Finished (20):
* (304) (OUT), TLS handshake, Certificate (11):
* (304) (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / AEAD-AES128-GCM-SHA256
* ALPN: server accepted h2
* Server certificate:
*  subject: OU=CENTRAL_SERVICE; CN=CENTRAL_SERVICE: Central; serialNumber=16561514245731368659
*  start date: Feb 14 20:08:00 2024 GMT
*  expire date: Feb 13 21:08:00 2025 GMT
*  issuer: CN=StackRox Certificate Authority; serialNumber=1018846594209314016
*  SSL certificate verify result: unable to get local issuer certificate (20), continuing anyway.
* using HTTP/2
* Server auth using Basic with user 'admin'
* [HTTP/2] [1] OPENED stream for https://localhost:8000/api/extensions/scannerdefinitions
* [HTTP/2] [1] [:method: POST]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: localhost:8000]
* [HTTP/2] [1] [:path: /api/extensions/scannerdefinitions]
* [HTTP/2] [1] [authorization: Basic YWRtaW46SzNEWGJmZ3NlNVR0UzQ3c01ibG5PZTBqVQ==]
* [HTTP/2] [1] [user-agent: curl/8.4.0]
* [HTTP/2] [1] [accept: */*]
* [HTTP/2] [1] [content-length: 284614740]
* [HTTP/2] [1] [content-type: multipart/form-data; boundary=------------------------GlBMII0WAr7HTBb57GfFwh]
> POST /api/extensions/scannerdefinitions HTTP/2
> Host: localhost:8000
> Authorization: Basic YWRtaW46SzNEWGJmZ3NlNVR0UzQ3c01ibG5PZTBqVQ==
> User-Agent: curl/8.4.0
> Accept: */*
> Content-Length: 284614740
> Content-Type: multipart/form-data; boundary=------------------------GlBMII0WAr7HTBb57GfFwh
> 
* We are completely uploaded and fine
< HTTP/2 200 
< vary: Accept-Encoding
< content-type: text/plain; charset=utf-8
< content-length: 57
< date: Wed, 14 Feb 2024 21:19:38 GMT
< 
* Connection #0 to host localhost left intact
Successfully stored the offline vulnerability definitions

@daynewlee daynewlee marked this pull request as draft February 8, 2024 02:49
@daynewlee daynewlee added generate-dumps-on-pr Generates the image based on dumps from the PR and removed generate-dumps-on-pr Generates the image based on dumps from the PR labels Feb 8, 2024
@daynewlee daynewlee marked this pull request as ready for review February 8, 2024 13:41
@daynewlee daynewlee changed the title Start v4 dump job Include latest v4 vulnerability Feb 8, 2024
scripts/ci/jobs/diff-dumps.sh Outdated Show resolved Hide resolved
@daynewlee daynewlee requested a review from jvdm February 8, 2024 21:16
Copy link
Contributor

@dcaravel dcaravel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to be careful here to not 'break' the current latest bundle if this job runs and there is a bug.

I'd expect the output of this job to include updates to all the versioned v4 offline bundles and the latest offline bundle, would expect the following to be uploaded:

scanner-support-public/offline/v1/scanner-vuln-updates.zip
scanner-support-public/offline/v1/4.4/scanner-vulns-4.4.zip

As of this initial release would expect scanner-vuln-updates.zipto be updated and to be created/updated 4.4/scanner-vulns-4.4.zip and both contain the same data (will diverge in the future as more versioned bundles are created)

@daynewlee
Copy link
Contributor Author

daynewlee commented Feb 8, 2024

Need to be careful here to not 'break' the current latest bundle if this job runs and there is a bug.

I'd expect the output of this job to include updates to all the versioned v4 offline bundles and latest offline bundles, would expect the following to be uploaded:


scanner-support-public/offline/v1/scanner-vuln-updates.zip

scanner-support-public/offline/v1/4.4/scanner-vulns-4.4.zip

As of this initial release would expect scanner-vuln-updates.zip and 4.4/scanner-vulns-4.4.zip to contain the same data

I haven't decided how to generate scanner-vuln-[version].zip because I am hesitating to use loop to iterate all scanner-defs-[version].zip in this script at this moment as you said that might introduce bugs to this workflow. Since scanner-vuln-update.zip only contains latest v4 defs, so it makes sense to change this bash function. I'll make another PR for the v4 offline bundles.

@daynewlee daynewlee requested a review from dcaravel February 9, 2024 00:35
@dcaravel
Copy link
Contributor

dcaravel commented Feb 9, 2024

I haven't decided how to generate scanner-vuln-[version].zip because I am hesitating to use loop to iterate all scanner-defs-[version].zip in this script at this moment as you said that might introduce bugs to this workflow. Since scanner-vuln-update.zip only contains latest v4 defs, so it makes sense to change this bash function. I'll make another PR for the v4 offline bundles.

OK, we may be introducing more variance between latest and the versioned bundles by separating these workflows (since the versioned bundles need the Scanner V2 data produced by this job). If the jobs are separated it will be possible that (for example) scanner-defs-4.4.zip added to scanner-vuln-updates.zip will be different from scanner-defs-4.4.zip in 4.4/scanner-vulns-4.4.zip depending on when jobs complete. One may have newer/older data then the other, but be expected to be the same.

FWIW, there are ways to have functions / commands in bash fail without breaking the whole script (if you change your mind).

@daynewlee
Copy link
Contributor Author

daynewlee commented Feb 9, 2024

I haven't decided how to generate scanner-vuln-[version].zip because I am hesitating to use loop to iterate all scanner-defs-[version].zip in this script at this moment as you said that might introduce bugs to this workflow. Since scanner-vuln-update.zip only contains latest v4 defs, so it makes sense to change this bash function. I'll make another PR for the v4 offline bundles.

OK, we may be introducing more variance between latest and the versioned bundles by separating these workflows (since the versioned bundles need the Scanner V2 data produced by this job). If the jobs are separated it will be possible that (for example) scanner-defs-4.4.zip added to scanner-vuln-updates.zip will be different from scanner-defs-4.4.zip in 4.4/scanner-vulns-4.4.zip depending on when jobs complete. One may have newer/older data then the other, but be expected to be the same.

FWIW, there are ways to have functions / commands in bash fail without breaking the whole script (if you change your mind).

Yeah, that's true. I will take that into consideration but possibly build on top of this PR. Do you think this PR/change make sense?
Actually I will probably add generate and upload scanner-vuln-[version].zip at very end of this script in case it breaks previous steps. #1403

Copy link
Contributor

@dcaravel dcaravel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more nit.

LGTM, approving because believe it will achieve the desired outcome, however because it will modify scanner-vuln-updates.zip which is a 'live' file, HOLD OFF on the merge until others approve and are aware that the change is being made.

Also recommend before merging to test uploading this 'combo' file to older supported versions of Central and ensure nothing breaks (I don't expect things to break, but let's be sure - ie: because we're adding another 300MiB it's possible the default timeouts may need to be adjusted, and we need to be extra sure that older Centrals will ignore the extra V4 files in the bundle)

scripts/ci/jobs/diff-dumps.sh Outdated Show resolved Hide resolved
@daynewlee daynewlee added generate-dumps-on-pr Generates the image based on dumps from the PR and removed generate-dumps-on-pr Generates the image based on dumps from the PR labels Feb 12, 2024
@daynewlee daynewlee added the generate-dumps-on-pr Generates the image based on dumps from the PR label Feb 12, 2024
@daynewlee daynewlee force-pushed the yli3/v4Dump branch 3 times, most recently from 4525922 to a804c67 Compare February 13, 2024 16:51
Copy link
Contributor

@jvdm jvdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide your testing steps?

  1. Scanner V4 works.
  2. Scanner V2 works.

@daynewlee
Copy link
Contributor Author

Can you provide your testing steps?

1. Scanner V4 works.

2. Scanner V2 works.

That makes sense. I will add steps for testing new scanner-vuln-update.zip working with Scanner V2 but probably I can't test the bundles working with Scanner V4 since stackrox/stackrox#9409 and ROX-21981 are not done yet. @jvdm

@daynewlee daynewlee removed the generate-dumps-on-pr Generates the image based on dumps from the PR label Feb 14, 2024
@daynewlee daynewlee requested a review from jvdm February 14, 2024 21:24
@RTann
Copy link
Collaborator

RTann commented Feb 15, 2024

@daynewlee can I download a sample ZIP to look at?

@daynewlee daynewlee force-pushed the yli3/v4Dump branch 2 times, most recently from de69440 to 2b3c1c6 Compare February 16, 2024 17:05
@daynewlee daynewlee requested a review from RTann February 16, 2024 17:07
@daynewlee daynewlee added the generate-dumps-on-pr Generates the image based on dumps from the PR label Feb 16, 2024
scripts/ci/jobs/diff-dumps.sh Outdated Show resolved Hide resolved
@daynewlee daynewlee force-pushed the yli3/v4Dump branch 2 times, most recently from 4e28888 to be5a87a Compare February 21, 2024 02:59
@daynewlee daynewlee requested a review from jvdm February 21, 2024 03:00
Co-authored-by: J. Victor Martins <[email protected]>
@daynewlee daynewlee merged commit 0606b0a into master Feb 21, 2024
18 checks passed
@daynewlee daynewlee deleted the yli3/v4Dump branch February 21, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
generate-dumps-on-pr Generates the image based on dumps from the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants