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

Switch to server-side hashing for SignServer support #260

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Vampire
Copy link
Contributor

@Vampire Vampire commented Nov 18, 2024

When using for example a Nitrokey HSM 2, client-side hashing seems to not work.
Additionally if CLIENTSIDEHASHING was set to true in the worker as documented,
the certificate fetching failed.

This PR also uses client-side hashing for the certificate retrieval,
and additionally allows to suffix the alias with |serverside, to allow doing
server-side hashing when necessary.

@ebourg
Copy link
Owner

ebourg commented Nov 19, 2024

Thank you for the follow up. Does the serverside hashing work in all cases? Maybe we could use that to keep things simple.

@Vampire
Copy link
Contributor Author

Vampire commented Nov 19, 2024

I wouldn't recommend that, and no, depends on the configuration on the SignServer.
By using client-side hashing the hash-calculation is done on the client and only the hash sent to the server and signed.
When using server-side hashing, you have to send the whole data blob over the line, and the server then has to calculate the hash additionally to doing the signing.
Especially when doing many sign operations e.g. from multiple CI agents in parallel, I think the saved network and computational overhead should warrant the bit of more complexity in JSign.

@ebourg
Copy link
Owner

ebourg commented Nov 19, 2024

I'm suggesting this because the data to be signed is an ASN.1 structure (SpcIndirectDataContent) containing the hash of the file, it's just a few bytes larger than the hash that would be produced on the client side. It's not like the whole file would be sent over the network.

What error did you get with the Nitrokey HSM? It's a bit odd that client side hashing doesn't work, that sounds like a bug in SignServer.

@Vampire
Copy link
Contributor Author

Vampire commented Nov 19, 2024

The error is Caused by: java.security.NoSuchAlgorithmException: No such algorithm: RSA/ECB/PKCS1Padding.
As far as I understood it might be a bug or shortcoming in Nitrokey HSM 2, or in the used PKCS#11 library or in the SunPKCS11 provider, or in SignServer. 🤷‍♂️

It's not like the whole file would be sent over the network.

Oh, ok, then this was a misunderstanding on my side, it is indeed 126 byte vs. 32 byte for SHA-256.
That's sad. I hoped that with client-side hashing the speed would significantly improve, as the signing seems to be pretty slow.
But yeah, if that is the case, then network overhead is minimal of course and the hashing is probably also not the performance bottleneck. :-(

So yeah, I guess server-side hashing should always work if the worker is configured properly and I'll change this PR to only do server-side hashing.
Only drawback that comes to mind, is that then you cannot control the hash algorithm through JSign.
Can it be validated that the user did not specify a hash algorithm, so that we can fail if he tries, telling him to configure is on the worker instead?

@Vampire Vampire changed the title Add server-side hashing for SignServer support Switch to server-side hashing for SignServer support Nov 19, 2024
@ebourg
Copy link
Owner

ebourg commented Nov 19, 2024

I hoped that with client-side hashing the speed would significantly improve, as the signing seems to be pretty slow.

The Nitrokey 3 NFC was quite slow last year when I tested it, around 1 second per RSA 4096 signature. The Yubikey 5 is faster, about 100ms per sign operation. I don't remember the performance of the Nitrokey HSM 2 but I think it was close to the Nitrokey 3. There was some work in the latest firmwares to enable hardware acceleration on some operations but I haven't checked.

Only drawback that comes to mind, is that then you cannot control the hash algorithm through JSign.

That's an annoying drawback :( That means the algorithm has to be specified to Jsign, the process is open to misuse and errors.

@ebourg
Copy link
Owner

ebourg commented Nov 19, 2024

Just noticed your message on the Nitrokey support forum:
https://support.nitrokey.com/t/is-rsa-signing-a-pre-computed-hash-with-nitrokey-hsm-2-not-possible/6652

@Vampire
Copy link
Contributor Author

Vampire commented Nov 19, 2024

That's an annoying drawback :( That means the algorithm has to be specified to Jsign, the process is open to misuse and errors.

Did not fully get that.
I modified the PR to do only server-side now.
Does JSign needs to know the hash algorithm somewhere besides the sign service where it now just is not used for SignServer?

@ebourg
Copy link
Owner

ebourg commented Nov 19, 2024

I think Jsign has to know the algorithm, I'll check.

@Vampire
Copy link
Contributor Author

Vampire commented Nov 19, 2024

Maybe this will help if it gets considered Keyfactor/signserver-ce#108 :-)

@Vampire
Copy link
Contributor Author

Vampire commented Nov 19, 2024

But currently you are right.
The worker config has to be consistent with the arguments to Jsign.
I tried to create a SHA265withRSA signature on Jsign side with a worker configured as SHA1withRSA and earned an ObjectIdentifier mismatch: 1.3.14.3.2.26 during signature verification.

@Vampire
Copy link
Contributor Author

Vampire commented Nov 19, 2024

For now I changed

Configuring the hashing algorithm when invoking Jsign will be ignored.

to

It is important that the hashing algorithm that is configured for the worker is consistent with the hashing algorithm configured for Jsign, otherwise the signature verification will fail.

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