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)