GAM insecurely stored sensitive credentials

[mryerse]

GAM makes use of the local filesystem to store refresh tokens, which is are sensitive credentials, especially considering that the credential typically grants administrative access to messaging environments.

Proposal: make use of keyring so that the tokens are saved into Mac Keychain, Windows credential locker, etc, encrypted with user account control.

[jay0lee]

I'm open to an optional ability to store credentials in keyring. I don't think we can make it required/default as many GAM installations run unattended on servers.

[jay0lee]

PoC Python script that encrypts / decrypts files based on a password. On encrypt we save the results to a .enc file. On decrypt (when a file ending in .enc is specified) we read the encrypted file, decrypt and print it to stdout. This should be capable of handling binary and text files though dumping a binary file to the console obv won't be pretty.

A few thoughts of how GAM might use this

• GAM will prompt for local encrypt/decrypt password before storing oauth2.txt and oauth2service.json files as well as when reading the files.
• Can automatically read password from env variable like GAMPASS that saves multiple re-entries of decrypt password during an admin GAM session.
• password should obviously not be stored in normal config locations
• TBD - can we come up with a way to get a unique ID from a system that we can use as the password for non-interactive GAM commands. This would make the credentials files unique to the machine they ran on.
• All of this would be optional, GAM should still be able to read and use non-encrypted credential files.

@taers232c in case you have thoughts here

import argparse
import os
import sys
from cryptography.fernet import Fernet
from cryptography.hazmat.backends import default_backend
from cryptography.hazmat.primitives import hashes
from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC
import getpass
import base64

def get_fernet(salt):
  kdf = PBKDF2HMAC(
    algorithm=hashes.SHA512_256(),
    length=32,
    salt=salt,
    iterations=100000,
    backend=default_backend()
    )
  key = base64.urlsafe_b64encode(kdf.derive(options.password))
  return Fernet(key)

parser = argparse.ArgumentParser(add_help=False)
parser.add_argument('--file',
    dest='file',
    help='File to encrypt or decrypt')
parser.add_argument('--password',
    dest='password',
    help='Password used to encrypt/decrypt the file')
options = parser.parse_args(sys.argv[1:])

if not options.password:
  options.password = getpass.getpass('Enter the password: ')
options.password = options.password.encode()

with open(options.file, 'rb') as f:
  file_data = f.read()

if options.file.endswith('.enc'):
  # DECODE .enc FILE
  salt = file_data[:16] # first 16 bytes are the salt
  print('Salt is %s' % base64.b64encode(salt))
  encrypted_data = file_data[16:]
  fnet = get_fernet(salt)
  decrypted_data = fnet.decrypt(encrypted_data)
  print(decrypted_data.decode())
else:
  salt = os.urandom(16)
  print('Salt is %s' % base64.b64encode(salt))
  fnet = get_fernet(salt)
  encrypted_file = options.file+'.enc'
  with open(encrypted_file, 'wb') as f:
    f.write(salt)
    f.write(fnet.encrypt(file_data))

[taers232c]

Jay, Interesting thought. - Can automatically read password from env variable like GAMPASS that saves multiple re-entries of decrypt password during an admin GAM session. - password should obviously not be stored in normal config locations - TBD - can we come up with a way to get a unique ID from a system that we can use as the password for non-interactive GAM commands. This would make the credentials files unique to the machine they ran on. How do the first two points relate? If the password is available in an environment variable, that's going to be visible; what would the file be used for in this case? For a unique ID, here's uuid.getnode() but a little searching points out some nagging issues. Ross

…

On Sat, Nov 2, 2019 at 5:19 PM Jay Lee ***@***.***> wrote: PoC Python script that encrypts / decrypts files based on a password. On encrypt we save the results to a .enc file. On decrypt (when a file ending in .enc is specified) we read the encrypted file, decrypt and print it to stdout. This should be capable of handling binary and text files though dumping a binary file to the console obv won't be pretty. A few thoughts of how GAM might use this - GAM will prompt for local encrypt/decrypt password before storing oauth2.txt and oauth2service.json files as well as when reading the files. - Can automatically read password from env variable like GAMPASS that saves multiple re-entries of decrypt password during an admin GAM session. - password should obviously not be stored in normal config locations - TBD - can we come up with a way to get a unique ID from a system that we can use as the password for non-interactive GAM commands. This would make the credentials files unique to the machine they ran on. - All of this would be optional, GAM should still be able to read and use non-encrypted credential files. @taers232c <https://github.com/taers232c> in case you have thoughts here import argparse import os import sys from cryptography.fernet import Fernet from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes from cryptography.hazmat.primitives.kdf.pbkdf2 import PBKDF2HMAC import getpass import base64 def get_fernet(salt): kdf = PBKDF2HMAC( algorithm=hashes.SHA512_256(), length=32, salt=salt, iterations=100000, backend=default_backend() ) key = base64.urlsafe_b64encode(kdf.derive(options.password)) return Fernet(key) parser = argparse.ArgumentParser(add_help=False) parser.add_argument('--file', dest='file', help='File to encrypt or decrypt') parser.add_argument('--password', dest='password', help='Password used to encrypt/decrypt the file') options = parser.parse_args(sys.argv[1:]) if not options.password: options.password = getpass.getpass('Enter the password: ') options.password = options.password.encode() with open(options.file, 'rb') as f: file_data = f.read() if options.file.endswith('.enc'): # DECODE .enc FILE salt = file_data[:16] # first 16 bytes are the salt print('Salt is %s' % base64.b64encode(salt)) encrypted_data = file_data[16:] fnet = get_fernet(salt) decrypted_data = fnet.decrypt(encrypted_data) print(decrypted_data.decode()) else: salt = os.urandom(16) print('Salt is %s' % base64.b64encode(salt)) fnet = get_fernet(salt) encrypted_file = options.file+'.enc' with open(encrypted_file, 'wb') as f: f.write(salt) f.write(fnet.encrypt(file_data)) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#867>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACCTYLZT33ERKZCFN5LJXHTQRYKJTANCNFSM4HAUMVJA> .

-- Ross Scroggs [email protected]

[mryerse]

Any reason to not consider keyring? I suspect some organizations would much prefer they keys be stored in a UAC protected place than a password protected place.

[vhale-kayak]

Has anyone looked into using Hashicorp's Vault for this? It would take a bit of extra setup on the user's part but it's a great free keystore. I'm currently working on a PoC for it myself.

[ghost]

I'm confused as to how this project can distribute dangerous custom installers, curl external servers on startup, and inserts itself as the arbiter of what environment the user is allowed to be using but does not follow the most rudimentary security practices.

@jay0lee, please rethink adding your own credentials management. It's possible to add integrations to keyrings for various environments and I would argue that it's the correct way forward.

This is part of a trend of perplexing decisions regarding security in GAM. Please also consider the following:

• Pip will solve your packaging concerns and bring it in line with more sane distribution practices. I'd love to create a patch for bringing that support in but not if the attitude of this project is to continue supporting the blobs of madness that is the .sh and .bat install scripts as well (as noted in Make GAM pip installable #720).
• Stop checking OS versions. Set supported library versions through package management.
• Respect the FDO standards for Linux (Respect freedesktop's XDG_CONFIG_HOME and XDG_DATA_HOME #791, [Linux] Source configuration/data from standard locations #792) and other OS equivalents so that actually secure methods of computing like SELinux can protect files.

Do you have any considerations of the above? I truly feel that this project would become more secure and well-made if a general shift away from custom implementations were embraced. Removing external library dependencies from the repository was a great step; But GAM can do better.

[daethnir]

@zingyb This sounds more like a rant than a proposal or bug report. I agree with some of your points but the tone detracts from them.

There are different administration capability levels. We pull from git rather than using the curl installer or binary package, for example, but plenty of small shops rely on simplicity. However that doesn't mean that they can't be a valid gam admins. They accept the risk (just as if they downloaded a random FTP client) of not inspecting deeper.

That said, making those tools more secure and reliable is always a good thing for all. set -e is my best friend.

Jay and Ross are always open for MRs. Keep 'em small and focused and the commentary less accusatory and you'll have better luck.

[jay0lee]

It's been some time since I've updated this issue but:

• GAM now supports Google's concept of Default Application Credentials for the service account. To understand how GAM can use this on a platform like Google Compute Engine see https://github.com/GAM-team/GAM/wiki/Running-GAM-on-Google-Compute-Engine-(GCE)-Securely - the advantage here is we don't need to use a RSA private key token that can be stolen, the service account authentication is based on the identity of the machine, not a static key file.
• In the future, I hope to add support for protecting GAM projects and the admin credentials oauth2.txt with Context Aware Access: https://support.google.com/a/answer/7281227?hl=en