Replace default insecure SHA1 hash algorithm

[jornfranke]

Replace default insecure SHA1 hash algorithm with SHA256.

SHA1 has been broken and should not be used anymore for signatures and should not be the default, cf. also https://security.googleblog.com/2016/11/sha-1-certificates-in-chrome.html

[asfgit]

Can one of the admins verify this patch?

[pjfanning]

Can you fix the javadoc on line 227?

  /**
     * @return the main digest algorithm, defaults to sha-1
     */

[pjfanning]

@kiwiwings Does this change in default Digest Algorithm look ok to you?

[Alain-Bearez]

Did you test this change with existing legacy files?

[kiwiwings]

I'm not sure what to recommend ...

• SHA256/512 seems to be usable without JCE jurisdiction policy files, but I guess for RSA you'll need anyway the policy files and bouncycastle ... i.e. I haven't tried it myself now ...
• setting this by default might have unexpected results on people using the old default.
• it's easy to set it explicit over the SignatureConfig
• as usual, we should stick with the defaults Office uses (yes, this can be modified via the ADM templates ...)

So I give it a "0" - I won't commit it, if anyone else does ... I don't mind

[jornfranke]

I agree with you. As an alternative we may only update the encryption webpage of the poi webpage where we explicitly tell the users to set the hash algorithm to a secure one, such as SHA 256. The reason is that certain users will most likely use the default and are not aware that there could be issues with it.

…

On 10. Jan 2018, at 00:22, Andreas Beeker ***@***.***> wrote: I'm not sure what to recommend ... SHA256/512 seems to be usable without JCE jurisdiction policy files, but I guess for RSA you'll need anyway the policy files and bouncycastle ... i.e. I haven't tried it myself now ... setting this by default might have unexpected results on people using the old default. it's easy to set it explicit over the SignatureConfig as usual, we should stick with the defaults Office uses (yes, this can be modified via the ADM templates ...) So I give it a "0" - I won't commit it, if anyone else does ... I don't mind — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[pjfanning]

+1 to updating the docs to show how users can avoid SHA-1
I'd still favour adding the code change if it isn't too disruptive (especially since the next release is a major release).
The unit tests all pass with this change. I don't know if we have enough test coverage in this area for this to be regarded as a useful metric. I'm guessing but I would expect that any files that we read will have the hash algorithm specified in the file itself - so this mean the default shouldn't affect reads. In cases where users are writing files, then with the changed default, they might find that some tools can't read the new file. In this case, it is still possible for a user to modify their code to set the hash algorithm for the write to be SHA-1.

[pjfanning]

If there are no objections, I will merge this change.
I think 4.0.0 is a good place to make changes like this.

[kiwiwings]

To repeat Alain question ... Have you tried that on legacy files?
I've done it and spend some time on a fix for a SO question ... SHA256 leads to a file which Powerpoint 2016 rejects because of an invalid signature. Setting it back to SHA1 makes it working again.
I'm now searching for the cause, to make it eventually work with SHA256 - maybe it's my key/cert, maybe something other is not configured right, maybe something on my windows machine has to be enabled - but if you can't prove otherwise, I'll revert that for POI 4.0.0

[jornfranke]

I did not try it on legacy files. For me there is no necessity to include it in the latest version until it can be fully tested. I do not have office 2016 though. Could you share your source code. Nevertheless, we should encourage POI users to not rely on the default algorithms, but always specify it and make it ideally configurable as a good practice. For your office 2016 test: office supports also sha2 etc. it could be your certificate although i doubt it. There was some time ago a fix to handle invalid signatures on Windows ( if I remember correctly). Maybe it has not been applied to all signature algorithms. Finally we should test that Apache POI generates correct signatures with SHA256 in any case.

…

On 18. Feb 2018, at 11:35, Andreas Beeker ***@***.***> wrote: To repeat Alain question ... Have you tried that on legacy files? I've done it and spend some time to on fix for a SO question ... SHA256 leads to a file which Powerpoint 2016 rejects because of an invalid signature. Setting it back to SHA1 makes it working again. I'm now searching for the cause, to make it eventually work with SHA256 - maybe it's my key/cert, maybe something other is not configured right, maybe something on my windows machine has to be enabled - but if you can't prove otherwise, I'll revert that for POI 4.0.0 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[kiwiwings]

Ok it seems to be partly a false alarm - I've used the Office ADM templates to set the hash algorithm there and compared the generated sig1.xml. So now I have a test case (a) with sha256 which is based on a pkcs12/pfx file and one (b) that reads the same certificate from the windows cert store.
(a) is accepted by Powerpoint 2016 and only differs in the SignatureValue entry with (b), i.e. all other parts of sig1.xml are the same.

An intermediate fix can be found here

I'll keep searching for the difference ...

/* Update - 19.02.2018 */
In a small test without all that POI stuff ... :
if the keys (PFX / MS-Capi) are used to encrypt the same digest, I'll get a different response when using the Cipher.doFinal() method, but when using the Signature API they are the same. So I check now, if the code can be changed along the lines of this SO example, i.e. the hash magic in POI seems to be the DER-encoded digestAlgorithm

/* Update - 26.02.2018 */
The SunMSCAPI kept me busy the last week - I found a SO post which describes the problems which I've faced. So finally I have a solution for SunMSCAPI by using the Signature API - the existing approach by calculating the hash separately and then using the RSA cipher simply doesn't work with the SunMSCAPI, i.e. the encrypted bytes can't be decrypted by the public but only with the private key, which is not correct.
I will finish now the implementation, but at least the blocker is now solved ...