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

Add user:keys:verify command #43646

Merged
merged 1 commit into from Feb 28, 2024
Merged

Conversation

SystemKeeper
Copy link
Contributor

@SystemKeeper SystemKeeper commented Feb 18, 2024

Summary

This occ command checks if the stored public key belongs to the stored private key. In case of a mismatch, some nextcloud functionality is broken (e.g. push proxy registration fails). While this should not happen, it is hard to debug when we end up in such a situation. The command is for verification only, in a next step we could add a second command to update the stored public key.

Checklist

@skjnldsv skjnldsv added the 2. developing Work in progress label Feb 21, 2024
@SystemKeeper SystemKeeper force-pushed the feat/noid/occ-keys-test-command branch 3 times, most recently from d0322ff to 1aa71d4 Compare February 24, 2024 17:11
@nextcloud nextcloud deleted a comment from github-actions bot Feb 24, 2024
@SystemKeeper SystemKeeper marked this pull request as ready for review February 24, 2024 17:14
@SystemKeeper SystemKeeper self-assigned this Feb 24, 2024
@SystemKeeper SystemKeeper added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Feb 24, 2024
@SystemKeeper SystemKeeper requested review from nickvergessen, come-nc, a team, ArtificialOwl and sorbaugh and removed request for a team February 24, 2024 17:14
@SystemKeeper
Copy link
Contributor Author

Doc add at nextcloud/documentation#11580

@SystemKeeper
Copy link
Contributor Author

SystemKeeper commented Feb 24, 2024

I saw that on a production instance for a single user. Is it possible that two requests end up generating a new keypair and one wins to store the private key and one the public key? Otherwise I am not really sure how to end up in that situation. The instance in question uses S3 to store data, in case that makes a difference.

Ref

$folder = $this->appData->getFolder($id);
$folder->newFile('private')
->putContent($this->crypto->encrypt($privateKey));
$folder->newFile('public')
->putContent($publicKey);

Ref

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Good apart from nitpick

core/Command/User/Keys/Test.php Outdated Show resolved Hide resolved
core/Command/User/Keys/Test.php Outdated Show resolved Hide resolved
core/Command/User/Keys/Test.php Outdated Show resolved Hide resolved
core/Command/User/Keys/Test.php Outdated Show resolved Hide resolved
@SystemKeeper SystemKeeper force-pushed the feat/noid/occ-keys-test-command branch 2 times, most recently from 92e56a1 to ccf1331 Compare February 27, 2024 11:30
@SystemKeeper
Copy link
Contributor Author

SystemKeeper commented Feb 27, 2024

Good apart from nitpick

Thanks! Changed as suggested and updated the documentation.

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Shouldn’t that be part of the encryption application actually?

@SystemKeeper
Copy link
Contributor Author

SystemKeeper commented Feb 27, 2024

Since the keypair is handled in server (https://github.com/nextcloud/server/blob/master/lib/private/Security/IdentityProof/Manager.php) and is also used outside of encryption (e.g. for push at https://github.com/nextcloud/notifications/blob/8c752c19903c7db7e7ea766289258cc9f9df2130/lib/Push.php#L320), I don't think so?

Signed-off-by: Marcel Müller <marcel-mueller@gmx.de>
@SystemKeeper SystemKeeper changed the title Add user:keys:test command Add user:keys:verify command Feb 27, 2024
@nickvergessen nickvergessen merged commit 6f95feb into master Feb 28, 2024
160 checks passed
@nickvergessen nickvergessen deleted the feat/noid/occ-keys-test-command branch February 28, 2024 08:20
@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants