Ignore legacy keys when caching keys on startup #510
Conversation
When AcraServer starts and loads all the keys into cache (if the option is enabled), ignore old keys related to AcraConnector. Add new key kind "legacy" and ignore this key if found inside keystore.
|
TODO:
|
| }, nil | ||
| } | ||
|
|
||
| if lastKeyPart == "server" || lastKeyPart == "server.pub" || lastKeyPart == "translator" || lastKeyPart == "translator.pub" { |
There was a problem hiding this comment.
Correct me if I am wrong.
Here, you are checking that if fileInfo contains .pub suffix and you truncate it, right?
https://github.com/cossacklabs/acra/pull/510/files#diff-ca8937b8ce39cdc821c3d9b826aecd30d40cc5beb2aec26ea11497f39e13658fR847
So why are you checking scenarios that lastKeyPart == "server.pub" or lastKeyPart == "translator.pub". Is it not enough to just check lastKeyPart == "server" || lastKeyPart == "translator"
There was a problem hiding this comment.
See these lines above
components := strings.Split(fileInfo.Name(), "_") // note the separator
// ...
lastKeyPart := components[len(components)-1]And checked while debugging that lastKeyPart really contains server.pub or translator.pub when corresponding files exist.
tests/test.py
Outdated
| super().setUp() | ||
|
|
||
| def create_key_file(name): | ||
| open(f"{KEYS_FOLDER.name}/{name}", "w").close() |
There was a problem hiding this comment.
Do we really want to create some test oriented key files inside general KEYS_FOLDER directory. Probably it would be better to create some temporary directory and drop it when test is finished?
Since `testLegacyKeysIgnored` is failing right now, no need to trigger CI yet. Resolving the issue, maybe key files are created in a wrong dir.
tests/test.py
Outdated
|
|
||
| def tearDown(self): | ||
| for key_file in self.legacy_key_files: | ||
| os.remove(f"{KEYS_FOLDER.name}/{key_file}") |
There was a problem hiding this comment.
$ python3 -c 'import os; os.remove("/tlqkeqwleqwlekqweq");'
Traceback (most recent call last):
File "<string>", line 1, in <module>
FileNotFoundError: [Errno 2] No such file or directory: '/tlqkeqwleqwlekqweq'
we should catch exceptions here too or we may fail on first file and leave all remaining files
There was a problem hiding this comment.
Sure, wrapped with try: ... except: pass
Change order of test initialization so the legacy key files will be created and then read by Acra. Now we're sure Acra sees but ignores them, as expected. Wrapped legacy key file deletion with try-catch to avoid cases where the removal will suddenly fail and break more things.
3 participants
When AcraServer/AcraTranslator starts and loads all the keys into cache (if the option is enabled), ignore old keys related to AcraConnector.
Add new key kind "legacy" and ignore this key if found inside keystore.
Checklist
Public API has proper documentation in the Acra documentation site or has PR on documentation repositorywith new changes
CHANGELOG_DEV.md is updatedBenchmark results are attached (if applicable)Example projects and code samples are up-to-date (in case of API changes)