bcgit / bc-java

Bouncy Castle Java Distribution (Mirror)
https://www.bouncycastle.org/java.html
MIT License
2.3k stars 1.14k forks source link

insufficient_security with bctls-fips-1.0.12.x and TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 #1022

Open gstanchev opened 3 years ago

gstanchev commented 3 years ago

All is working with bctls-fips-1.0.11.x. Getting [1]. The bc jsse is set to debug but not much it is coming out of it. I traced the issue to this commit: https://github.com/bcgit/bc-java/commit/1dd2eeecbe60358ec85aa3f5653e615080625abd#

Let me know if I can provide more info

[1] │ │ 2021-09-10 16:21:08.392 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.allowLegacyMasterSecret] defaulted to: true │ │ 2021-09-10 16:21:08.392 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.allowLegacyResumption] defaulted to: false │ │ 2021-09-10 16:21:08.393 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Integer system property [jdk.tls.maxCertificateChainLength] defaulted to: 10 │ │ 2021-09-10 16:21:08.393 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Integer system property [jdk.tls.maxHandshakeMessageSize] defaulted to: 32768 │ │ 2021-09-10 16:21:08.394 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [com.sun.net.ssl.requireCloseNotify] defaulted to: true │ │ 2021-09-10 16:21:08.394 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.useExtendedMasterSecret] defaulted to: true │ │ 2021-09-10 16:21:08.445 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.fips.allowRSAKeyExchange] defaulted to: true │ │ 2021-09-10 16:21:08.470 INFO 355 --- [ main] o.b.jsse.provider.PropertyUtils : Found string security property [jdk.tls.disabledAlgorithms]: SSLv3, RC4, DES, MD5withRSA, DH keySize < 1024, EC k │ │ 2021-09-10 16:21:08.474 INFO 355 --- [ main] o.b.jsse.provider.PropertyUtils : Found string security property [jdk.certpath.disabledAlgorithms]: MD2, MD5, SHA1 jdkCA & usage TLSServer, RSA key │ │ 2021-09-10 16:21:08.475 WARN 355 --- [ main] o.b.j.p.DisabledAlgorithmConstraints : Ignoring unsupported entry in 'jdk.certpath.disabledAlgorithms': SHA1 jdkCA & usage TLSServer │ │ 2021-09-10 16:21:08.491 INFO 355 --- [ main] o.b.jsse.provider.PropertyUtils : Found boolean security property [keystore.type.compat]: true │ │ 2021-09-10 16:21:08.833 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [com.sun.net.ssl.checkRevocation] defaulted to: false │ │ 2021-09-10 16:21:08.833 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.trustManager.checkEKU] defaulted to: true │ │ 2021-09-10 16:21:08.930 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Integer system property [javax.net.ssl.sessionCacheSize] defaulted to: 20480 │ │ 2021-09-10 16:21:08.935 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.ec.disableChar2] defaulted to: false │ │ 2021-09-10 16:21:08.936 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.ec.disable_f2m] defaulted to: false │ │ 2021-09-10 16:21:08.936 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jsse.enableFFDHE] defaulted to: true │ │ 2021-09-10 16:21:09.791 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.client.assumeOriginalHostName] defaulted to: false │ │ 2021-09-10 16:21:09.791 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.trustNameService] defaulted to: false │ │ 2021-09-10 16:21:09.806 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.client.acceptRenegotiation] defaulted to: false │ │ 2021-09-10 16:21:09.837 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.client.enableCAExtension] defaulted to: false │ │ 2021-09-10 16:21:09.837 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.client.enableSessionResumption] defaulted to: true │ │ 2021-09-10 16:21:09.838 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.client.enableStatusRequestExtension] defaulted to: true │ │ 2021-09-10 16:21:09.838 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.client.enableTrustedCAKeysExtension] defaulted to: false │ │ 2021-09-10 16:21:09.838 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jsse.enableSNIExtension] defaulted to: true │ │ 2021-09-10 16:21:10.004 DEBUG 355 --- [ main] o.b.jsse.provider.ProvTlsClient : Client notified of selected protocol version: TLSv1.2 │ │ 2021-09-10 16:21:10.005 DEBUG 355 --- [ main] o.b.jsse.provider.ProvTlsClient : Server did not specify a session ID │ │ 2021-09-10 16:21:10.015 DEBUG 355 --- [ main] o.b.jsse.provider.ProvTlsClient : Client notified of selected cipher suite: TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 │ │ 2021-09-10 16:21:10.033 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Integer system property [org.bouncycastle.jsse.client.dh.minimumPrimeBits] defaulted to: 2048 │ │ 2021-09-10 16:21:10.034 DEBUG 355 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.client.dh.unrestrictedGroups] defaulted to: false │ │ 2021-09-10 16:21:10.576 INFO 355 --- [ main] o.b.jsse.provider.ProvTlsClient : Client raised fatal(2) insufficient_security(71) alert: Failed to read record │ │ org.bouncycastle.tls.TlsFatalAlert: insufficient_security(71) │ │ at org.bouncycastle.tls.TlsDHUtils.receiveDHConfig(TlsDHUtils.java:139) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsDHEKeyExchange.processServerKeyExchange(TlsDHEKeyExchange.java:95) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsClientProtocol.handleHandshakeMessage(TlsClientProtocol.java:672) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsProtocol.processHandshakeQueue(TlsProtocol.java:695) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsProtocol.processRecord(TlsProtocol.java:584) ~[bctls-fips-1.0.12.2.jar:1.0.12.2]

Caused by: org.bouncycastle.tls.TlsFatalAlert: insufficient_security(71) │ │ at org.bouncycastle.tls.TlsDHUtils.receiveDHConfig(TlsDHUtils.java:139) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsDHEKeyExchange.processServerKeyExchange(TlsDHEKeyExchange.java:95) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsClientProtocol.handleHandshakeMessage(TlsClientProtocol.java:672) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsProtocol.processHandshakeQueue(TlsProtocol.java:695) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsProtocol.processRecord(TlsProtocol.java:584) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.RecordStream.readRecord(RecordStream.java:245) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsProtocol.safeReadRecord(TlsProtocol.java:843) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsProtocol.blockForHandshake(TlsProtocol.java:417) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.tls.TlsClientProtocol.connect(TlsClientProtocol.java:88) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.jsse.provider.ProvSSLSocketWrap.startHandshake(ProvSSLSocketWrap.java:620) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.bouncycastle.jsse.provider.ProvSSLSocketWrap.startHandshake(ProvSSLSocketWrap.java:596) ~[bctls-fips-1.0.12.2.jar:1.0.12.2] │ │ at org.postgresql.ssl.MakeSSL.convert(MakeSSL.java:41) ~[postgresql-42.2.23.jar!/:42.2.23] │ │ ... 52 common frames omitted

gstanchev commented 3 years ago

As a temporary workaround i have set -Dorg.bouncycastle.jsse.client.dh.unrestrictedGroups=true which allows the handshake to proceed but it is a bandaid solution

peterdettman commented 3 years ago

I've reviewed that commit and can't see how it can be causing an issue, but perhaps if you could collect the specific p, g values of this group I could test locally.

gstanchev commented 3 years ago

image image

[-163424329, 142542871, 1923267286, -1455230203, -386313455, 961525489, 1857637176, 551158183, 1453528297, -1891609101, -804796725, 1135187117, -145988474, 1240992899, -1117394665, -727592112, -118138529, -596225457, 1027444187, -828776105, -1839608323, -1196526513, -973310276, -2084223184, 1640922313, 1727993337, -1141010082, -956192045, 1537117600, -1519240492, 451985222, 5778405, -192348840, 1313463245, 548707812, -1861798293, 862730253, 1159532412, -2001527684, 1529712374, -204921920, 1139844443, 411930299, 1435285597, 953365757, 2081904451, -1558374292, -567074516, -1255473348, -508483264, 403803516, -2069427598, -695811069, 432539433, 2060096780, -644440149, -804630373, 38195976, 1030136925, 1100979324, -1115075806, 421968555, -1570847915, -382596293]

gstanchev commented 3 years ago

I was able to step through and " DHGroup dhGroup = getStandardGroupForDHParameters(p, g);" returns -1 and then "if (!dhGroupVerifier.accept(dhGroup))" returns false due to DefaultTlsDHGroupVerifier.checkGroup(DHGroup dhGroup) returns false due to not finding a match...

peterdettman commented 3 years ago
  1. It seems the server is simply using parameters that we don't recognize, and this is considered insecure by default (but can be accepted using the system property, as you have done). FYI this is because it is not feasible to fully validate the DH parameter set as secure on the time scale of a handshake.
  2. I don't see then how the commit you reference could be introducing this problem, since with a null return value the new code behaves exactly the same as the old (or am I missing something?).
  3. Do you definitely need to be using DH cipher suites for this server? It might be better to just disable all DHE_* cipher suites from the enabled list for your SSLContext. (There is a system property "jsse.enableFFDHE" that could be set to false, but looking at it I am not clear that it will achieve this properly on the client - I may need to improve some things to make this viable).
gstanchev commented 3 years ago

I don't have a control over the SSLContext as the issue is on the bottom of a JDBC client making connection to a Postgres service - both running in a container managed by kubernetes. The actual client call is buried in layers of JDBC, JPA and Spring... You are probably correct that this commit is probably OK - my presumption was that 1.0.11.4 negotiates the same protocol as 1.0.12.2 but it doesn't [1] - the server picks TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256. So apples to oranges and it doesn't go through the same code. I can try to set "jsse.enableFFDHE" as you recommended to see if it will help.

Since the server is a vanilla Postgress (unsure about the version) doesn't it make sense to expand the "known" groups to cover this one? What are the chances of others running into the same situation with 1.0.12.2 if not addressed?

[1]

2021-09-14 17:25:38.701 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.allowLegacyMasterSecret] defaulted to: true 2021-09-14 17:25:38.703 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.allowLegacyResumption] defaulted to: false 2021-09-14 17:25:38.704 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Integer system property [jdk.tls.maxCertificateChainLength] defaulted to: 10 2021-09-14 17:25:38.704 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Integer system property [jdk.tls.maxHandshakeMessageSize] defaulted to: 32768 2021-09-14 17:25:38.704 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [com.sun.net.ssl.requireCloseNotify] defaulted to: true 2021-09-14 17:25:38.705 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.useExtendedMasterSecret] defaulted to: true 2021-09-14 17:25:38.758 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.fips.allowRSAKeyExchange] defaulted to: true 2021-09-14 17:25:38.788 INFO 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Found string security property [jdk.tls.disabledAlgorithms]: SSLv3, RC4, DES, MD5withRSA, DH keySize < 1024, EC keySize < 224, 3DES_EDE_CBC, anon, NULL 2021-09-14 17:25:38.792 INFO 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Found string security property [jdk.certpath.disabledAlgorithms]: MD2, MD5, SHA1 jdkCA & usage TLSServer, RSA keySize < 1024, DSA keySize < 1024, EC keySize < 224 2021-09-14 17:25:38.794 WARN 2660 --- [ main] o.b.j.p.DisabledAlgorithmConstraints : Ignoring unsupported entry in 'jdk.certpath.disabledAlgorithms': SHA1 jdkCA & usage TLSServer 2021-09-14 17:25:38.836 INFO 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Found boolean security property [keystore.type.compat]: true 2021-09-14 17:25:39.169 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [com.sun.net.ssl.checkRevocation] defaulted to: false 2021-09-14 17:25:39.170 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.trustManager.checkEKU] defaulted to: true 2021-09-14 17:25:39.278 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Integer system property [javax.net.ssl.sessionCacheSize] defaulted to: 20480 2021-09-14 17:25:39.282 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.ec.disableChar2] defaulted to: false 2021-09-14 17:25:39.282 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.ec.disable_f2m] defaulted to: false 2021-09-14 17:25:39.282 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jsse.enableFFDHE] defaulted to: true 2021-09-14 17:25:40.513 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.trustNameService] defaulted to: false 2021-09-14 17:25:40.556 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.client.enableCAExtension] defaulted to: false 2021-09-14 17:25:40.557 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.client.enableSessionResumption] defaulted to: true 2021-09-14 17:25:40.557 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jdk.tls.client.enableStatusRequestExtension] defaulted to: true 2021-09-14 17:25:40.557 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [org.bouncycastle.jsse.client.enableTrustedCAKeysExtension] defaulted to: false 2021-09-14 17:25:40.558 DEBUG 2660 --- [ main] o.b.jsse.provider.PropertyUtils : Boolean system property [jsse.enableSNIExtension] defaulted to: true 2021-09-14 17:25:40.691 DEBUG 2660 --- [ main] o.b.jsse.provider.ProvTlsClient : Client notified of selected protocol version: TLSv1.2 2021-09-14 17:25:40.692 DEBUG 2660 --- [ main] o.b.jsse.provider.ProvTlsClient : Server did not specify a session ID 2021-09-14 17:25:40.703 DEBUG 2660 --- [ main] o.b.jsse.provider.ProvTlsClient : Client notified of selected cipher suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256

peterdettman commented 3 years ago

Since the server is a vanilla Postgress (unsure about the version) doesn't it make sense to expand the "known" groups to cover this one? What are the chances of others running into the same situation with 1.0.12.2 if not addressed?

Well, to be frank, the more sensible thing would be for vanilla Postgres to use a standardised group.

I am increasingly tempted to remove DHE from the default enabled cipher suites, or perhaps add a system property to easily enable/disable them from that list.

mouse07410 commented 3 years ago

I suggest that removing DHE would be a bad idea, and not justified from cryptography point of view.

Making it easy to enable/disable - sounds good.

jasonberanek commented 1 year ago

We ran into this problem recently as well when updating from 1.0.11.4 to 1.0.14.1 over the last few weeks, also in connection to performing a JDBC connection. Our problem was resolved by upgrading our database version to one that support/resolved ECDHE ciphers rather than DHE ciphers.

In terms of where this was introduced, I was looking at the code and it appears that it was introduced in this commit that should have been originally released with 1.0.12 (https://github.com/bcgit/bc-java/commit/9029670befa0cf87f9d8d7f5e1d9f4af61d3cdc4). That added DHE ciphers into the default cipher suite list for the library, where they were not part of the default list previously. That would explain @gstanchev findings that 1.0.11 and 1.0.12 use different ciphers.