Closed dtitov closed 5 years ago
Thank you for raising this issue! -- it's a very good point, and your suggestions make a lot of sense to me.
I'll play around with it a bit over the next few weeks, and see if I can put together an update that:
char[]
PBESecretKeyDecryptor
where possible (or maybe even the extracted PGPPrivateKey
itself)Subkey
class that zeroes-out the passphrase (and any other secret data the Subkey
has stored)Key key = new Key(new File("path/to/alice.asc"));
Decryptor decryptor = new Decryptor(key);
char[] passphrase = ... // load this as a char[] from some source
try {
key.setPassphrase(passphrase);
decryptor.decrypt(
new File("path/to/ciphertext.txt.gpg"),
new File("path/to/plaintext.txt")
);
} finally {
key.clearSecrets(); // new method to zero-out passphrase
}
Thanks for the quick response. Sounds like a plan!
Ping me if you need any assistance, I would gladly help.
I coded up some changes as PR #20, trying to avoid breaking the existing use of passphrases as String
s, but enabling passphrases as char[]
s to be used without too much additional awkwardness -- please take a look when you have some time, and let me known what you think. These changes:
char[]
and stored via a new passphraseChars
property on the Subkey
class (and via convenience methods on the Key
class); and also allows the PGPPrivateKey
object to be extracted without storing the passphrase at all, via a new unlock()
method. If the passphrase is supplied as a char[]
via the passphraseChars
property, no copies are made -- so zeroing-out the original char[]
wipes the passphrase from memory.PGPPrivateKey
in the Subkey
class -- which should at least be an improvement over the current behavior, where a separate copy of the PGPPrivateKey
is created every time a separate message is signed or decrypted.clearSecrets()
method to the Subkey
class (and all the other principal classes), zeroing-out the stored passphrase char[]
, and releasing the stored PGPPrivateKey
for garbage collection.char[]
to the Encryptor
and Decryptor
classes, via new symmetricPassphraseChars
properties (analogous to the passphraseChars
property on a Subkey
).I also wrote up some changes to the wiki pages, including updating the section on setting passphrases to document how to supply the passphrase as a char[]
, and how to clean up the passphrases in memory once decryption/signing is done. I put these changes in the passphrase-storing
branch of the wiki -- Github doesn't seem to have a web UI for viewing wiki branches, but you can at least see the diff online here:
Thanks again for your help @dtitov! -- I merged #20 into master, and released v0.5 with the change. I also merged those wiki updates in -- the main section for documenting how best to use the new API methods is here: https://github.com/justinludwig/jpgpj/wiki/KeyRings#cleaning-up-memory
Thanks! :)
Hi! Very nice library, but we've discovered a potential security issue: passphrase for the private key is stored as
String
, which is a bit insecure.I would suggest not just replacing the type of
passphrase
field in theSubkey
class fromString
tochar[]
, but rather not storing passphrase at all: it should be possible to store builtPBESecretKeyDecryptor
instead. Or even not store anything at all and just ask for achar[]
each time upon decryption.What do you think? Would it be possible to implement such a change?