Added --client-signing-algorithms flag by ret2libc · Pull Request #1974 · sigstore/rekor

ret2libc · 2024-01-23T14:50:34Z

Summary

Add --client-signing-algorithms flag to rekor-server to restrict the set of client keys accepted by a Rekor instance. See #1724 .

This work depends on sigstore/sigstore#1601

Release Note

Documentation

codecov · 2024-01-29T12:44:05Z

Codecov Report

Attention: Patch coverage is 50.00000% with 53 lines in your changes missing coverage. Please review.

Project coverage is 49.88%. Comparing base (488eb97) to head (e2e4a9b).
Report is 312 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/api/entries.go	45.83%	31 Missing and 8 partials ⚠️
pkg/api/api.go	50.00%	7 Missing and 5 partials ⚠️
cmd/rekor-server/app/root.go	80.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #1974       +/-   ##
===========================================
- Coverage   66.46%   49.88%   -16.58%     
===========================================
  Files          92      192      +100     
  Lines        9258    24857    +15599     
===========================================
+ Hits         6153    12401     +6248     
- Misses       2359    11341     +8982     
- Partials      746     1115      +369

Flag	Coverage Δ
e2etests	`46.68% <50.00%> (-0.88%)`	⬇️
unittests	`41.77% <7.54%> (-5.92%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

ret2libc · 2024-01-29T14:10:18Z

--- FAIL: TestJAR (0.07s)
    e2e_test.go:33: Using config file:%!(EXTRA string=/tmp/rekor_test.9ofp49.rekor.yaml)
        [POST /api/v1/log/entries][400] createLogEntryBadRequest  &{Code:400 Message:error processing entry: getting verifiers: jar v0.0.1 entry not initialized}
        warning: GOCOVERDIR not set, no coverage data emitted

Apparently the e2e test does not fill the Signature part of the JARModel:

	if v.JARModel.Signature == nil || v.JARModel.Signature.PublicKey == nil || v.JARModel.Signature.PublicKey.Content == nil {
		return nil, errors.New("jar v0.0.1 entry not initialized")
	}

Shall this be handled or is the signature always expected to be present?

Hayden-IO · 2024-02-15T21:40:50Z

Signature should be present, it's required to validate a new jar entry -

rekor/pkg/types/jar/v0.0.1/entry.go

Line 248 in d596e9d

if v.JARModel.Signature == nil || v.JARModel.Signature.Content == nil {

go.mod

cmd/rekor-server/app/root.go

go.mod

Signed-off-by: Riccardo Schirone <[email protected]>

bobcallaway · 2025-02-01T15:12:45Z

pkg/api/entries.go

+	// Default to SHA256 if no artifact hash is specified
+	artifactHashValue := crypto.SHA256
+	// Get artifact hash from entry
+	artifactHash, err := entry.ArtifactHash()


what if err != nil?

refactored this a bit, let me know if it's better!

bobcallaway · 2025-02-01T15:17:52Z

pkg/api/entries.go

+
+	areEntryAlgorithmsAllowed, err := checkEntryAlgorithms(entry)
+	if err != nil {
+		return nil, handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf(validationError, err))
+	}
+	if !areEntryAlgorithmsAllowed {
+		return nil, handleRekorAPIError(params, http.StatusBadRequest, errors.New("entry algorithms are not allowed"), fmt.Sprintf(validationError, "entry algorithms are not allowed"))
+	}
+


this feels like it should be inside types.CreateVersionedEntry instead of in the API layer; wdyt?

Maybe, but it uses api.algorithmRegistry and nothing in pkg/types use api. Would that be fine anyway?

pkg/types/jar/v0.0.1/entry.go

Signed-off-by: Riccardo Schirone <[email protected]>

ret2libc · 2025-02-05T16:29:07Z

pkg/api/entries.go

+	// Only check algorithms for hashedrekord entries
+	switch entry.(type) {
+	case *hashedrekord.V001Entry:
+		break
+	default:
+		return true, nil
+	}
+


@bobcallaway @haydentherapper shall we start with hashedrekord entries only? If we want to add support for other types I guess we need to change the schemas similarly to what we did for hashedrekord.

My 0.02c is that it's fine to start here: my understanding is that the long term plan is to remove a lot of the non-hashed entries with Rekor v2 anyways, plus this flag is primarily for test deployment purposes anyways 🙂.

late reply, but yes, I agree, starting with just hashedrekord is totally reasonable

Signed-off-by: Riccardo Schirone <[email protected]>

…gorithms-flag-2

bobcallaway · 2025-02-06T16:01:01Z

code looks fine. I would like to see some e2e tests that show it works before merging.

ret2libc · 2025-02-07T15:26:07Z

@bobcallaway I added an e2e test, though I don't see it running in the CI. Could it be because it's not in main yet?

bobcallaway · 2025-02-07T15:33:02Z

@bobcallaway I added an e2e test, though I don't see it running in the CI. Could it be because it's not in main yet?

It requires build so it won't show up in the list until that is done since it's a new job.

https://github.com/sigstore/rekor/actions/runs/13202747393/job/36858854058?pr=1974

Signed-off-by: Riccardo Schirone <[email protected]>

ret2libc · 2025-02-10T09:20:00Z

@bobcallaway I added an e2e test, though I don't see it running in the CI. Could it be because it's not in main yet?

It requires build so it won't show up in the list until that is done since it's a new job.

https://github.com/sigstore/rekor/actions/runs/13202747393/job/36858854058?pr=1974

Great! Seems to be green :) Let me know what else I can do to make the PR better/ready

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch from ee1f9ae to be96ecd Compare January 29, 2024 12:38

ret2libc marked this pull request as ready for review January 29, 2024 12:39

ret2libc requested a review from a team as a code owner January 29, 2024 12:39

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch from be96ecd to b450afb Compare January 29, 2024 12:42

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch 3 times, most recently from bffe52e to 55f01c3 Compare January 29, 2024 13:29

ret2libc mentioned this pull request Feb 19, 2024

Create Algorithm Registry API sigstore/sigstore#1601

Merged

ret2libc requested a review from bobcallaway as a code owner February 19, 2024 10:40

bobcallaway reviewed Feb 19, 2024

View reviewed changes

go.mod Outdated Show resolved Hide resolved

cmd/rekor-server/app/root.go Show resolved Hide resolved

go.mod Outdated Show resolved Hide resolved

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch from 2ac074e to 34f31c3 Compare February 29, 2024 11:30

ret2libc requested a review from bobcallaway January 22, 2025 15:43

Add agile client signing algorithms support

c31e028

Signed-off-by: Riccardo Schirone <[email protected]>

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch from 808f8d9 to c31e028 Compare January 30, 2025 10:46

bobcallaway reviewed Feb 1, 2025

View reviewed changes

pkg/api: refactor code and restrict checks to hashedrekord

4062394

Signed-off-by: Riccardo Schirone <[email protected]>

ret2libc requested a review from a team as a code owner February 5, 2025 15:22

ret2libc commented Feb 5, 2025

View reviewed changes

ret2libc added 2 commits February 5, 2025 17:32

remove jar changes

cae7a27

Signed-off-by: Riccardo Schirone <[email protected]>

update go.mod

7a031f8

Signed-off-by: Riccardo Schirone <[email protected]>

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch from 74d6afa to 7a031f8 Compare February 5, 2025 16:33

Merge remote-tracking branch 'origin/main' into add-client-signing-al…

8fe29f9

…gorithms-flag-2

ret2libc requested a review from bobcallaway February 6, 2025 09:30

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch from 4cbdc5d to 2899d57 Compare February 7, 2025 15:20

bobcallaway previously approved these changes Feb 7, 2025

View reviewed changes

Add e2e tests for client-signing-algorithms server flag

e2e4a9b

Signed-off-by: Riccardo Schirone <[email protected]>

ret2libc dismissed bobcallaway’s stale review via e2e4a9b February 7, 2025 15:35

ret2libc force-pushed the add-client-signing-algorithms-flag-2 branch from 2899d57 to e2e4a9b Compare February 7, 2025 15:35

ret2libc requested a review from bobcallaway February 10, 2025 09:19

bobcallaway approved these changes Feb 10, 2025

View reviewed changes

bobcallaway merged commit faae13b into sigstore:main Feb 10, 2025
16 checks passed

woodruffw deleted the add-client-signing-algorithms-flag-2 branch February 11, 2025 20:45

cmurphy mentioned this pull request Mar 14, 2025

Implement hashedrekord algorithm check sigstore/rekor-tiles#100

Closed

cmurphy mentioned this pull request Mar 27, 2025

Add entry hash algorithm config and validation sigstore/rekor-tiles#154

Merged

ret2libc mentioned this pull request Jul 1, 2025

Allow configurable client signing algorithms #1724

Closed

Conversation

ret2libc commented Jan 23, 2024

Summary

Release Note

Documentation

Uh oh!

codecov bot commented Jan 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ret2libc commented Jan 29, 2024

Uh oh!

Hayden-IO commented Feb 15, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bobcallaway Feb 1, 2025

Choose a reason for hiding this comment

Uh oh!

ret2libc Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

bobcallaway Feb 1, 2025

Choose a reason for hiding this comment

Uh oh!

ret2libc Feb 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ret2libc Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

woodruffw Feb 5, 2025

Choose a reason for hiding this comment

Uh oh!

Hayden-IO Feb 11, 2025

Choose a reason for hiding this comment

Uh oh!

bobcallaway commented Feb 6, 2025

Uh oh!

ret2libc commented Feb 7, 2025

Uh oh!

bobcallaway commented Feb 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ret2libc commented Feb 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Jan 29, 2024 •

edited

Loading

bobcallaway commented Feb 7, 2025 •

edited

Loading