Fix Error Handling of Call to CertOpenSystemStoreA #1
Conversation
|
Hi Fabian, |
CertOpenSystemStoreA returns null when it fails. This can e.g. be due to insufficient access rights to the certificate store. However, ffi returns a buffer whose truthiness will always be true. To properly catch the error and prevent the application from crashing, we need to check for null using the ref package.
FabianScheidt
force-pushed
the
fix-error-handling
branch
from
2aa3045 to
6d9feef
Compare
|
Hi Simon, thanks for looking at my changes! The fact that you are not using the library yourself gives me extra confidence to put this into production 😉 Joking aside: I am building a web application that has to run on a windows server and should communicate with some other internal components, that make use of corporate certificates. I looked at some other libraries that try to solve similar problems, but yours looks most straight forward to me. Since I found my configuration problem in IIS, it seems to work great so far. As mentioned, I tweaked my code to actually throw an error when the certificate store can not be opened. I bumped the version in the package.json, so you can push it straight to npm. I think this is good enough for now, so feel free to merge the changes. Thanks again! |
2 participants
Hi!
I came across a problem when using your library in a web server running inside of iisnode. It turns out that by default the IIS application pool identity is configured to not load a user profile. Therefore it is unable to access the certificate store. Since the error of Crypt32 is not handled properly, it causes the node process to crash without any error message in a subsequent ffi-call. This PR attempts to fix that behaviour:
CertOpenSystemStoreA returns null when it fails (see here). As stated before, this can e.g. be due to insufficient access rights to the certificate store. However, ffi returns a buffer whose truthiness will always be true. To properly catch the error and prevent the application from crashing, we need to check for null using the ref package as done with the other call to Crypt32.
My changes avoid application crashes. However, I think we should actually throw an appropriate error that can be handled by the application that uses the library. I'm happy to add that. Just let me know your thoughts!
Greetings,
Fabian