DieTechniker / secon-tool

Verschlüsselung nach GKV Datenaustausch (SECON)
GNU Lesser General Public License v3.0
21 stars 7 forks source link

RSASSA-PSS 4096 doesn't work #13

Closed christian-schlichtherle closed 4 years ago

christian-schlichtherle commented 4 years ago

There seems to be a problem with RSA keys with 4096 bits and the RSASSA-PSS algorithm when the private keys and certificates are generated by the keytool. Here is a transcript demonstrating that it works with 2048 bits, but not 4096 bits:

$ echo 'Hello world' > message.txt
$ keytool -keystore keystore.p12 -storepass secret -storetype PKCS12 -genkeypair -alias test2k -dname "cn=Test 2048" -keyalg rsa -keysize 2048 -sigalg rsassa-pss
Generating 2.048 bit RSA key pair and self-signed certificate (RSASSA-PSS) with a validity of 90 days
    for: CN=Test 2048
$ keytool -keystore keystore.p12 -storepass secret -storetype PKCS12 -genkeypair -alias test4k -dname "cn=Test 4096" -keyalg rsa -keysize 4096 -sigalg rsassa-pss
Generating 4.096 bit RSA key pair and self-signed certificate (RSASSA-PSS) with a validity of 90 days
    for: CN=Test 4096
$ java -jar build/libs/kks-0.0.1-SNAPSHOT-all.jar -keystore keystore.p12 -storepass secret -alias test2k -recipient test2k -source message.txt -sink message.cms
$ java -jar build/libs/kks-0.0.1-SNAPSHOT-all.jar -keystore keystore.p12 -storepass secret -alias test2k -source message.cms -sink /dev/tty
Hello world
$ java -jar build/libs/kks-0.0.1-SNAPSHOT-all.jar -keystore keystore.p12 -storepass secret -alias test4k -recipient test4k -source message.txt -sink message.cms
$ java -jar build/libs/kks-0.0.1-SNAPSHOT-all.jar -keystore keystore.p12 -storepass secret -alias test4k -source message.cms -sink /dev/tty
Hello world
Exception in thread "main" de.tk.security.kks.KksInvalidSignatureException
    at de.tk.security.kks.KksSubscriber$1.verify(KksSubscriber.java:180)
    at de.tk.security.kks.KksSubscriber$1.verifyIo(KksSubscriber.java:165)
    at de.tk.security.kks.SideEffect.lambda$andThen$0(SideEffect.java:46)
    at de.tk.security.kks.SideEffect.runAll(SideEffect.java:31)
    at de.tk.security.kks.KksSubscriber$1.close(KksSubscriber.java:160)
    at de.tk.security.kks.SideEffect.lambda$andThen$0(SideEffect.java:40)
    at de.tk.security.kks.SideEffect.runAll(SideEffect.java:31)
    at de.tk.security.kks.Streams$1.close(Streams.java:37)
    at global.namespace.fun.io.api.Socket.accept(Socket.java:111)
    at global.namespace.fun.io.spi.Copy.copy(Copy.java:91)
    at global.namespace.fun.io.bios.BIOS.copy(BIOS.java:537)
    at de.tk.security.kks.KKS.lambda$copy$3(KKS.java:237)
    at de.tk.security.kks.KKS.lambda$callable$4(KKS.java:253)
    at de.tk.security.kks.KKS.call(KKS.java:247)
    at de.tk.security.kks.KKS.copy(KKS.java:236)
    at de.tk.security.kks.Main.run(Main.java:104)
    at de.tk.security.kks.Main.main(Main.java:42)
$ 

Debugging the problem doesn't help because the validation fails in BC and the byte code in their lib doesn't have debugging infos. However, I suspect the problem in the special setup for keys of 4096 bits or more. This is happening in KksSubscriber.java.

Is this a bug or a feature? I would expect the tool to work with any proper sigalg and keysize, but it doesn't.

loetifuss commented 4 years ago

That's odd, since all the production certs with 4096 bit keys use RSASSA-PSS signatures. For testing we also use a selfsigned 4096 bit cert (not generated with keytool, though)

Using OpenJDK 11.0.5.10 I managed to create a certificate using the above keytool cmdline. The generated cert has "SHA384withRSAandMGF1" set as signature algorithm which is different from the "SHA256withRSAandMGF1" used in production.

loetifuss commented 4 years ago

I added some tests using certificates with different signature/key settings:

RSA 2048 with SHA256 (works) RSA 4096 with SHA256withRSAandMGF1 (works) RSA 4096 with SHA384withRSAandMGF1 (did not work)

With #14 the algorithm information is taken from the certificate. However, this means the API will also create/verify signatures using certificates that don't comply with the specs in "SECON".

Further, a lot of code isn't necessary anymore with the new BouncyCastle version and has been removed.

christian-schlichtherle commented 4 years ago

Thank you very much for the fix - LGTM. We will repeat our interoperability tests and let you know. Once it's done this issue can get closed.

Two minor notes:

Thanks again for your efforts on resolving this issue!

christian-schlichtherle commented 4 years ago

Our interoperability tests with the AppSec tool have been successful.