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

Unable to read & update package header outside of rpm crate #80

Open
TommyLike opened this issue Feb 7, 2023 · 16 comments
Open

Unable to read & update package header outside of rpm crate #80

TommyLike opened this issue Feb 7, 2023 · 16 comments
Assignees

Comments

@TommyLike
Copy link
Contributor

Background
I am trying to do a remote pgp sign by utilizing this rpm package, but it seems the header and signature functions are public to crate only, how can we read&update these sections outside of rpm package?

package.metadata.header.**write**(&mut header_bytes)?;
package.metadata.signature = Header::<IndexSignatureTag>::**new_signature_header**();
@TommyLike TommyLike changed the title Unable to read package header outside of rpm crate Unable to read & update package header outside of rpm crate Feb 7, 2023
@TommyLike
Copy link
Contributor Author

@drahnr @dralley could you take a look?

@dralley
Copy link
Collaborator

dralley commented Feb 20, 2023

There is a sign() method on Package which is public, is that not usable in your case?

@TommyLike
Copy link
Contributor Author

@dralley yeah, what I need is to split the content and do a remote signature then assemble them back into signature.

@drahnr
Copy link
Contributor

drahnr commented Feb 21, 2023

So from what I understand, you send the hash to a remote entity which ows your signing key rather than signing it locally?

I'd probably recommend to implement a custom Signer

pub trait Signing<A>: Debug
where the actual signing is deferred, so
hash_alg: ::pgp::crypto::hash::HashAlgorithm::SHA2_256,
would be invoked on the remote with None rather than SHA256 and you'd have to calculate the hash locally, send it to your signing entity and return it.

@dralley
Copy link
Collaborator

dralley commented Mar 1, 2023

@TommyLike I imagine the idea here is to implement obs-sign, or something like it, on your own - yes? That makes sense. A custom client perhaps?

@TommyLike
Copy link
Contributor Author

TommyLike commented Mar 2, 2023

@drahnr yeah, I am trying to create rust version of obs sign with more efficient, secure and support more key and file types..

@drahnr
Copy link
Contributor

drahnr commented Mar 2, 2023

But then the method of choice should be to implement the mentioned traits. If you have trouble with that, or the API is insufficient, please open another issue :)

I am not sure there is anything to do given the generality of the issue at hand?

@TommyLike
Copy link
Contributor Author

But then the method of choice should be to implement the mentioned traits. If you have trouble with that, or the API is insufficient, please open another issue :)

I am not sure there is anything to do given the generality of the issue at hand?

the question is in order to sign multiple rpms concurrently we seperated the signing process/trait into three different workers: split file->do remote sign->assemble signature in which we can't simply impl them all in this trait.

@drahnr
Copy link
Contributor

drahnr commented Mar 2, 2023

'worker' meaning individual vms/nodes/machines? And that cannot be changed/re-partitioned?

My point is: you don't need to split, you don't need to assemble. All you need to do is sign the thing. If pkg content is living in a DB, then we'd need more info.

But to be frank, it feels like you're trying to form the cold end of the iron, or you need to provide a whole lot more info to make it reasonable to add such an API (a PR would be nice)

@dralley
Copy link
Collaborator

dralley commented Mar 2, 2023

@drahnr I believe the basic idea of obs-sign (which I am not fully familiar with, so anyone please feel free to correct me) that you have a client which opens the RPM and makes a checksum of the relevant bits. The client can then pass only this checksum to an external service which manages the signing. The service then returns a signature which the client can inject into the RPM.

This approach avoids uploading the RPM contents themselves and keeps the signing key management contained to a single designated machine, which is easier to lock down.

Imagine a build system where you have multiple build servers. Putting the signing keys on each individual build server opens up more attack surface and opportunity for those keys to be compromised. You could probably dump all the unsigned RPMs onto a shared NFS drive, or upload full RPMs to a signing service, but maybe that involves too much data movement and/or potential for needlessly sharing sensitive data? So instead you just pass around the checksum.

@TommyLike Is that a good summary?

@TommyLike
Copy link
Contributor Author

TommyLike commented Mar 2, 2023

@drahnr I believe the basic idea of obs-sign (which I am not fully familiar with, so anyone please feel free to correct me) that you have a client which opens the RPM and makes a checksum of the relevant bits. The client can then pass only this checksum to an external service which manages the signing. The service then returns a signature which the client can inject into the RPM.

This approach avoids uploading the RPM contents themselves and keeps the signing key management contained to a single designated machine, which is easier to lock down.

Imagine a build system where you have multiple build servers. Putting the signing keys on each individual build server opens up more attack surface and opportunity for those keys to be compromised. You could probably dump all the unsigned RPMs onto a shared NFS drive, or upload full RPMs to a signing service, but maybe that involves too much data movement and/or potential for needlessly sharing sensitive data? So instead you just pass around the checksum.

@TommyLike Is that a good summary?

@dralley exactly 😄 , much appreciated for your explanation.

@drahnr
Copy link
Contributor

drahnr commented Mar 2, 2023

x ---{ digest }--> y
x <--{ signature }-- y

where x is the location/machine where the rpm is stored, and y is the location/machine the signature is created.

Replacing the signature is doable, the Header<Sign...> is a public field in https://docs.rs/rpm/latest/rpm/struct.RPMPackageMetadata.html - which can built with a signature builder.

Now all you need is digest of the relevant pieces. The code is https://docs.rs/rpm/latest/src/rpm/rpm/builder.rs.html#470-493 and could be copied or be made public (assuming we'd add a few more types to improve type safety).

Technically, there is nothing stopping you from implementing all of this in a type that implements Signer (running on x), given sufficient flexibility of your existing codebase.

If you need further help, then you'd need to provide a whole lot more context on the things you can change, and the constraints you have. Otherwise, that's as much support I can give around using rpm-rs.

I hope this helps

@TommyLike
Copy link
Contributor Author

@drahnr sure

  1. this is the changes I made from official rpm-rs project: master...TommyLike:rpm-rs:master
  2. and this is how we utilize the updated rpm-rs in our demo project:
    https://github.com/TommyLike/signatrust/blob/main/src/client/file_handler/rpm.rs check the split_data and assemble_data functions

@dralley
Copy link
Collaborator

dralley commented Mar 4, 2023

@TommyLike If you do plan to work on this could you take it as assigned?

@TommyLike
Copy link
Contributor Author

@dralley sure, will create PR when ready.

@dralley
Copy link
Collaborator

dralley commented May 5, 2024

@TommyLike In #225 I'm removing all of the implementation that uses legacy signatures and digests, which includes the header + payload ones. That simplifies the problem quite a bit.

I think what you should be able to do is pull the sha256 checksum of the header out of the signature header, and then sign it, then build your new signature header with the sha256 checksum and the signature.

But if you like, I can look at making a function available to just serialize the header so that you can generate whatever checksum you want

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

No branches or pull requests

3 participants