Support Loading Key for RSA #174
Conversation
neilnaveen
commented
Nov 6, 2023
neilnaveen
commented
- Support loading RSA keys into a securesystemlib key for LoadKeyFromBytes.
- Support loading RSA keys into a securesystemlib key for LoadKeyFromBytes. Signed-off-by: neilnaveen <42328488+neilnaveen@users.noreply.github.com>
|
@adityasaky What should the Scheme be for the RSA key? |
|
Hi @neilnaveen, thanks for trying to address the FIXME. I'm half wondering if we ought to get rid of the function altogether and leave it to go-securesystemslib. Note that once upon a time, tuf.Key wasn't just an alias to the go-securesystemslib key definition, and we needed this method. WDYT? Also see: https://github.com/secure-systems-lab/go-securesystemslib/blob/main/signerverifier/rsa.go#L97-L141. This is for a signerverifier specifically but general key loading functions can also be supported there I suspect. |
|
On a separate note, we ought to go back and open issues to track some of the TODO / FIXME items in the codebase. It's a good idea to take stock of them again so that there's space for discussion before a bunch of work and time is put into a solution that may not be applicable anymore. 🤔 |
We should probably use this https://github.com/secure-systems-lab/go-securesystemslib/blob/8c0fa8d1979fda745dae5325708417965522313a/signerverifier/utils.go#L27-L45. The only problem is that this is not a exported func. I am thinking about creating a PR to make this an exported func, and push a version of these changes I have made in this PR to go-securesystemslib, for that function. |
That doesn't handle PEM encoded RSA keys, we'd have the same issue. That said, I think we generally need to solve this in go-sslib rather than here. in-toto, TUF etc. could then benefit as well. cc @mnm678 |
Do you mean that this should be fixed directly in go-sslib, for this function and we should use that function instead of LoadKeyFromBytes. If so, I was meaning that I would push a PR to go-sslib to add this feature there, and then use that function instead of LoadKeyFromBytes here. |
- Changed LoadKeyFromSSLibBytes to support RSA keys - Seperated LoadRSAPSSKeyFromFile into helper function LoadRSAPSSKeyFromBytes so that data can be passed as a byte array or as a file name. - Made LoadRSAPSSKeyFromBytes into an exported function to use in gittuf/gittuf#174 - Added test for RSA key to SSLibKey for LoadKeyFromSSLibbytes Signed-off-by: neilnaveen <42328488+neilnaveen@users.noreply.github.com>
|
Just created a PR for this in go-sslib secure-systems-lab/go-securesystemslib#60 |
|
Thanks, I'll review it but I'm at KubeCon so I may be a bit delayed. I don't know that we can use it right away either because we use a separate branch for sigstore compat right now. I'll leave the PR open so we can repurpose it later for this if you're still interested in tracking the issue. |
|
@adityasaky Now that this secure-systems-lab/go-securesystemslib#60 has been merged in, should I implement these changes in this PR? If so, would you like to use LoadKeyFromBytes as a wrapper for https://github.com/secure-systems-lab/go-securesystemslib/blob/8402a9fff48716051a7ca8e9737907b978d08157/signerverifier/utils.go#L30C1-L30C1, or totally replace LoadKeyFromBytes with LoadKeyFromSSLibBytes from go-sslib |
|
Not quite yet. 1) We use go-sslib off a custom branch at the moment for sigstore support. 2) We need to cut a release with the RSA key change you submitted. We need to reconcile both of those to see if we pull in the sigstore WIP changes into the release as well. |
|
There's a new go-sslib version that we're using for gittuf too, so this should be unblocked. |
|
I've lost context on the motivation for this one though. |
|
#257 and some upstream changes to go-securesystemslib means that this is handled beyond just the RSA case. We won't be using the custom format for any key type for much longer except inline in metadata. Thanks for working on this and on go-securesystemslib! |
