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

Fix acra-keys list with keystore v1 #530

Merged
merged 4 commits into from
Apr 19, 2022

Conversation

G1gg1L3s
Copy link
Contributor

@G1gg1L3s G1gg1L3s commented Apr 19, 2022

This PR fixes issues with acra-keys that were discovered at cossacklabs/product-docs#245 (comment).

The changes:

  • keystore.ListKeys now lists files only once, if public key path is the same as private key one.
  • in log key names secure is no longer interpreted as client id

Checklist

The v1 keystore can have two paths for public and private keys.
But most often they are the same. The `ListKeys` function of keystore
doesn't check if these paths are the same, and if yes, enumerates all
keys twice.

Change this by checking the paths.
The filename is `secure_log_key` and it's parsed as a regular client
key, which consists of `<client_id>_<purpose>_<kind>`. So in case of
`secure_log_key` the client_id is `secure, which is not correct.

Fix this by setting empty array as a ClientId.
[skip ci]
keystore/filesystem/server_keystore.go Show resolved Hide resolved
}
}

// copied from `acra-keys list` code
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the good reason to move this function to keystore/utils.go (probably) and use it in tests and acra-tools too, if we already need it in several places) Maybe in the future we will need it in one more place

Removes code duplication.
Copy link
Collaborator

@Lagovas Lagovas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

G1gg1L3s added a commit to cossacklabs/product-docs that referenced this pull request Apr 19, 2022
@G1gg1L3s G1gg1L3s merged commit baf7577 into master Apr 19, 2022
@G1gg1L3s G1gg1L3s deleted the G1gg1L3s/T2535-fix-acra-keys-list-with-keystore-v1 branch April 19, 2022 15:16
vixentael pushed a commit to cossacklabs/product-docs that referenced this pull request Apr 20, 2022
* Replace v2 with v1 in some examples

* Fix acra-keys list table

The result of cossacklabs/acra#530.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants