search

GAM-team / GAM

command line management for Google Workspace
https://github.com/GAM-team/GAM/wiki
Apache License 2.0
3.54k stars 473 forks source link

GAM insecurely stored sensitive credentials #867

Open mryerse opened 5 years ago

mryerse commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

@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 commented 5 years ago

Jay,

Interesting thought.

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 notifications@github.com 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 https://github.com/jay0lee/GAM/issues/867?email_source=notifications&email_token=ACCTYL3RQDUFRY6L4YSP3F3QRYKJTA5CNFSM4HAUMVJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEC5H5FY#issuecomment-549093015, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACCTYLZT33ERKZCFN5LJXHTQRYKJTANCNFSM4HAUMVJA .

-- Ross Scroggs ross.scroggs@gmail.com

mryerse commented 5 years ago

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 commented 4 years ago

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 commented 4 years ago

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:

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 commented 4 years ago

@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 commented 1 month ago

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