Password Encryption // Master Password

[Theo D.]

Hi everyone,

I've just managed to make my patch working so I share it with you: ​https://github.com/tdubourg/filezilla-encrypted

I've been using Filezilla for a long time now but I have always been concerned about the really low level of security it provided regarding to password storage - that is to say, no security (clear text). On Windows there are even now malwares that have been targeting Filezilla sitemanager.xml file....

So as I spare had time 2 days ago I decided to make a patch.

Here is what currently works:

• Turning on password encryption in the settings window and entering the master password.
• Storing the master password (encrypted using itself as password key) in the FileZilla.xml options
• Turning off password encryption...
• Starting filezilla in password encrypted mode and entering the master password at the start up of the application (if the user does not want to enter any password then FileZilla will close).
• Building on GNU/Linu x86

I use the Crypto++ cross platform library which has integrated packages for most GNU/Linux distributions and has a non-viral license.

What I would need help for, in order to finalize this patch:

• Explanations about the build process of FileZilla. I added a class called CCrypto (following the current naming conventions in the source code) and for now I edited manually the makefiles to get it building but that's just a quick-dirty-fix and we have to put the right things in the right place for autoconf and libtool to recognize everything. Same for the -lcryptopp option. I need to know how to add this new dependancy.

• Light help on handling password fields. For now I use text fields and this not very pretty :)

• Information about possible edge cases that come as a consequence of the changes I made (in terms of dealing with password in filezilla, I'd need to know if there are some tricky part of filezilla dealing with passwords that I should have a look at, to make sure the patch is compatible)

What still needs to be developped:

• Converting the password when changing the master password (decrypting them with the former one and encrypting them again with the new one).
• Decrypting the passwords when the user turns off the encryption

For now these two cases obviously end up with encrypted passwords that are not decryptable (because no master password or master password has changed).

What could be improved : Randomly generating a salt / iv (instead of hardcoded ones) when turning on encryption and then storing them in the settings file.

I am using AES by the way.

This patch is not intended to provide military security. It is intended to provide standard security that should be implemeted in every software storing password (like Firefox, Thunderbird, Opera... and even WinSCP (that has a CRAP UI but master password feature !!!).

I do not provide a patch file for now but you can see the changes I made by searching for the string "@td". That's a tag that I put where I make changes.

BTW I do not know if I can branch the SVN repo instead of holding the stuff on Github...? I think I need additional rights for that.

[Alexander Schuch]

This patch refers mostly to #2935.

[Theo D.]

Hi,

I have not managed to find that feature requests when I tried to see if there was any work in progress on that feature on filezilla in the bugtacker... Certainly because I was looking for "Password Enryption" instead of "Master password" (geeky way of thinking)) and certainly also because I was not looking at closed thread... for such an important feature.

I saw the flamewar going on there...I don't think there is any point about redoing this war in this thread so I will just ask:

Does that mean I cannot hope for any help from the dev. team of FileZilla because they are convinced the devil is inside that feature?

Or is there a hope that they may accept the patch and provide light help / feedback to improve it, taking advantage of not having to do it by themselves?

(I say "they" but as far as I saw, I've only seen one dev. ... )

Thanks

[Alexander Schuch]

The linking is just to make navigating around easier.

You can talk to "codesquid" in IRC. See ​http://wiki.filezilla-project.org/Support for information.

[Theo D.]

Hi,

Yes yes, I know the linking was not to intended to say anythong positive/negative. I was just asking.

I'll try to catch Codesquid, thanks. Putting him as cc on this thread might help too, no?

[Theo D.]

I've discussed with Codesquid on IRC. Who did not tell me anything about the feature by the way but gave me some (very) small directions about how to integrate the new stuff into the build system.

I've updated the Github repo and deleted from the repo most of the autoconf-generated but some files may still remain. I also corrected two bugs and improved the patch during the day. I've been using this version of FileZilla without problems for now...

I will provide asap a clean patch diff file so that it is reviewable :) But for that I need to re-checkout the SVN repo and my current Internet connection might not allow me to do that!

[Theo D.]

First version of a clean patch.

[Theo D.]

Hi everyone,

So, I've had a bit of additional work on the patch.

I fixed a couple of bugs, and more important, I added the feature of "changing" the master password (decrypting and re-encrypting the stored passwords when changing the master password).

The patch currently builds and runs without any problem on my computer (32bits Ubuntu).

I've integrated the new features in the FZ build system so that it can detect the new dependency and see if it is missing (libcrypto++).

I am uploading the clean patch now so that you can review it. I'll wait for feedback from you before dealing with the last part of what will need to be implented : Dealing with exports and imports (but some choices have to be made : how to we allow the user to do that, when do we prompt for the password... I have my idea about that but would like to get feedback from FZ devs first).

[Tim Kosse]

Some comments on the patch:

• crypto.h and crypto.cpp are missing
• Lots of comments in the diff indicating changed places. Please remove them, the diff alone is enough to see the changes
• Does import/export functionality work?
• There's no indication in the XML for a site whether the password is plain or encrypted. How do you distinguish between the two?
• What about the transfer queue? Also passwords in that but inside of a SQLite database.
• Kiosk mode 2 where writing to settings or sites is completley disabled. Doesn't seem to be handled.
• What if writing to settings.xml succeeds but sitemanager.xml cannot be written to for some reason? (or vice-versa)
• What if one of the two files is lost?

I think information whether a given password is encrypted or not, in addition to individual salt/IV for each password need to be part of the saved information, individually for every password.

[Theo D.]

Hi,

Thanks for the comments!

• I did not notice those files were missing, going to upload a new patch with them ASAP.
• Yep, I use these "tags" to organize myself when changing only some bits a big existing codebase. I've never seen anyone having problems with "comments" (instead of having issues with a lack of them) but I'll remove them if necessary.
• import/export : Actually, it depends. Depends on what you export/import and if you have the same master password in the original and the target FileZilla but I would say that generally speaking: No. It should _not_ crash or anything like that but it will just not be able to read the passwords if the master password is not set or is different on the target FileZilla. I was actually thinking of just exporting in clear text (decrypting passwords when exporting). That would make things easier I guess.
• What do I have to know about the transfer queue? Is there anything special? A SQLite DB, what for? Could you explain a bit that question?
• Actually, kiosk mode 2 is I think currently compatible. If nothing is written to the files, then, nothing is written, that's all. Might need additional testing on that.
• If writing to settings work but not to sitemanager.xml then we will have corrupted passwords, the same as before: you can't write to that file, then you don't have up-to-date data... In the other case, though, if the master password is not saved but the passwords are well encrypted, then the passwords will be read as non encrypted one though they are, and thus will be kind of corrupted. Do you have any suggestion about that?
• Same as before?

I don't think we need the information whether a password is encrypted or not because the "master password" feature is a global feature (well, as always, that's the idea behind "Master" password!) so either you have _everything_ encrypted or you have _nothing_ encrypted.

I currently don't generate a different salt for each password, I just always use the same (which is not the best way, but implied less changes - no additional data to be stored along with the "server" information).

The idea behind this implementation of the patch is that it is "transparent". Nothing in the way we interact with the objects in the code has changed, there's just an additional layer of embedding, that encrypts or decrypts or does not do anything. I think adding the salt information to the server would be good, but I am afraid it would involve changing some things in the code that is loading / saving those servers and I don't know FileZilla code enough for being sure that I will update _all_ those places.

If you could help me on that, then it would be much easier, of course.

Thanks for the feedback, waiting for your answer on that :) And I'll upload the patch with the missing 2 files ASAP.

[Theo D.]

First version of a clean patch (with missing files)

[Theo D.]

Hi,

Sorry for being so long to come back. Had totally forgotten about the two missing files.

I have just attached the patch with the missing file and the @td comments removed.

I am now waiting for review / feedback from you, following the my last comment's answer in particular.

[Tim Kosse]

I had another look at the patch. Things still missing:

• Import and export needs to work correctly instead of silently failing to decrypt passwords. Ideally it should ask on export whether to export encrypted or plain. On import it needs to detect mismatch.
• Changing the master password corrupts existing queues: Start FZ 2 times. Add 100 files in the first instance, close it. Change the master password in instance 2. Close it. Start FZ again and the queue items' passwords can no longer be decrypted.
• The password should only be asked from the user when actually attempting to decrypt a password, akin to the "Ask for password" logon type.
• Saving of passwords should not require the password to be entered. Can be easily done via assymetric encryption, where the secret key is protected via the password.
• There already is a crypto library in use by FileZilla, albeit transitively via GnuTLS. Please use nettle instead of crypto++, this avoids needlessly adding another dependency.