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

Lockfiles are not created/checked when loading from keystore cache #5374

Closed
nflaig opened this issue Apr 18, 2023 · 3 comments · Fixed by #5474
Closed

Lockfiles are not created/checked when loading from keystore cache #5374

nflaig opened this issue Apr 18, 2023 · 3 comments · Fixed by #5474
Labels
prio-high Resolve issues as soon as possible. scope-security Issues that fix security issues: DOS, key leak, CVEs.
Milestone

Comments

@nflaig
Copy link
Member

nflaig commented Apr 18, 2023

When loading from keystore cache file we are currently not checking if a .lock for any of the keystores exists and the cache file itself does not have a lock file.

It is not possible to load the cache file is stored in dataDir and if you start a 2nd validator with same dataDir you would get ✖ Error: Database is not open but if a VC has a cache file it will not create lock files which is dangerous.

Originally posted by @nflaig in #5357 (comment)

@philknows philknows added prio-high Resolve issues as soon as possible. scope-security Issues that fix security issues: DOS, key leak, CVEs. labels Apr 23, 2023
@h4l
Copy link

h4l commented Apr 27, 2023

I just discovered this behaviour myself while debugging an unrelated file permission error. A workaround (if running in a container) is to disable the cache by mounting a tmpfs at <dataDir>/cache so that the cached keys are not persisted.

I would guess that the path of the cache could change across versions and shouldn't be relied on though. I could open another issue, but would you be open to having an option to disable the keystore cache? Aside from this workaround, It would be nice to not leave decrypted keys on disk. (I see the code has an option to disable it, but I think it's not possible to control that option via the global options.)

@nflaig
Copy link
Member Author

nflaig commented Apr 27, 2023

We can add a CLI flag to disable the cache but there no disadvantage of having it enabled besides what is mentioned in this issue.

It would be nice to not leave decrypted keys on disk.

There should not be decrypted keys on disk, the keystore cache file is encrypted by combining the password of all keystore files. This also means that if you remove or add a new keystore to importKeystores, the cache file will be invalidated as it can no longer be decrypted.

I could open another issue, but would you be open to having an option to disable the keystore cache?

Let's discuss it here first, I can create an issue but would like to better understand what are the use cases for disabling the keystore cache.

@h4l
Copy link

h4l commented Apr 27, 2023

There should not be decrypted keys on disk, the keystore cache file is encrypted by combining the password of all keystore files.

Ah OK good, I missed that. My concern is invalid then. I don't have a use case for disabling it other than as a workaround for this, so no need to add a flag I think. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio-high Resolve issues as soon as possible. scope-security Issues that fix security issues: DOS, key leak, CVEs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants