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: move_ent with signed binpkg #1043

Closed
wants to merge 1 commit into from
Closed

Conversation

syu-nya
Copy link
Contributor

@syu-nya syu-nya commented May 21, 2023

Signed-off-by: Sheng Yu syu.os@protonmail.com

@mgorny
Copy link
Member

mgorny commented May 23, 2023

I'm confused. Is there an actual reason we can't update signed binpkgs or is this a "placeholder" until someone implements that?

@thesamesam
Copy link
Member

thesamesam commented May 23, 2023

Doesn't it invalidate the binpkg signature if we're a client (and not the signer)? (Am I missing something silly?)

(we might have added something in the format for this and I forgot?)

@syu-nya
Copy link
Contributor Author

syu-nya commented May 23, 2023

I'm confused. Is there an actual reason we can't update signed binpkgs or is this a "placeholder" until someone implements that?

We can update signed binpkgs but for most of users they should not to do that. And without a configured portage it will create a broken binpkg, since part of the contents is not signed.

And for configured portage, the current implementation is not good, because the move action is done at start, which prevents users from unlocking GPG normally and could cause broken binpkg.

@mgorny
Copy link
Member

mgorny commented May 23, 2023

Well, the spec basically assumed we're be signing updated binpkgs with our own key. Ofc, if you're a user using remote repo and don't have a local signing key configured, then yes, you can't sign it. But then, I'm not sure if we should reject the update or just drop outer and metadata signatures.

@syu-nya
Copy link
Contributor Author

syu-nya commented May 23, 2023

This only reject to update the signed binpkg, not the db or other metadata.
User who do not control the signing should re-download from remote and not touch the signed binpkg.

@syu-nya
Copy link
Contributor Author

syu-nya commented May 23, 2023

Maybe it should be a FEATURES, not default behavior.

@mgorny
Copy link
Member

mgorny commented May 23, 2023

Do I understand correctly that this applies to binpkgs that were already downloaded in the past and reside in local directory?

Unless I'm missing something, we should really update them, dropping the signatures if necessary. IMO the primary reason for package signing is to verify binpkgs fetched from remote host, and once they were verified once, I don't think it's paramount to preserve the signatures and verify them again.

Ofc this implies that Portage must be able to recognize these updated packages and not reject them.

@syu-nya
Copy link
Contributor Author

syu-nya commented May 23, 2023

Do I understand correctly that this applies to binpkgs that were already downloaded in the past and reside in local directory?

Yes.

Unless I'm missing something, we should really update them, dropping the signatures if necessary. IMO the primary reason for package signing is to verify binpkgs fetched from remote host, and once they were verified once, I don't think it's paramount to preserve the signatures and verify them again.

In this case dropping the signatures seems reasonable. But under the current implementation we cannot easy distinguish whether the binpkg was downloaded from a remote or modified locally.

@thesamesam
Copy link
Member

This sounds like the PR is ok as it is, but we should file a bug and ideally work on sorting that in future?

@thesamesam
Copy link
Member

This sounds like the PR is ok as it is, but we should file a bug and ideally work on sorting that in future?

Sorry, I think I've changed my mind.

I think we need to:

  1. Delete signed binpkgs on the consumer side if they're needing a move.
  2. Make the producer side handle re-signing for updates.

And in future, maybe work on better separating local vs fetched binpkgs?

What do you think?

@syu-nya
Copy link
Contributor Author

syu-nya commented May 27, 2023

1. Delete signed binpkgs on the _consumer_ side if they're needing a move.

Not sure if we want to delete that or wait eclean to do that.

2. Make the _producer_ side handle re-signing for updates.

This is on plan, but it needs complex change to unlock gpg before move.

And in future, maybe work on better separating local vs fetched binpkgs?

Not sure TBH

@thesamesam
Copy link
Member

1. Delete signed binpkgs on the _consumer_ side if they're needing a move.

Not sure if we want to delete that or wait eclean to do that.

I think we should because they're invalid in a different way.

Could you do that part now, then we'll circle back to 2) later?

@syu-nya
Copy link
Contributor Author

syu-nya commented Aug 22, 2023

1. Delete signed binpkgs on the _consumer_ side if they're needing a move.

Not sure if we want to delete that or wait eclean to do that.

I think we should because they're invalid in a different way.

Could you do that part now, then we'll circle back to 2) later?

Sure

The gpkg file that cannot be updated will be removed.

Signed-off-by: Sheng Yu <syu.os@protonmail.com>
Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@gentoo-bot gentoo-bot closed this in a7bbb4f Dec 8, 2023
@thesamesam
Copy link
Member

Thank you!

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