feat: use sha256 if using a fips provider
When Node.JS is using a FIPS provider, md5 is an unsupported algorithm/routine and calls using it will throw/fail. This adjusts the digest authentication method to check crypto.getFips() and if fips is enabled, it will try to use sha256 instead which can be a supported hashing algorithm for onvif devices.
See section 5.9.2
Hi, Thanks for the Pull Request and for the reference into the Core Spec.
I agree that we should not be calling MD5 in a FIPs environment.
But what we should be doing is looking at all the "algorithm=XXX" strings in the HTTP Response from the ONVIF Device and picking a suitable hashing function that is accepted by the ONVIF device and by FIPS.
Is it possible to have some remote access to the ONVIF device you are using which uses SHA256 for some testing. I had a Hik camera that did SHA256 but had to return it as it was on loan. I'm not sure any of my other cameras support SHA256.
And then I think we need a change that does the following a) Parses the headers from the ONVIF device which can include multiple algorithm=NNN headers. b) if we are in FIPS mode, filter out MD5 c) use an Algorithm that the camera supports
This would make it future proof for SHA512 or anything else the camera offers up.
Are you up for making this change, or giving me some remote access from the UK
Thanks
I've made changes to the master branch which should help you. We have to obey the capabilities of the camera and cannot just assume it supports SHA256 so I now do the following a) I accept multiple WWW-Authenticate headers from the camera b) I then try and build a HTTP Digest Reply using whichever crypto algorithms are premitted c) I pick SHA256 over MD5 if both are available
Tested with a Hik Camera in MD5-Then-SHA256 mode. Also tested with an Custom ONVIF server that is in SHA256 mode
So I'll close this PR but please re-open a new one if there are problems.
Hello!
I think that we can add the private method _getHashAlgorithm() and _hashAlgorithm property to the Cam properties defaults to MD5.
Then, in every request request from the response check the headers for "algorithm=XXX" and change the value of _hashAlgorithm here: https://github.com/agsh/onvif/blob/master/lib/cam.js#L305
And if getFips() is ok and _hashAlgorithm also is SHA256, then use it. Otherwise use MD5.
Do you agree?
Hi Andrew,
_> And if getFips() is ok and hashAlgorithm also is SHA256, then use it. Otherwise use MD5.
It is easier than this. Everyone can use SHA256. We don't have to be using a FIPS system to use SHA256. So the check is "if the _hashAlgorith is SHA256, then use it". Nice and simple.
What FIPS does is throw an exception if you try and use crypto.createHash with MD5. It relies on everyone using the OS or NodeJS crypto and not having a local library of hash functions.
So instead of a test for _hashAlgorith SHA256, we really need a test for MD5 and getFips()
So in Cam.prototype.createHash I can add these two lines
const isFipsCompliant = crypto.getFips() === 1;
if (isFipsCompliant && (algorithm == "MD5" || algorithm == "md5")) throw "Error. MD5 not permitted on FIPS systems";
I already support this via a try/catch when calling crypto.createHash(). On a FIPS system it will throw an exception if I call crypto.createHash('md5');
But I can make my code clearer and test for FIPS first.
So you are correct that we need to use _hashAlgorith, but we do not need to check for FIPS to use SHA256.
It is also worth noting that the ONVIF Core Spec allows a Camera to advertise MD5 and SHA256 support, OR to only use SHA256. My new code handles this.
I do have to thank @bryopsida for the original PR as it showed up a weakness that the Digest code was hard coded to MD5 and as not using the algorithm=XXX value provided by the ONVIF device.