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

feat(keys): JSON unmarshal hardening. #275

Merged

Conversation

Zenithar
Copy link
Contributor

@Zenithar Zenithar commented Apr 22, 2022

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Description of the changes being introduced by the pull request:

  • Add io.LimitReader to block public key content larger than 512Kb.
  • Fix Ed25519 signer constructor to be referenced as ECDSASigner.
  • Rename NewEd25519Signer to NewEd25519SignerFromKey to support
    the signer registry registration and prevent double declaration. (breaking-change)
  • Add ECDSA unmarshaller tests.
  • Add crypto related tests on unmarshalling (private / public key association).

Please verify and check that the pull request fulfills the following requirements:

  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@ethan-lowman-dd
Copy link
Contributor

Fixes #258

pkg/keys/ecdsa.go Outdated Show resolved Hide resolved
@Zenithar Zenithar force-pushed the feat_key_json_unmarshaller_hardening branch from 5c25a92 to b383cc5 Compare April 22, 2022 15:35
pkg/keys/ed25519_test.go Outdated Show resolved Hide resolved
pkg/keys/ed25519_test.go Outdated Show resolved Hide resolved
pkg/keys/ecdsa_test.go Outdated Show resolved Hide resolved
pkg/keys/ed25519.go Outdated Show resolved Hide resolved
pkg/keys/ed25519.go Outdated Show resolved Hide resolved
pkg/keys/ed25519.go Outdated Show resolved Hide resolved
Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this!

data/types.go Outdated Show resolved Hide resolved
pkg/keys/ecdsa.go Outdated Show resolved Hide resolved
pkg/keys/ecdsa.go Outdated Show resolved Hide resolved
pkg/keys/ecdsa_test.go Outdated Show resolved Hide resolved
pkg/keys/ed25519.go Outdated Show resolved Hide resolved
@Zenithar Zenithar force-pushed the feat_key_json_unmarshaller_hardening branch 3 times, most recently from 08c4916 to 3085d4b Compare June 28, 2022 06:15
@Zenithar
Copy link
Contributor Author

Zenithar commented Jun 28, 2022

Branch rebased to master - e3efe98

asraa
asraa previously approved these changes Jun 29, 2022
@asraa
Copy link
Contributor

asraa commented Jun 29, 2022

One more quick check? @mnm678 @joshuagl

go.mod Outdated Show resolved Hide resolved
pkg/keys/ed25519.go Show resolved Hide resolved
* Add io.LimitReader to block public key content larger than 512Kb.
* Fix Ed25519 signer constructor to be referenced as ECDSASigner.
* Rename `NewEd25519Signer` to `NewEd25519SignerFromKey` to support
  the signer registry registration and prevent double declaration.
* Add ECDSA unmarshaller tests.

Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
Signed-off-by: Thibault Normand <me@zenithar.org>
@Zenithar Zenithar dismissed stale reviews from asraa and trishankatdatadog via a2635a0 July 1, 2022 08:28
@Zenithar Zenithar force-pushed the feat_key_json_unmarshaller_hardening branch from d5af22b to a2635a0 Compare July 1, 2022 08:28
@Zenithar
Copy link
Contributor Author

Zenithar commented Jul 1, 2022

Branch rebased to master - 0d40b25

Copy link
Member

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

go.mod Show resolved Hide resolved
go.sum Show resolved Hide resolved
@joshuagl joshuagl merged commit 4febe4c into theupdateframework:master Jul 20, 2022
@trishankatdatadog
Copy link
Member

Great job, thanks Thibault, and Marina/Joshua for reviewing!

@Zenithar Zenithar deleted the feat_key_json_unmarshaller_hardening branch July 21, 2022 08:50
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.

None yet

6 participants