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

fix: add retry on 503 errors #90

Merged
merged 1 commit into from
Nov 24, 2023
Merged

fix: add retry on 503 errors #90

merged 1 commit into from
Nov 24, 2023

Conversation

jeremylong
Copy link
Owner

With this update I was able to create a local cache of the NVD in ~30 minutes. Prior to releasing ODC 9.0.0 this took ~2 minutes - so the number of users from the ODC community appear to be crushing the NVD API atm.

@jeremylong jeremylong merged commit 2cc6b70 into main Nov 24, 2023
1 check passed
@jeremylong jeremylong deleted the scratch/retry branch November 24, 2023 14:20
@danshome
Copy link

danshome commented Nov 24, 2023

@jeremylong Thanks for fixing this on a holiday weekend! My change was more of a hack to that one method because I wasn't familiar enough with code to attempt to make changes everywhere. I didn't see a check for the Retry-After response header in your change. You should use that in your retry logic if it's provided, and log it at DEBUG if the API responds with an explicit Retry-After; if the Retry-After is greater than 5 minutes, then log it at INFO so it's obvious to the end users that NVD is having a more serious issue. Ignoring the Retry-After in the response could be viewed as a DOS and lead to NIST blocking networks. I know because I do that on our firewalls, if we set a Retry-Time out and we see clients ignoring it, we will immediately block their IP. I'd only use your meter logic if you find that Retry-Time isn't in the response, and let the user configure the value you are passing into meter, I'd prefer to be the one to decide if I want to risk setting it lower and getting rate-limited by NIST.

@jeremylong
Copy link
Owner Author

I know about the Retry-After response - it is used in the default implementation. However, I validated that the 503s do not include this header. Good point and I probably should add the code to what has been implemented in case the NVD starts to respond with that header.

@jeremylong
Copy link
Owner Author

@danshome see #94 and #95.

@danshome
Copy link

danshome commented Nov 25, 2023

@jeremylong I'm testing your changes now. Has it been your experience that the new API is considerably slower? I'm quite surprised they aren't supporting HTTP2 and haven't moved to ECC certificates; both would help significantly reduce the load on their systems. It might be worth adding timings to your default logging. It may come in handy to make the case to NIST that they need to do something about the performance of the new API. If things are this slow now, they are going to get killed when they take the old API offline. I've already sent them several emails about my concerns. I've done several tests downloading the entire database, and I haven't had a single run that hasn't taken well over 1.5-2 hours; that's considerably slower than the old API.

@jeremylong
Copy link
Owner Author

@danshome I've been able to use the vulnz CLI to create the offline cache in less then 30 minutes. Prior to releease ODC 9.0.0 I was able to do this in about 2 minutes.

$ op run -- ./vulnz-5.0.2.jar cve --cache --directory ./cache  --threads=4 --debug --delay=3000 --prettyPrint
Completed in 1545 seconds

If I up the thread count to 10 it speeds up a little bit:

$ op run -- ./vulnz-5.0.2.jar cve --cache --directory ./cache  --threads=10 --debug --delay=3000 --prettyPrint
Completed in 716 seconds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants