Private key information leakage in BCRSAPrivateCrtKey#toString

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Private key information leakage in BCRSAPrivateCrtKey#toString

Mark Grandi
As you can see here:

https://github.com/bcgit/bc-java/blob/5fdb1face92f596300323c25cba9fe18726645e8/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/BCRSAPrivateCrtKey.java#L225

if something calls toString() on a BCRSAPrivateCrtKey object, it happily returns the private key material. This is bad, because if something happens to include this object in a logging call, it will print out the private key information to the logs, which might not be as protected as the certificate itself (which might be password protected)

One such thing is the ssl-config project by typesafe, The method, com.typesafe.sslconfig.ssl.CompositeX509KeyManager#getPrivateKey logs what private key its using if DEBUG logging is turned on:

def getPrivateKey(alias: String): PrivateKey = {
  logger.debug(s"getPrivateKey: alias = $alias")
  withKeyManagers { keyManager =>
    val privateKey = keyManager.getPrivateKey(alias)
    if (privateKey != null) {
      logger.debug(s"getPrivateKey: privateKey $privateKey with keyManager $keyManager")
      return privateKey
    }
  }
  null


When using the normal java JCE classes, you get a message like:

 DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: privateKey sun.security.rsa.RSAPrivateCrtKeyImpl@fffd220d with keyManager sun.security.ssl.SunX509KeyManagerImpl@b7a894b


but when using BouncyCastle as the security provider, this same logging statement would print out the entire private key contents to the log

DEBUG com.typesafe.sslconfig.ssl.CompositeX509KeyManager - getPrivateKey: privateKey RSA Private CRT Key
            modulus: ....
    public exponent: ...
   private exponent: ...
             primeP: ...
             primeQ: ...
     primeExponentP: ...
     primeExponentQ: ...
     crtCoefficient: ...

this toString statement is dangerous, and needs to be changed so that it does not leak private key material. 

~Mark
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Private key information leakage in BCRSAPrivateCrtKey#toString

David Hook-3

I wouldn't describe it as a leak - there's nothing subtle about what is going on.

This is in the realm of dammed if you do, dammed if you don't.

If you don't understand why, imagine using:

 DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: privateKey sun.security.rsa.RSAPrivateCrtKeyImpl@fffd220d with keyManager sun.security.ssl.SunX509KeyManagerImpl@b7a894b

To try and work out which public key certificate the private key is likely to match.

There has been a bit of discussion about this "feature" recently though, in terms of trying to find some middle ground, conventional wisdom having shifted over the years.

I am actually leaning heavily towards getting toString() on a private key to print any public information it holds and to also include a fingerprint based on the key encoding so people actually using the information have some way of matching what's in the logs with what is in their keystores, at the same time the fingerprint won't contain enough information to reconstruct the private key in a situation where the consequences of the logging have not been properly recognized.

Does anyone else have an opinion on  this?

Regards,

David

On 25/05/17 06:24, Mark Grandi wrote:
As you can see here:

https://github.com/bcgit/bc-java/blob/5fdb1face92f596300323c25cba9fe18726645e8/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/BCRSAPrivateCrtKey.java#L225

if something calls toString() on a BCRSAPrivateCrtKey object, it happily returns the private key material. This is bad, because if something happens to include this object in a logging call, it will print out the private key information to the logs, which might not be as protected as the certificate itself (which might be password protected)

One such thing is the ssl-config project by typesafe, The method, com.typesafe.sslconfig.ssl.CompositeX509KeyManager#getPrivateKey logs what private key its using if DEBUG logging is turned on:

def getPrivateKey(alias: String): PrivateKey = {
  logger.debug(s"getPrivateKey: alias = $alias")
  withKeyManagers { keyManager =>
    val privateKey = keyManager.getPrivateKey(alias)
    if (privateKey != null) {
      logger.debug(s"getPrivateKey: privateKey $privateKey with keyManager $keyManager")
      return privateKey
    }
  }
  null


When using the normal java JCE classes, you get a message like:

 DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: privateKey sun.security.rsa.RSAPrivateCrtKeyImpl@fffd220d with keyManager sun.security.ssl.SunX509KeyManagerImpl@b7a894b


but when using BouncyCastle as the security provider, this same logging statement would print out the entire private key contents to the log

DEBUG com.typesafe.sslconfig.ssl.CompositeX509KeyManager - getPrivateKey: privateKey RSA Private CRT Key
            modulus: ....
    public exponent: ...
   private exponent: ...
             primeP: ...
             primeQ: ...
     primeExponentP: ...
     primeExponentQ: ...
     crtCoefficient: ...

this toString statement is dangerous, and needs to be changed so that it does not leak private key material. 

~Mark


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Private key information leakage in BCRSAPrivateCrtKey#toString

Mark Grandi
To be fair, ssl-config does log more than just that one line, which makes it easy to see what certificate its dealing with:

2017-05-25 12:32:24,972 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509TrustManager - checkServerTrusted: trustManager sun.security.ssl.X509TrustManagerImpl@74a29b85 using authType ECDHE_RSA found a match for ArraySeq(CN=api-qa3.exampledev.net, OU=Server, OU=CA, O=example, C=CH, CN=Server CA Test 1, OU=CH 005, OU=CA, O=example, C=CH)
2017-05-25 12:32:24,975 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509TrustManager - getAcceptedIssuers:
2017-05-25 12:32:24,976 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509TrustManager - getAcceptedIssuers:
2017-05-25 12:32:25,005 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - chooseEngineClientAlias: keyType = WrappedArray(RSA, DSA, EC), issuers = WrappedArray(CN=Server CA Test 1, OU=CH 005, OU=CA, O=example, C=CH, CN=api-qa3.exampledev.net, OU=Server, OU=CA, O=example, C=CH), engine = 548fcaf1[SSLEngine[hostname=svc-api-qa3.exampledev.net port=443] SSL_NULL_WITH_NULL_NULL]
2017-05-25 12:32:25,006 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - chooseEngineClientAlias: keyType = WrappedArray(RSA, DSA, EC), issuers = WrappedArray(CN=Server CA Test 1, OU=CH 005, OU=CA, O=example, C=CH, CN=api-qa3.exampledev.net, OU=Server, OU=CA, O=example, C=CH), engine = 2013a9[SSLEngine[hostname=svc-api-qa3.exampledev.net port=443] SSL_NULL_WITH_NULL_NULL]
2017-05-25 12:32:25,014 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - chooseEngineClientAlias: using clientAlias tsi10147064 with keyManager sun.security.ssl.SunX509KeyManagerImpl@5cdb374e
2017-05-25 12:32:25,014 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - chooseEngineClientAlias: using clientAlias tsi10147064 with keyManager sun.security.ssl.SunX509KeyManagerImpl@5cdb374e
2017-05-25 12:32:25,015 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getCertificateChain: alias = tsi10147064
2017-05-25 12:32:25,015 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getCertificateChain: alias = tsi10147064
2017-05-25 12:32:25,019 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getCertificateChain: chain ArraySeq(CN=TSI10147064, OU=Server, OU=CA, O=example, C=CH, CN=Server CA Test 1, OU=CH 005, OU=CA, O=example, C=CH) with keyManager sun.security.ssl.SunX509KeyManagerImpl@5cdb374e
2017-05-25 12:32:25,019 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getCertificateChain: chain ArraySeq(CN=TSI10147064, OU=Server, OU=CA, O=example, C=CH, CN=Server CA Test 1, OU=CH 005, OU=CA, O=example, C=CH) with keyManager sun.security.ssl.SunX509KeyManagerImpl@5cdb374e
2017-05-25 12:32:25,020 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: alias = tsi10147064
2017-05-25 12:32:25,021 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: alias = tsi10147064
2017-05-25 12:32:25,022 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: privateKey sun.security.rsa.RSAPrivateCrtKeyImpl@fffd220d with keyManager sun.security.ssl.SunX509KeyManagerImpl@5cdb374e
2017-05-25 12:32:25,023 [partners-akka.actor.default-dispatcher-7] DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: privateKey sun.security.rsa.RSAPrivateCrtKeyImpl@fffd220d with keyManager sun.security.ssl.SunX509KeyManagerImpl@5cdb374e
2017-05-25 12:32:25,416 [partners-akka.actor.default-dispatcher-4] DEBUG c.t.s.ssl.DefaultHostnameVerifier - verify: hostname = svc-api-qa3.exampledev.net, sessionId (base64) = example
2017-05-25 12:32:25,417 [partners-akka.actor.default-dispatcher-4] DEBUG c.t.s.ssl.DefaultHostnameVerifier - verify: hostname = svc-api-qa3.exampledev.net, sessionId (base64) = example
2017-05-25 12:32:25,418 [partners-akka.actor.default-dispatcher-4] DEBUG c.t.s.ssl.DefaultHostnameVerifier - verify: returning true

But having it log a useful toString with public info would be acceptable, my main concern is that toString() gets called automatically in a ton of cases (such as String.format, logging frameworks, etc), so BouncyCastle I feel shouldn't have sensitive information be returned from such a common operation. There could be another method , like toStringFull() that returns what it returns now that someone could call intentionally, or if someone really needs the output, they could just write a helper method to do what toString() does now.

~Mark

On May 25, 2017, at 01:28, David Hook <[hidden email]> wrote:


I wouldn't describe it as a leak - there's nothing subtle about what is going on.

This is in the realm of dammed if you do, dammed if you don't.

If you don't understand why, imagine using:

 DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: privateKey sun.security.rsa.RSAPrivateCrtKeyImpl@fffd220d with keyManager sun.security.ssl.SunX509KeyManagerImpl@b7a894b

To try and work out which public key certificate the private key is likely to match.

There has been a bit of discussion about this "feature" recently though, in terms of trying to find some middle ground, conventional wisdom having shifted over the years.

I am actually leaning heavily towards getting toString() on a private key to print any public information it holds and to also include a fingerprint based on the key encoding so people actually using the information have some way of matching what's in the logs with what is in their keystores, at the same time the fingerprint won't contain enough information to reconstruct the private key in a situation where the consequences of the logging have not been properly recognized.

Does anyone else have an opinion on  this?

Regards,

David

On 25/05/17 06:24, Mark Grandi wrote:
As you can see here:

https://github.com/bcgit/bc-java/blob/5fdb1face92f596300323c25cba9fe18726645e8/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/rsa/BCRSAPrivateCrtKey.java#L225

if something calls toString() on a BCRSAPrivateCrtKey object, it happily returns the private key material. This is bad, because if something happens to include this object in a logging call, it will print out the private key information to the logs, which might not be as protected as the certificate itself (which might be password protected)

One such thing is the ssl-config project by typesafe, The method, com.typesafe.sslconfig.ssl.CompositeX509KeyManager#getPrivateKey logs what private key its using if DEBUG logging is turned on:

def getPrivateKey(alias: String): PrivateKey = {
  logger.debug(s"getPrivateKey: alias = $alias")
  withKeyManagers { keyManager =>
    val privateKey = keyManager.getPrivateKey(alias)
    if (privateKey != null) {
      logger.debug(s"getPrivateKey: privateKey $privateKey with keyManager $keyManager")
      return privateKey
    }
  }
  null


When using the normal java JCE classes, you get a message like:

 DEBUG c.t.s.ssl.CompositeX509KeyManager - getPrivateKey: privateKey sun.security.rsa.RSAPrivateCrtKeyImpl@fffd220d with keyManager sun.security.ssl.SunX509KeyManagerImpl@b7a894b


but when using BouncyCastle as the security provider, this same logging statement would print out the entire private key contents to the log

DEBUG com.typesafe.sslconfig.ssl.CompositeX509KeyManager - getPrivateKey: privateKey RSA Private CRT Key
            modulus: ....
    public exponent: ...
   private exponent: ...
             primeP: ...
             primeQ: ...
     primeExponentP: ...
     primeExponentQ: ...
     crtCoefficient: ...

this toString statement is dangerous, and needs to be changed so that it does not leak private key material. 

~Mark



Loading...