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

Jump to bottom

Include checkpoint (STH) in entry upload and retrieve responses #1015

Merged
merged 5 commits into from Sep 1, 2022
Merged

Include checkpoint (STH) in entry upload and retrieve responses #1015

merged 5 commits into from Sep 1, 2022

Conversation

haydentherapper
Copy link
Contributor

This associates a root hash in an inclusion proof with a signed
commitment from the log. Previously, without this included, there was no
connection between an inclusion proof and the log. An inclusion proof
and checkpoint can be an alternative proof of inclusion instead of a
SET.

Ref #988

Signed-off-by: Hayden Blauzvern hblauzvern@google.com

Summary

Release Note

Added a signed checkpoint when requesting an inclusion proof

Documentation

@haydentherapper haydentherapper requested a review from a team as a code owner August 30, 2022 01:38
@haydentherapper
Copy link
Contributor Author

haydentherapper commented Aug 30, 2022 •
edited

Tests are lacking - Not sure what's the best way to test this without significant refactoring. I added a check that a checkpoint is present on rekor-cli verify, and tested locally.

@haydentherapper
Copy link
Contributor Author

@asraa, I can wait until #986 is in to avoid conflicts on the CLI.

@codecov-commenter
Copy link

codecov-commenter commented Aug 30, 2022 •
edited

Codecov Report

Merging #1015 (058fcdb) into main (2081fbb) will decrease coverage by 0.12%.
The diff coverage is 10.11%.

@@            Coverage Diff             @@
##             main    #1015      +/-   ##
==========================================
- Coverage   41.59%   41.47%   -0.13%     
==========================================
  Files          71       71              
  Lines        6933     6980      +47     
==========================================
+ Hits         2884     2895      +11     
- Misses       3748     3781      +33     
- Partials      301      304       +3
Impacted Files Coverage Δ
cmd/rekor-cli/app/get.go 2.45% <0.00%> (-0.09%) ⬇️
cmd/rekor-cli/app/upload.go 3.03% <0.00%> (-0.27%) ⬇️
cmd/rekor-cli/app/verify.go 2.40% <0.00%> (-0.13%) ⬇️
pkg/api/entries.go 0.00% <0.00%> (ø)
pkg/api/tlog.go 0.00% <0.00%> (ø)
pkg/api/trillian_client.go 0.00% <0.00%> (ø)
pkg/verify/verify.go 29.77% <0.00%> (-3.28%) ⬇️
pkg/util/checkpoint.go 71.71% <50.00%> (-4.83%) ⬇️
pkg/types/helm/v0.0.1/entry.go 47.47% <0.00%> (ø)
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@haydentherapper
Copy link
Contributor Author

Sample response from an upload:

{
  "663755961b7b8f25a2134247f5ec4592fc1758c42e7ad158dd0a0cc0d2c1a4c3": {
    "body": {
      "apiVersion": "0.0.1",
      "kind": "rekord",
      "spec": {
        "data": {
          "hash": {
            "algorithm": "sha256",
            "value": "55148d64b214f0b75069feb4429a621fa225b8aabd0a4558bee6769c21d459d8"
          }
        },
        "signature": {
          "content": "MGUCMFHT3GKCpBzp1g+MUY44b4iTUIZlqHx4AY9jCyXtY8FVl3CxBAhQprOsjT4vyD4tUgIxAOBnm689mrROuWYqoU/Qb+hUHtvTvz2dqftKicadY3FVCNyHs/dLe+lNtNa9Lx3VBQ==",
          "format": "x509",
          "publicKey": {
            "content": "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUM0RENDQW1XZ0F3SUJBZ0lVUUQ5a25mblJINFFoMGxFa2lqdE9XTndJSTNRd0NnWUlLb1pJemowRUF3TXcKTnpFVk1CTUdBMVVFQ2hNTWMybG5jM1J2Y21VdVpHVjJNUjR3SEFZRFZRUURFeFZ6YVdkemRHOXlaUzFwYm5SbApjbTFsWkdsaGRHVXdIaGNOTWpJd09ESTVNak13TlRVeFdoY05Nakl3T0RJNU1qTXhOVFV4V2pBQU1IWXdFQVlICktvWkl6ajBDQVFZRks0RUVBQ0lEWWdBRTRjMXZNMWQvV3R6ckhaQThYNVJMNFFZTU15Znk0SVlkWnFGZVF0UzEKQW9LdldHc2dQalhxSDIzTVhDREpOZit6YWNTQUMrZWF6RmpsQmpxaXFuVHJSeHNhNjRod0xocU5McTg4QVZ2bwozTlJyaXI1emc4N0JGTzBVSTlHUGZnMnNvNElCWnpDQ0FXTXdEZ1lEVlIwUEFRSC9CQVFEQWdlQU1CTUdBMVVkCkpRUU1NQW9HQ0NzR0FRVUZCd01ETUIwR0ExVWREZ1FXQkJUbVFMeE40dDAvcXhjTCtsZkVCbm9zb0RhT1N6QWYKQmdOVkhTTUVHREFXZ0JUZjArblBWaVFSbHZtbzJPa29WYUxHTGhoa1B6QkVCZ05WSFJFQkFmOEVPakE0Z1RaMApaWE4wTFdGalkyOTFiblJBYUdKc1lYVjZkbVZ5YmkxMFpYTjBMVEV1YVdGdExtZHpaWEoyYVdObFlXTmpiM1Z1CmRDNWpiMjB3S1FZS0t3WUJCQUdEdnpBQkFRUWJhSFIwY0hNNkx5OWhZMk52ZFc1MGN5NW5iMjluYkdVdVkyOXQKTUlHS0Jnb3JCZ0VFQWRaNUFnUUNCSHdFZWdCNEFIWUFDR0NTOENoUy8yaEYwZEZySjRTY1JXY1lyQlk5d3pqUwpiZWE4SWdZMmIzSUFBQUdDNjlxbElnQUFCQU1BUnpCRkFpRUE3cmhpYlJ1bys2OFRGL1JsRDZVZU9YeURGS3dECkZHanp6UzNsSDFaWnU5VUNJRTJVTzJodnBKWHFuTG00NnhCYllLemQyemdjVy9HcDVFUmszZ1o0MHRlSE1Bb0cKQ0NxR1NNNDlCQU1EQTJrQU1HWUNNUUNaOHRLYVg4V3dPeUw1eEJQVWg2cXVWV3J5di9DSkFjenhkRmlCSG1SKwpZZmUvSytaQU01TnVqam93T1YwaTFkc0NNUUR6N1A4T3hjUUNKUm1MZHNTSjVsVnhqcW9UY1R2c1llakx1VDF4CnlwKzNEd2ttRXp3UGJpNVp5czJyVzFWVGtDUT0KLS0tLS1FTkQgQ0VSVElGSUNBVEUtLS0tLQotLS0tLUJFR0lOIENFUlRJRklDQVRFLS0tLS0KTUlJQ0dqQ0NBYUdnQXdJQkFnSVVBTG5WaVZmblUwYnJKYXNtUmtIcm4vVW5mYVF3Q2dZSUtvWkl6ajBFQXdNdwpLakVWTUJNR0ExVUVDaE1NYzJsbmMzUnZjbVV1WkdWMk1SRXdEd1lEVlFRREV3aHphV2R6ZEc5eVpUQWVGdzB5Ck1qQTBNVE15TURBMk1UVmFGdzB6TVRFd01EVXhNelUyTlRoYU1EY3hGVEFUQmdOVkJBb1RESE5wWjNOMGIzSmwKTG1SbGRqRWVNQndHQTFVRUF4TVZjMmxuYzNSdmNtVXRhVzUwWlhKdFpXUnBZWFJsTUhZd0VBWUhLb1pJemowQwpBUVlGSzRFRUFDSURZZ0FFOFJWUy95c0grTk92dURaeVBJWnRpbGdVRjlObGFyWXBBZDlIUDF2QkJIMVU1Q1Y3CjdMU1M3czBaaUg0bkU3SHY3cHRTNkx2dlIvU1RrNzk4TFZnTXpMbEo0SGVJZkYzdEhTYWV4TGNZcFNBU3Ixa1MKME4vUmdCSnovOWpXQ2lYbm8zc3dlVEFPQmdOVkhROEJBZjhFQkFNQ0FRWXdFd1lEVlIwbEJBd3dDZ1lJS3dZQgpCUVVIQXdNd0VnWURWUjBUQVFIL0JBZ3dCZ0VCL3dJQkFEQWRCZ05WSFE0RUZnUVUzOVBwejFZa0VaYjVxTmpwCktGV2l4aTRZWkQ4d0h3WURWUjBqQkJnd0ZvQVVXTUFlWDVGRnBXYXBlc3lRb1pNaTBDckZ4Zm93Q2dZSUtvWkkKemowRUF3TURad0F3WkFJd1BDc1FLNERZaVpZRFBJYURpNUhGS25meFh4NkFTU1ZtRVJmc3luWUJpWDJYNlNKUgpuWlU4NC85RFpkbkZ2dnhtQWpCT3Q2UXBCbGM0Si8wRHh2a1RDcXBjbHZ6aUw2QkNDUG5qZGxJQjNQdTNCeHNQCm15Z1VZN0lpMnpiZENkbGlpb3c9Ci0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0KLS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUI5ekNDQVh5Z0F3SUJBZ0lVQUxaTkFQRmR4SFB3amVEbG9Ed3lZQ2hBTy80d0NnWUlLb1pJemowRUF3TXcKS2pFVk1CTUdBMVVFQ2hNTWMybG5jM1J2Y21VdVpHVjJNUkV3RHdZRFZRUURFd2h6YVdkemRHOXlaVEFlRncweQpNVEV3TURjeE16VTJOVGxhRncwek1URXdNRFV4TXpVMk5UaGFNQ294RlRBVEJnTlZCQW9UREhOcFozTjBiM0psCkxtUmxkakVSTUE4R0ExVUVBeE1JYzJsbmMzUnZjbVV3ZGpBUUJnY3Foa2pPUFFJQkJnVXJnUVFBSWdOaUFBVDcKWGVGVDRyYjNQUUd3UzRJYWp0TGszL09sbnBnYW5nYUJjbFlwc1lCcjVpKzR5bkIwN2NlYjNMUDBPSU9aZHhleApYNjljNWlWdXlKUlErSHowNXlpK1VGM3VCV0FsSHBpUzVzaDArSDJHSEU3U1hyazFFQzVtMVRyMTlMOWdnOTJqCll6QmhNQTRHQTFVZER3RUIvd1FFQXdJQkJqQVBCZ05WSFJNQkFmOEVCVEFEQVFIL01CMEdBMVVkRGdRV0JCUlkKd0I1ZmtVV2xacWw2ekpDaGt5TFFLc1hGK2pBZkJnTlZIU01FR0RBV2dCUll3QjVma1VXbFpxbDZ6SkNoa3lMUQpLc1hGK2pBS0JnZ3Foa2pPUFFRREF3TnBBREJtQWpFQWoxbkhlWFpwKzEzTldCTmErRURzRFA4RzFXV2cxdENNCldQL1dIUHFwYVZvMGpoc3dlTkZaZ1NzMGVFN3dZSTRxQWpFQTJXQjlvdDk4c0lrb0YzdlpZZGQzL1Z0V0I1YjkKVE5NZWE3SXgvc3RKNVRmY0xMZUFCTEU0Qk5KT3NRNHZuQkhKCi0tLS0tRU5EIENFUlRJRklDQVRFLS0tLS0K"
          }
        }
      }
    },
    "integratedTime": 1661822960,
    "logID": "bb74d288f20e47568e97216e4e5cbed892cc8a4e634b1a7d9c4fcfc90c00c0ac",
    "logIndex": 2,
    "verification": {
      "inclusionProof": {
        "checkpoint": "71ea7e61d1c2 - 2187580196092226528\n3\nPTV9PPDu0TbAAMA+wIQBMztMTsaArMvsfParYzgldi8=\nTimestamp: 1661822960\n\n— 71ea7e61d1c2 u3TSiDBEAiB0cnATML2WLPlX8D2woVhUF6tRVzPkM0o/iC17jSvjeAIgQt6nkFDegIRqmtMaCYnbqFK66Iw1GvJbx4OfyCenmas=\n",
        "hashes": [
          "219bb939b401f04ab9eafe81e99b2e58e9da2ef0f0e4496f2defdd32fb119c95"
        ],
        "logIndex": 2,
        "rootHash": "3d357d3cf0eed136c000c03ec08401333b4c4ec680accbec7cf6ab633825762f",
        "treeSize": 3
      },
      "signedEntryTimestamp": "MEUCIBuwYaQ51NUOczXf/1hEMTUgErapNnlIgJ36T0pZjv06AiEAwA/JXxMWKi6ZQcGIhfWWYHK4/LkSlPJn/yBfJ4kbDZ0="
    }
  }
}```

asraa
asraa reviewed Aug 30, 2022
View reviewed changes
cmd/rekor-cli/app/upload.go Outdated Show resolved Hide resolved
pkg/api/entries.go Outdated Show resolved Hide resolved
pkg/api/entries.go Outdated Show resolved Hide resolved
pkg/api/entries.go Outdated Show resolved Hide resolved
pkg/api/entries.go Outdated Show resolved Hide resolved
pkg/api/entries.go Outdated Show resolved Hide resolved
@haydentherapper
Include checkpoint (STH) in entry upload and retrieve responses
2770fb8 
This associates a root hash in an inclusion proof with a signed
commitment from the log. Previously, without this included, there was no
connection between an inclusion proof and the log. An inclusion proof
and checkpoint can be an alternative proof of inclusion instead of a
SET.

Ref sigstore#988

Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Refactor with common implementation for creating signed checkpoint
c0c8c1c 
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Update client to verify checkpoint signature
5df9779 
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
@haydentherapper
Copy link
Contributor Author

Addressed feedback, there's a lot less code duplication now.

@haydentherapper
Address linter
311a1d3 
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
asraa
asraa suggested changes Sep 1, 2022
View reviewed changes
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! looking great

cmd/rekor-cli/app/upload.go Show resolved Hide resolved
pkg/util/checkpoint.go Outdated Show resolved Hide resolved
pkg/verify/verify.go Show resolved Hide resolved
@haydentherapper
Add checkpoint verification to VerifyLogEntry
058fcdb 
Signed-off-by: Hayden Blauzvern <hblauzvern@google.com>
bobcallaway
bobcallaway approved these changes Sep 1, 2022
View reviewed changes
Copy link
Member

@bobcallaway bobcallaway left a comment

Choose a reason for hiding this comment

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

lgtm

asraa
asraa approved these changes Sep 1, 2022
View reviewed changes
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.

Yay!

@haydentherapper
Copy link
Contributor Author

Thanks y'all, it's ready to merge!

priyawadhwa
priyawadhwa approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@priyawadhwa priyawadhwa left a comment

Choose a reason for hiding this comment

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

nice!!

priyawadhwa
priyawadhwa approved these changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@priyawadhwa priyawadhwa left a comment

Choose a reason for hiding this comment

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

nice!!

@priyawadhwa priyawadhwa merged commit a0c78e7 into sigstore:main Sep 1, 2022
@github-actions github-actions bot added this to the v1.0.0 milestone Sep 1, 2022
@haydentherapper haydentherapper mentioned this pull request Dec 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobcallaway bobcallaway bobcallaway approved these changes

@asraa asraa asraa approved these changes

@priyawadhwa priyawadhwa priyawadhwa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.0.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@haydentherapper @codecov-commenter @bobcallaway @asraa @priyawadhwa