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.

Sign up for GitHub

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

Add trusted-root create helper command #3876

Merged
merged 11 commits into from
Oct 29, 2024
Merged

Add trusted-root create helper command #3876

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2552371
Fixes #3700: add trusted-root create helper command
steiza Sep 11, 2024
ea1c77f
Linter fixes and docgen
steiza Sep 11, 2024
a53ec25
Fix Windows unit test
steiza Sep 11, 2024
e0041bb
Output via stdout instead of stderr
steiza Sep 12, 2024
06284e2
Add ctlogs to `cosign trusted-root create`
steiza Sep 12, 2024
b3262d7
Replace `--rekor-url` with `--ignore-tlog`
steiza Sep 12, 2024
cab9148
Just use paths to files on disk
steiza Sep 26, 2024
f705836
Add the ability to supply multiple verification material
steiza Oct 8, 2024
b8d58d7
Don't panic if there's unexpected content in PEM file
steiza Oct 17, 2024
1e7a436
remove trailing newline
steiza Oct 17, 2024
1bd2b08
Simplify imports
steiza Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/cosign/cli/options/trustedroot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (o *TrustedRootCreateOptions) AddFlags(cmd *cobra.Command) {
"path to a list of CA certificates in PEM format which will be needed "+
"when building the certificate chain for the signing certificate. "+
"Must start with the parent intermediate CA certificate of the "+
"signing certificate and end with the root certificate. Conflicts with --ca-roots and --ca-intermediates.")
Copy link
Contributor

Choose a reason for hiding this comment

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

@why do we remove the notice about conflicting flags? When I run cosign verify built from this branch with both --ca-roots file.pem and --certificate-chain file.crt, I'm getting the following error:

Error: if any flags in the group [ca-roots certificate-chain] are set none of the others can be; [ca-roots certificate-chain] were all set
main.go:74: error during command execution: if any flags in the group [ca-roots certificate-chain] are set none of the others can be; [ca-roots certificate-chain] were all set

so the "mutual exclusion" of these flags is still there - why then the change of the flags documentation, here and also in doc/cosign_trusted-root_create.md?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, so cosign verify supports both --certificate-chain and --ca-roots, but later we decided that cosign trusted-root create would only support --certificate-chain. There isn't a --ca-roots option for cosign trusted-root create anymore.

Copy link
Contributor

@dmitris dmitris Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

would --certificate-chain support PEM files with multiple chains, or only a single one? If you have a PEM bundle with a dozen "top-level" self-signed CA root certificates, would you be able to use cosign trusted-root create to create a trusted root JSON file or not? Would one had to "manually" split that bundle into multiple PEM files, one per certificate (or certificate chain)? Can you use --certificate-chain multiple times?

As mentioned on Slack , if you currently try to pass a PEM bundle with multiple CA root certificates as --certificate-chain, the generated JSON file is incorrect, or at least not what one would expect: it puts all of them in the same chain, with one certificate being the root, and the rest - Intermediates, even though they are all self-signed roots.
The same issue is with parsing a timestamp certificate bundle with multiple certificates.

If these are use cases that we do not intend to support, let's explicitly state it in the documentation.

"signing certificate and end with the root certificate.")
_ = cmd.Flags().SetAnnotation("certificate-chain", cobra.BashCompFilenameExt, []string{"cert"})

cmd.Flags().StringArrayVar(&o.CtfeKeyPath, "ctfe-key", nil,
Expand Down
2 changes: 1 addition & 1 deletion cmd/cosign/cli/trustedroot/trustedroot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func parseCerts(path string) ([]*x509.Certificate, error) {
return nil, err
}

for block, contents := pem.Decode(contents); ; block, contents = pem.Decode(contents) {
for block, contents := pem.Decode(contents); block != nil; block, contents = pem.Decode(contents) {
cert, err := x509.ParseCertificate(block.Bytes)
if err != nil {
return nil, err
Expand Down
9 changes: 7 additions & 2 deletions cmd/cosign/cli/trustedroot/trustedroot_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
// Make some certificate chains
td := t.TempDir()

fulcioChainPath := filepath.Join(td, "fulcio.crt")
fulcioChainPath := filepath.Join(td, "fulcio.pem")
makeChain(t, fulcioChainPath, 2)

tsaChainPath := filepath.Join(td, "timestamp.crt")
tsaChainPath := filepath.Join(td, "timestamp.pem")
makeChain(t, tsaChainPath, 3)

outPath := filepath.Join(td, "trustedroot.json")
Expand Down Expand Up @@ -73,7 +73,8 @@
if len(timestampAuthorities[0].Intermediates) != 2 {
t.Fatal("unexpected number of timestamp intermediate certificates")
}

}

Check failure on line 77 in cmd/cosign/cli/trustedroot/trustedroot_test.go

View workflow job for this annotation

GitHub Actions / lint 

unnecessary trailing newline (whitespace)

func makeChain(t *testing.T, path string, size int) {
fd, err := os.Create(path)
Expand Down Expand Up @@ -120,6 +121,10 @@
}
err = pem.Encode(fd, block)
checkErr(t, err)

// Ensure we handle unexpected content at the end of the PEM file
_, err = fd.Write([]byte("asdf\n"))
checkErr(t, err)
}

func checkErr(t *testing.T, err error) {
Expand Down
2 changes: 1 addition & 1 deletion doc/cosign_trusted-root_create.md
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading