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

(Copy of) private key is stored unencrypted #38

Open
michaz opened this issue Jun 14, 2024 · 4 comments
Open

(Copy of) private key is stored unencrypted #38

michaz opened this issue Jun 14, 2024 · 4 comments

Comments

@michaz
Copy link

michaz commented Jun 14, 2024

Maybe that's intended because we consider local storage secure for purposes of this code.

But it may be surprising in some situations, since the code looks like it wants to use the KeyStore in the user directory, which is password-protected. So one might expect that once I remove my password from the config file, my secret key is safe. Or that when I delete the user directory, my secret key is gone.

However, the secret key is also saved unencrypted in the "serialized User", which is not in the user directory, and the password doesn't seem to have any role at all?

@michaz michaz changed the title Private key is stored unencrypted (Copy of) private key is stored unencrypted Jun 14, 2024
@michaz
Copy link
Author

michaz commented Jun 15, 2024

It looks like different parts of the User class disagree on what the KeyStore is for.

@uwemaurer
Copy link
Collaborator

My plan is to change the way the data is stored. I strongly dislike the Java serialized files, but it was like this when I took over the project and I didn't change it at the time.
I would much prefer file formats which are text based so that you can easily look into them (and maybe make a change directly).

I think we can assume the local storage to be secure, the EBICS library needs to be able to restore the data from the files so even if we encrypt the data then the key needs to be stored somewhere locally too.

Do you have any suggestions how to improve how the data is stored?

@michaz
Copy link
Author

michaz commented Jun 17, 2024

About the keys, I think we can just stop storing them in the User, i.e. remove all the key fields from the User class. The keys are already in the KeyStore. The getters in User can just delegate there.

I think we can assume the local storage to be secure, the EBICS library needs to be able to restore the data from the files so even if we encrypt the data then the key needs to be stored somewhere locally too.

Agreed. Yes, the encryption password for the keystore is in the config file anyway. That's why I was so surprised, because that means there's no reason to store the keys yet another time unencrypted.

And if we don't find the password in the config, maybe because the user removed it, we can even ask them to type it in. There's already a passwordCallback for that. All of that is already there, it just somehow got lost over the generations I think.

If we're lucky the entire User disappears then and we only have the KeyStore and the Config. :-)

@michaz
Copy link
Author

michaz commented Jun 17, 2024

I'll give it a try, with #16 because they kind of go together.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants