Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interest in adding client-side asymmetric encryption? #83

Open
andrew-d opened this issue Jun 21, 2018 · 14 comments
Open

Interest in adding client-side asymmetric encryption? #83

andrew-d opened this issue Jun 21, 2018 · 14 comments
Assignees
Labels
swift-migration Migrate existing python code to swift.

Comments

@andrew-d
Copy link

Hi there! Thanks for writing this - it's super useful 👍 I've got a branch of this that adds support for client-side asymmetric encryption using swift-sodium - you essentially can specify a public key in the plist and it'll encrypt the FDE key in the output plist to that key.

Is there any interest in merging those changes into this repository? I'm happy to send over a PR with the changes, if so, but I wanted to check first 😃

@grahamgilbert
Copy link
Owner

In theory, but what happens when we want to use the prk locally?

@andrew-d
Copy link
Author

Yeah, this wouldn't let you use it locally - the intent is that this lets you encrypt the key on the client, and then send it to Munkireport/another backend without needing to secure the entire flow from the client -> backend.

OOC, what cases are there where we would use the key locally? I'll admit to being somewhat unfamiliar with the specifics of FDE on OS X - I'm helping out another team at work with this stuff.

@weswhet
Copy link
Collaborator

weswhet commented Jun 21, 2018

One reason for reading the key is to validate that the key actually works to unlock the drive. @andrew-d if you could open a PR I'd like to look at the code.

@andrew-d
Copy link
Author

Sure, here's a comparison:
master...andrew-d:andrew/libsodium

Sorry for the massive diff; I used CocoaPods to add the dependency (and checked it in for local testing). The bulk of the code lives in FDEEncryptor.swift, which is responsible for doing the encryption; the rest of the change is just calling it at the right time when writing to the output .plist.

Also: good point on the key validation. If the approach looks good, I can add some error-checking that ensure you don't have both ValidateKey and the encryption key specified?

@grahamgilbert
Copy link
Owner

We would probably need to check on the rotate key pref as well (and possibly others as well, I’ll need to look when not mobile).

@mactroll
Copy link

This is how the PRK gets stashed on the disk for MDM to pick up already... so it's quite fitting for Crypt to do the same.

FWIW, if all you're looking to do is to encrypt the file with a public cert you can probably bail on libsodium.

   myErr = SecIdentityCopyCertificate(id, &cert)
    
    // build an encoder
    
    myErr = CMSEncodeContent(nil, cert, nil, false, CMSSignedAttributes.init(rawValue: 0), CFDataGetBytePtr(fileData as! CFData), CFDataGetLength(fileData as! CFData), &encFileRaw)

Would be easy to do something like this:

  1. push CryptServer public cert via configuration profile
  2. use that cert with CMSEncodeContent() to encrypt the raw PSK output plist, like you're doing

I'm not sure it would have to be either/or, as in either ValidateKey or EncryptKeyLocally but would seem strange to have both at the same time I guess. Although if you're only using the encryption to better handle the transport of the key to the CryptServer, via Lambda or something perhaps, then it would sensible to have both at the same time.

Happy to help with the encryption routine.

@groob
Copy link

groob commented Jun 22, 2018

nacl is much nicer than pkcs7 :)

I'm interested in this being available as an option as well.

@andrew-d
Copy link
Author

Yeah, I considered going down the certificate route, but the nice part about using libsodium is that it's relatively self-contained; you don't need to push a config profile, work with the keychain, etc. - and decryption is literally:

#!/usr/bin/env python

# Install instructions:
#   - Create a new virtualenv and activate it
#   - Install libnacl in the virtualenv (`pip install libnacl`)
#
# Usage:
#   python decrypt.py SECRET-KEY ENCRYPTED-VALUE

import sys
import libnacl.sealed
import libnacl.public

# Load the keypair
keypair = libnacl.public.SecretKey(sk=sys.argv[1].decode('hex'))

# Decrypt message
box = libnacl.sealed.SealedBox(keypair)
bclear = box.decrypt(sys.argv[2].decode('hex'))

print("FV key: %s" % (bclear,))

@groob
Copy link

groob commented Jun 22, 2018

@andrew-d wanted to get your thoughts on this question: Why build it directly into crypt instead of doing it in the escrow script that picks up the key from disk?

@weswhet
Copy link
Collaborator

weswhet commented Jun 22, 2018

I'm assuming because he doesn't want it sitting on the disk unencrypted from time between write and escrow.

@grahamgilbert
Copy link
Owner

I assume you have a accompanying PR to add support for handling these in Crypt Server?

@andrew-d
Copy link
Author

andrew-d commented Jun 22, 2018

Why build it directly into crypt instead of doing it in the escrow script that picks up the key from disk?

I'm assuming because he doesn't want it sitting on the disk unencrypted from time between write and escrow.

Yeah, this is pretty much it; I'd like to keep it off the disk if at all possible.

I assume you have a accompanying PR to add support for handling these in Crypt Server?

I don't have one written yet, no, though I'm happy to try and throw something together if you'd like! We'll be using Munkireport for storing FDE keys, and we're going to store them encrypted unless we need to decrypt them - the encryption key will be kept offline.

@grahamgilbert
Copy link
Owner

That would be lovely - feel free to hop into #crypt on MacAdmins slack (macadmins.org) and we can talk about this a bit more real time.

@weswhet
Copy link
Collaborator

weswhet commented May 25, 2023

We have an internal fork here at Stripe for this. I would like to stop forking this so I believe moving this all to the keychain would be a better solution here. More info in #115

@weswhet weswhet self-assigned this May 25, 2023
@weswhet weswhet added the swift-migration Migrate existing python code to swift. label May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
swift-migration Migrate existing python code to swift.
Projects
None yet
Development

No branches or pull requests

5 participants