-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
windows root certificate scanning #14229
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Good work.
Would you mind using CertOpenSystemStoreW
instead of CertOpenSystemStoreA
? See #534 for more details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more thing-
cb.bytes.clearRetainingCapacity(); | ||
cb.map.clearRetainingCapacity(); | ||
|
||
const store = os.windows.crypt32.CertOpenSystemStoreA(null, "ROOT"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function may fail. Please check for an error after this call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. There is no documentation about possible errors, so I used unexpectedError
. Also, used unreachable in closeStore function in defer clause (closeStore also can return error), because in any case, it will free memory (should I change it to do noop?). I hope this is ok. Also, I don't think that CertOpenSystemStoreW can return error, if i provide string other than "ROOT"
(some other unexpected string), it doesn't seem to return any error. But in any case, i handled possible errors, i think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CertOpenSystemStoreW
Errors from the called function CertOpenStore are propagated to this function.
Which includes ERROR_FILE_NOT_FOUND
.
CertOpenStore
CreateFile, ReadFile, or registry errors might be propagated and their error codes returned.
Alongside these all of the error codes associated with ReadFile and WriteFile.
6a6c76a
to
1eb311a
Compare
* keep helper functions out of the DLL bindings APIs * unify the logic for linux and windows certificate scanning with regards to error handling
Closes #14170
Works as expected.
Any code reviews are pleased.