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

Changed LoadKeyFromSSLibBytes to Support RSA Keys #60

Merged
merged 5 commits into from
Nov 20, 2023

Conversation

neilnaveen
Copy link
Contributor

  • 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 Support Loading Key for RSA gittuf/gittuf#174
  • Added test for RSA key to SSLibKey for LoadKeyFromSSLibbytes

- 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>
Copy link

@alanssitis alanssitis left a comment

Choose a reason for hiding this comment

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

Also, make sure to run go fmt ./...!

signerverifier/rsa.go Outdated Show resolved Hide resolved
signerverifier/utils_test.go Outdated Show resolved Hide resolved
signerverifier/utils_test.go Outdated Show resolved Hide resolved
Signed-off-by: neilnaveen <42328488+neilnaveen@users.noreply.github.com>
Copy link

@alanssitis alanssitis left a comment

Choose a reason for hiding this comment

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

LGTM, I don't have permissions. @adityasaky may have other comments.

Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

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

Minor nits, but I think this is fine. @mnm678 can you take a look too? Thanks!

signerverifier/rsa.go Show resolved Hide resolved

// LoadRSAPSSKeyFromFile returns an SSLibKey instance for an RSA key stored in a
Copy link
Member

Choose a reason for hiding this comment

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

Can you add some information about the expected encoding of the key in the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have expanded on this, if their is something else you would like to add, tell me

Copy link
Member

Choose a reason for hiding this comment

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

Could you indicate the RSA key is expected to be PEM encoded on disk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@adityasaky
Copy link
Member

Also, looks like go fmt is needed?

Signed-off-by: neilnaveen <42328488+neilnaveen@users.noreply.github.com>
Signed-off-by: neilnaveen <42328488+neilnaveen@users.noreply.github.com>
Signed-off-by: neilnaveen <42328488+neilnaveen@users.noreply.github.com>
@mnm678 mnm678 merged commit 8402a9f into secure-systems-lab:main Nov 20, 2023
13 checks passed
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.

4 participants