P12 certificates with "Basic constraints" extension and "PathLen" constraint are ignored

[jportner]

Kibana version: 7.9? - 7.16.2

Describe the bug:

Kibana can be configured to read/parse certificates in P12 files (configured via server.ssl.keystore.path, server.ssl.truststore.path, elasticsearch.ssl.keystore.path, or elasticesearch.ssl.truststore.path) to facilitate Browser-to-Kibana or Kibana-to-Elasticsearch TLS communication.

This bug causes certificates that contain the "Basic constraints" X509 extension (OID 2.5.29.19) and a defined "PathLen" constraint to be completely and silently ignored. The symptoms can vary, but this generally causes some sort of TLS communication to fail because Kibana cannot establish trust without the missing certificate(s). Depending on the configuration of the other network entities, errors can include messages like "unable to verify the first certificate" or "self signed certificate in certificate chain".

We have confirmed internal reports of customers upgrading from <= 7.8 to >= 7.14 that experience this bug, where a P12 key store or trust store was working before, suddenly doesn't work after the upgrade.

Steps to reproduce:

Not so straightforward, you need:

• A P12 file that contains a certificate with the "Basic constraints" extension with a defined "PathLen" constraint, and
• Configure Kibana to use the P12 file (either as a key store or trust store) in a configuration that triggers the bug -- then verify that the symptom matches what is expected, OR add console.log statements in Kibana where the certificate(s) are getting consumed to verify that the expected certificate(s) are missing.

I've provided such a P12 file here: localhost.p12.zip

Click to see certificate contents

``` % keytool -list -v -keystore localhost.p12 Enter keystore password: Keystore type: PKCS12 Keystore provider: SUN Your keystore contains 1 entry Alias name: localhost Creation date: Jan 4, 2022 Entry type: PrivateKeyEntry Certificate chain length: 1 Certificate[1]: Owner: CN=localhost Issuer: CN=localhost Serial number: 7de786cc6a5f7f5f2e2093e37b8acfee837644de Valid from: Mon Dec 27 16:53:36 EST 2021 until: Tue Dec 27 16:53:36 EST 2022 Certificate fingerprints: SHA1: A6:34:18:0B:61:9F:B1:0E:03:5B:89:CB:20:90:BD:AE:7E:52:FC:20 SHA256: 82:C8:F1:75:B5:38:F3:34:CC:D8:AB:39:0D:15:43:7D:12:B7:9C:54:98:55:01:84:E5:8B:6D:C2:B7:24:F7:EA Signature algorithm name: SHA256withRSA Subject Public Key Algorithm: 2048-bit RSA key Version: 3 Extensions: #1: ObjectId: 2.5.29.35 Criticality=false AuthorityKeyIdentifier [ KeyIdentifier [ 0000: 99 9E 6E EB 17 49 6F B0 CA C9 9E BB 6E CB 9E 21 ..n..Io.....n..! 0010: 04 30 05 CD .0.. ] ] #2: ObjectId: 2.5.29.19 Criticality=true BasicConstraints:[ CA:true PathLen:0 ] #3: ObjectId: 2.5.29.37 Criticality=true ExtendedKeyUsages [ serverAuth clientAuth ] #4: ObjectId: 2.5.29.15 Criticality=true KeyUsage [ DigitalSignature Key_Encipherment ] #5: ObjectId: 2.5.29.17 Criticality=false SubjectAlternativeName [ DNSName: localhost ] #6: ObjectId: 2.5.29.14 Criticality=false SubjectKeyIdentifier [ KeyIdentifier [ 0000: 99 9E 6E EB 17 49 6F B0 CA C9 9E BB 6E CB 9E 21 ..n..Io.....n..! 0010: 04 30 05 CD .0.. ] ] ******************************************* ******************************************* ```

You can start using the following config:

server.ssl.keystore.path: /path/to/localhost.p12
server.ssl.keystore.password: storepass

Kibana should crash with the following error:

FATAL  Error: Did not find certificate in keystore at [keystore.path]

Expected behavior:

Kibana should be able to read all contents of P12 files.

Any additional context:

I tracked down the root cause to this. Given the sample/problematic P12 file, node-forge returned the certificate from the P12 just fine in unit tests, but when actually starting Kibana, node-forge did not find a certificate.

When Kibana parses a P12 file, it uses node-forge to read the file in base64 format, then converts that to DER format, then converts that to ASN.1 format, then decrypts/reads the contents of each contained SafeBag into a node-forge internal Certificate structure, and finally converts them to PEM format for the consumer.

Through debugging, I discovered that if the aforementioned extension was used, an error condition would be encountered in node-forge while converting the SafeBags from ASN.1 to the internal Certificate structure: https://github.com/digitalbazaar/forge/blob/c0bb359afca73bb0f3ba6feb3f93bbcb9166af2e/lib/pkcs12.js#L700-L706

The error is "bytes.getSignedInt is not a function", which originates in the derToInteger function here: https://github.com/digitalbazaar/forge/blob/c0bb359afca73bb0f3ba6feb3f93bbcb9166af2e/lib/asn1.js#L1110

Additional debugging determined that the prototype of the bytes object value is different when this error scenario is encountered. In unit tests this is object has a prototype of ByteStringBuffer, but when running the Kibana server this is object instead has a prototype of DataBuffer that has different properties:

Click to expand and see properties

| ByteStringBuffer properties | DataBuffer properties | |---|---| | `__defineGetter__` | `__defineGetter__` | | `__defineSetter__` | `__defineSetter__` | | `__lookupGetter__` | `__lookupGetter__` | | `__lookupSetter__` | `__lookupSetter__` | | `__proto__` | `__proto__` | | | `accommodate` | | `_constructedStringLength` | | | `_optimizeConstructedString` | | | `at` | `at` | | | `available` | | | `buffer` | | `bytes` | `bytes` | | `clear` | `clear` | | `compact` | `compact` | | `constructor` | `constructor` | | `copy` | `copy` | | `data` | `data` | | | `equals` | | `fillWithByte` | `fillWithByte` | | | `getBuffer` | | `getByte` | `getByte` | | `getBytes` | `getBytes` | | `getInt` | `getInt` | | `getInt16` | `getInt16` | | `getInt16Le` | `getInt16Le` | | `getInt24` | `getInt24` | | `getInt24Le` | `getInt24Le` | | `getInt32` | `getInt32` | | `getInt32Le` | `getInt32Le` | | | `getNative` | | `getSignedInt` | | | | `growSize` | | `hasOwnProperty` | `hasOwnProperty` | | `isEmpty` | `isEmpty` | | `isPrototypeOf` | `isPrototypeOf` | | `last` | `last` | | `length` | `length` | | | `native` | | `propertyIsEnumerable` | `propertyIsEnumerable` | | `putBuffer` | `putBuffer` | | `putByte` | `putByte` | | `putBytes` | `putBytes` | | `putInt` | `putInt` | | `putInt16` | `putInt16` | | `putInt16Le` | `putInt16Le` | | `putInt24` | `putInt24` | | `putInt24Le` | `putInt24Le` | | `putInt32` | `putInt32` | | `putInt32Le` | `putInt32Le` | | | `putNative` | | `putSignedInt` | `putSignedInt` | | `putString` | `putString` | | `read` | `read` | | `setAt` | `setAt` | | `toHex` | `toHex` | | `toLocaleString` | `toLocaleString` | | `toString` | `toString` | | `truncate` | `truncate` | | `valueOf` | `valueOf` | | | `write` |

That led me to discover that the version of node-jose we are using (1.1.0) actually replaces the node-forge implementation of ByteStringBuffer (via ByteBuffer) with its own DataBuffer prototype: https://github.com/cisco/node-jose/blame/4014f5c2ceb7bae8b516749f6a329132a115bc24/lib/util/databuffer.js#L474

This DataBuffer is not a 1:1 replacement for ByteStringBuffer, and that is what is causing this error scenario. Only certain certificates with the "Basic constraints" extension trigger this code path, and when that happens, node-forge swallows the error and doesn't return the contents of that certificate.

A few things to note:

1. This is a known issue in node-jose that was fixed in the 1.1.4 release but was not mentioned in the changelog.
2. Kibana includes node-jose as a transitive dependency of @elastic/request-crypto.
3. This code path is only triggered in Kibana at runtime because node-jose changes the behavior of the node-forge package before ElasticsearchConfig parses the P12 file. Why are we only starting to see this bug in 7.9+? This isn't a new transitive dependency, we've had it for years. My guess is that one the innumerable changes in our build system caused the node-jose import to happen at a different (earlier) time during the startup process. It is probably a race condition of some sort.
4. The code path to encounter the bug is only triggered if the certificate includes the "Basic constraints" extension: https://github.com/digitalbazaar/forge/blob/c0bb359afca73bb0f3ba6feb3f93bbcb9166af2e/lib/x509.js#L1550 That calls the derToInteger function, which throws the aforementioned error.
5. You can verify the certificate contents/extensions of a P12 file by using the following command:

   keytool -list -v -keystore filename.p12

6. When using keytool to view the X509 attributes of a given P12 file, if a certificate does not have the "PathLen" constraint defined then it may incorrectly print that the extension has a "PathLen" of 2147483647. That appears to be due to a bug that was fixed in JDK 17.

[elasticmachine]

Pinging @elastic/kibana-security (Team:Security)