CA key and signing key are not really password protected

[John Dennis]

OpenSSL offers two methods to protect sensitive cryptographic keys.

1) standard file system permissions on key file
2) encrypting the key file with a password, i.e. PBE (Password Passed Encryption)

The key management code in keystone/common/openssl.py is confused to to whether it's encrypting keys or not.

The keystone configuration defines a ssl.ca_password and signing.ca_password configuration option. These are defined in keystone/common/config.py and etc/keystone.conf.sample.

However these passwords are never actually used to password protect any of the keys.

The only use of either of the ca_password variables is a no-op when the CA self-signed cert is created in BaseCertificateConfigure.build_ca_cert()

        if not file_exists(ca_cert):
            self.exec_command('openssl req -new -x509 -extensions v3_ca '
                              '-passin pass:%(ca_password)s '
                              '-key %(ca_private_key)s -out %(ca_cert)s '
                             ...

In this instance there is no point in passing in a password to decrypt the ca_private_key file because the ca_private_key file was never encrypted to begin with.

The ca_private_key file is created a few lines above with this code:

            self.exec_command('openssl genrsa -out %(ca_private_key)s '
                              '%(key_size)d')

To have applied PBE (Password Based Encryption) to the ca_private_key the openssl genrsa command would need to have included the -passout arg (i.e. the password to encrypt the output file with, the output file is the key) and the encryption method which is either DES, triple-DES, or IDEA (-des, -des3, or -idea respectively).

The same problems exist with respect to the signing key.

Issue:

The point of protecting a key with a password is to further protect the key (keys are the most security sensitive components, especially both the CA and signing key). But using PBE on the keys currently requires storing the password in a configuration file which can only be protected by file system permissions. Thus using PBE on keys may not offer much additional security if the configuration file containing the key password can be read by an unauthorized user. If an attacker can read the file containing the password they probably also can read the key file or visa-versa. On the other hand one more layer of security doesn't hurt (perhaps the attacker only stole the key file because it's an obvious target but failed to find or obtain the config file with the password needed to extract the key from the key file.

As to which PBE scheme to use? DES is too weak. It's too bad AES is not an option. IDEA is faster and probably stronger than triple-DES (the patent protection on IDEA just recently expired so there shouldn't be any licensing issues)

Summary:

The the presence of key passwords in the configuration options gives the false illusion keys are password protected when in fact they aren't The only use of the password option in the current code is meaningless because it's being used to unlock a key file that was never locked in the first place.

Either the key passwords should be removed (because they aren't actually used) or they should be used consistently to lock and unlock the keys.

The security vulnerability is probably just "do we want to alert folks that the key passwords are not actually used?" and by extension grabbing the key file is sufficient to initiate an attack because you'll have immediate access to the key.

[Thierry Carrez (ttx)]

Nice find!

This is definitely a bug that should be fixed ASAP, but it's a bit in the grey area on the vulnerability side. Yes, people may think their key is more protected than it actually is, but then we are not relying on that "feature" to actually protect the key, and the fix does not provide extra security beyond the obscurity of needing to steal two files (protected with the same rights) instead of one. The ca_password option sounds pretty useless to begin with, so we could probably get rid of it.

I'm wandering on the "no" side, but could easily be convinced otherwise.

[John Dennis (jdennis-a)]

Whatever the resolution is with respect to whether we use passwords on keys or not I can make the fix pretty quickly. You can assign the bug to me and once a consensus is reached I can provide a patch very quickly.

This issue was discovered while we were working on a proof-of-concept using NSS as the crypto provider instead of OpenSSL (blueprint for this will be published shortly). I means I've been all over how certs are created and managed. FWIW NSS also has an (optional) key password. However in the case of NSS there is one password that protects all keys as opposed to OpenSSL for which you can apply a unique password to all key files (or of course you could use the same password on all key files).

If we do remove key passwords from config files I believe key protection is going to come back again as feature in some form, likely in conjunction with hardware based key storage. Exactly how this will be implemented to be generic across a range of devices and interacting with specific trusted processes is an open topic. Even when these devices utilize a password it's clear that password should never be stored in a configuration file. But for the time being, until we start using (hardware or kernel) locked key storage I see little point in storing key passwords in config files, except for ...

If an organization already has an existing key file they want to use and they've encrypted it, then they probably will want to use the encrypted key file, which means they have to put the password somewhere and right now the only place is the config file :-(.

So ...

maybe we can't just get rid of the key password from the config files, but if we keep the password option in the config file then we had better attempt to use it when creating keys instead of ignoring it, which would argue for not removing the key password option but rather keeping it and providing the implementation to use it.

[Thierry Carrez (ttx)]

Agreed, we need to decide on the fix, and we also need to decide on whether we keep this private or not, since the workflow is different (post patch as attachment here vs. push patch to review.o.o). Will keep you posted, might go slowly since we have a few team members on vacation this week :)

[Jeremy Stanley (fungi)]

Handling private key material encryption/decryption safely in unattended systems is tricky. In most cases you wind up making the "key encryption key" readily available to the same parties who already have access to the encrypted version of the key. What are the actual security risks which would be mitigated by fixing this feature (for example, making sure non-encrypted keystone server backups or remote filesystems don't expose keys)? Or is its only real purpose to provide a convenient way of automatically decrypting key material which was encrypted for other reasons (blind policy compliance, et cetera)? It seems likely to me, in most cases, that encrypted copies of the key material and the keystone server configuration containing the decryption passwords are likely to end up being treated with the same degree of security, but perhaps my understanding of keystone in operation is too shallow?

[John Dennis (jdennis-a)]

re comment #4

You are correct on several counts.

Yes, handling key material in unattended systems is tricky. Fixing this bug is not meant to address this issue which really deserves it's own blueprint feature.

Yes, it is likely the PBE password and the key material are likely to be treated the same way, hence the feature is dubious from a security viewpoint.

The primary reason to keep the PBE password feature is for externally provided key material which is already encrypted, possibly because the organizations security policy demands it be encrypted. (Of course the security policy should also address how the PBE password is managed, but we can't make those decisions on behalf of an organization)

Right now the primary issue is the feature is broken because it's not consistently applied. So we either remove the feature, or fix it so it's functional in all code paths.

I suspect you would vote for removing the feature because of it's dubious advantage. I would tend to agree if it weren't for the fact organizational security policies, even if misguided usually must be observed. If a piece of software can't comply with the policy then often deploying the software is forbidden. Policy compliance is the only to keep the feature I can think of.

[Jeremy Stanley (fungi)]

I wouldn't necessarily push for removal if doing so is likely to cause regressions in functionality for deployers. Instead I think it's worthwhile to visibly document this feature for what it is (nothing more than a convenience mechanism), and make it plainly obvious it brings no actual security benefits.

[Jeremy Stanley (fungi)]

And more specifically (as it was the reason for seeking clarity on those questions), since neither fixing this feature nor removing it is going to solve any real-world security vulnerability (only possibly addressing compliance impedance mismatches), I don't believe there's any benefit to our users in keeping this bug report embargoed. I think we should change it to public and bring the OSSG in on notifying the community if they see fit.

[Thierry Carrez (ttx)]

Agreed, will open this bug publicly tomorrow unless someone complains.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/45469

[John Dennis (jdennis-a)]

Adding the commit message from the proposed patch which provides further information:

Keys are generated with the genrsa command. The key is only password
protected if the -des -des3 or -idea argument is provided (encrypted
with DES, triple-DES, or IDEA respectively). The password must be
supplied via the --passout argument otherwise it will be interactively
prompted for.

ISSUE: genrsa is never called with -des, -des3 or -idea therefore it's
never password protected. Nor is the -passout argument
provided. Therefore the ca_password option is pointless. If the
ca_password were actually used then genrsa should have been called
with "-des3 -passout pass:%(ca_password)s".

The key is then provided to the openssl req command. In order for the
req command to be able to read the key the password must be provided
via the -passin argument or it will be interactively prompted for.

ISSUE: -passin is never passed to the req command therefore it cannot
read a passord protected key non-interactively.

The openssl req command will attempt to password protect any key it
output's unless it's passed the -nodes argument. It only generates a
key if a key is not supplied via the -key argument.

ISSUE: The req command used when generating the signing cert is passed
-nodes but since req is not generating a key it has nothing to
encrypt hence it's pointless.

The openssl ca command needs the ca key in order to sign requests. The
ca may be encrypted with a password, if so it will need the ca
password to decrypt the key. The key password is passed via the
-passin argument, if the -passin argument is not provided it will
prompt for it interactively.

ISSUE: The openssl ca command is not passed the key password, if it
did it would need to so like this: "-passin pass:%(ca_password)s".
Since the ca_password is not passed to the ca command it's not
doing anything.

Passwords used by openssl are either prompted for interactively or
obtained non-interactively via a comman line argument, an environment
variable, a file, read from a file descriptor (e.g. pipe) or read from
stdin.

ISSUE: Since OpenStack components are meant to run non-interactively
(at least not requiring admins to type in passwords at random prompts)
we must store the password in the file system, typically inside a
config file. The only thing that protects that password are
DAC (Discretionary Access Control) commonly known as file
system protections and/or MAC (Mandatory Access Control) typically
implemented via SELinux. Since the protection on the key file is not
fundamentally different than the protection on the file containing the
password it's dubious as to what extra protection is being provided by
password protecting the key. In a properly configured system the same
protections should be applied to both the key file and the password
file. If an attacker has access to the one of the files he almost
certainly has access to the other file. This begs the question as to
what actual protection is being afforded via key passwords. In fact an
argument can be made that having to carefully protect configuration
files containing passwords is an additional security item which has to
be verified, something which a...

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/45469
Committed: http://github.com/openstack/keystone/commit/baa57f3efa2c3306b0c100ea95a2bdf935960d2e
Submitter: Jenkins
Branch: master

commit baa57f3efa2c3306b0c100ea95a2bdf935960d2e
Author: John Dennis <email address hidden>
Date: Fri Sep 6 13:38:02 2013 -0400

    Remove CA key password from cert setup

    Password protection in the current implementation is not actually
    applied despite the presence of the ca_password config
    option. Password protection is of marginal benefit because the
    password must be stored in a file using the same protections afforded
    to the key file. Password protection currently is not utilized with
    externally provided keys.

    Remove the ca_password config option and remove the use of the various
    password related options to the OpenSSL commands whose net effect was
    not actually accomplishing anything.

    See the bug report for a thorough description of the issues.

    Change-Id: Iaeb97f2338c4d3c6e770b410dee8f1b62778b561
    Fixes: Bug #1214016