-
Notifications
You must be signed in to change notification settings - Fork 547
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
Add trusted-root create helper command #3876
Changes from 7 commits
2552371
ea1c77f
a53ec25
e0041bb
06284e2
b3262d7
cab9148
f705836
b8d58d7
1e7a436
1bd2b08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,70 @@ | ||||
// | ||||
// Copyright 2024 The Sigstore Authors. | ||||
// | ||||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||||
// you may not use this file except in compliance with the License. | ||||
// You may obtain a copy of the License at | ||||
// | ||||
// http://www.apache.org/licenses/LICENSE-2.0 | ||||
// | ||||
// Unless required by applicable law or agreed to in writing, software | ||||
// distributed under the License is distributed on an "AS IS" BASIS, | ||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||
// See the License for the specific language governing permissions and | ||||
// limitations under the License. | ||||
|
||||
package options | ||||
|
||||
import ( | ||||
"github.com/spf13/cobra" | ||||
) | ||||
|
||||
type TrustedRootCreateOptions struct { | ||||
CAIntermediates string | ||||
CARoots string | ||||
CertChain string | ||||
CtfeKeyPath string | ||||
RekorKeyPath string | ||||
Out string | ||||
TSACertChainPath string | ||||
} | ||||
|
||||
var _ Interface = (*TrustedRootCreateOptions)(nil) | ||||
|
||||
func (o *TrustedRootCreateOptions) AddFlags(cmd *cobra.Command) { | ||||
cmd.Flags().StringVar(&o.CAIntermediates, "ca-intermediates", "", | ||||
"path to a file of intermediate CA certificates in PEM format which will be needed "+ | ||||
"when building the certificate chains for the signing certificate. "+ | ||||
"The flag is optional and must be used together with --ca-roots, conflicts with "+ | ||||
"--certificate-chain.") | ||||
_ = cmd.Flags().SetAnnotation("ca-intermediates", cobra.BashCompFilenameExt, []string{"cert"}) | ||||
|
||||
cmd.Flags().StringVar(&o.CARoots, "ca-roots", "", | ||||
"path to a bundle file of CA certificates in PEM format which will be needed "+ | ||||
"when building the certificate chains for the signing certificate. Conflicts with --certificate-chain.") | ||||
_ = cmd.Flags().SetAnnotation("ca-roots", cobra.BashCompFilenameExt, []string{"cert"}) | ||||
|
||||
cmd.Flags().StringVar(&o.CertChain, "certificate-chain", "", | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should all of these options be plural, so that we can provide multiple chains, multiple logs, and multiple TSAs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this point, you can probably anticipate my response : D but I think we should stick to the bare minimum required to get folks off the ground, and let them iterate on top of the basic trusted root we give them. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel less strongly on this one, but I think pluralizing is a small enough change that if we did that now, we'd cover future use cases (e.g. trusting both an internal deployment and public deployment) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This did end up being a pretty minor change! You can now use the flags multiple times to specify additional verification materials. |
||||
"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.") | ||||
_ = cmd.Flags().SetAnnotation("certificate-chain", cobra.BashCompFilenameExt, []string{"cert"}) | ||||
|
||||
cmd.MarkFlagsMutuallyExclusive("ca-roots", "certificate-chain") | ||||
cmd.MarkFlagsMutuallyExclusive("ca-intermediates", "certificate-chain") | ||||
|
||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not seeing a way to provide keys for the CT log service, is that intentional? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort of - I was trying to mirror the flags of commands like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is overridden via an environment variable Line 57 in 780780b
The CTFE key is used for signing, so unfortunately I don't think the flags for verify-blob are a good analog for what needs to be included in the trusted root. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a little tricky because of the diversity of private deployments of Sigstore. I added a |
||||
cmd.Flags().StringVar(&o.CtfeKeyPath, "ctfe-key", "", | ||||
"path to a PEM-encoded public key used by certificate authority for "+ | ||||
"certificate transparency log.") | ||||
|
||||
cmd.Flags().StringVar(&o.RekorKeyPath, "rekor-key", "", | ||||
"path to a PEM-encoded public key used by transparency log like Rekor.") | ||||
|
||||
cmd.Flags().StringVar(&o.Out, "out", "", | ||||
"path to output trusted root") | ||||
|
||||
cmd.Flags().StringVar(&o.TSACertChainPath, "timestamp-certificate-chain", "", | ||||
"path to PEM-encoded certificate chain file for the RFC3161 timestamp authority. Must contain the root CA certificate. "+ | ||||
"Optionally may contain intermediate CA certificates") | ||||
} | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I'm leaning towards keeping this tool more minimal and mirroring the flags of existing |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
// | ||
// Copyright 2024 The Sigstore Authors. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package cli | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/spf13/cobra" | ||
|
||
"github.com/sigstore/cosign/v2/cmd/cosign/cli/options" | ||
"github.com/sigstore/cosign/v2/cmd/cosign/cli/trustedroot" | ||
) | ||
|
||
func TrustedRoot() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "trusted-root", | ||
Short: "Interact with a Sigstore protobuf trusted root", | ||
Long: "Tools for interacting with a Sigstore protobuf trusted root", | ||
} | ||
|
||
cmd.AddCommand(trustedRootCreate()) | ||
|
||
return cmd | ||
} | ||
|
||
func trustedRootCreate() *cobra.Command { | ||
o := &options.TrustedRootCreateOptions{} | ||
|
||
cmd := &cobra.Command{ | ||
Use: "create", | ||
Short: "Create a Sigstore protobuf trusted root", | ||
Long: "Create a Sigstore protobuf trusted root by supplying verification material", | ||
RunE: func(cmd *cobra.Command, _ []string) error { | ||
trCreateCmd := &trustedroot.CreateCmd{ | ||
CAIntermediates: o.CAIntermediates, | ||
CARoots: o.CARoots, | ||
CertChain: o.CertChain, | ||
CtfeKeyPath: o.CtfeKeyPath, | ||
Out: o.Out, | ||
RekorKeyPath: o.RekorKeyPath, | ||
TSACertChainPath: o.TSACertChainPath, | ||
} | ||
|
||
ctx, cancel := context.WithTimeout(cmd.Context(), ro.Timeout) | ||
defer cancel() | ||
|
||
return trCreateCmd.Exec(ctx) | ||
}, | ||
} | ||
|
||
o.AddFlags(cmd) | ||
return cmd | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,211 @@ | ||
// | ||
// Copyright 2024 The Sigstore Authors. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package trustedroot | ||
|
||
import ( | ||
"context" | ||
"crypto" | ||
"crypto/x509" | ||
"encoding/hex" | ||
"encoding/pem" | ||
"fmt" | ||
"os" | ||
|
||
"github.com/sigstore/sigstore-go/pkg/root" | ||
"github.com/sigstore/sigstore/pkg/cryptoutils" | ||
|
||
"github.com/sigstore/cosign/v2/pkg/cosign" | ||
steiza marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
type CreateCmd struct { | ||
CAIntermediates string | ||
CARoots string | ||
CertChain string | ||
CtfeKeyPath string | ||
Out string | ||
RekorKeyPath string | ||
TSACertChainPath string | ||
} | ||
|
||
func (c *CreateCmd) Exec(_ context.Context) error { | ||
var fulcioCertAuthorities []root.CertificateAuthority | ||
ctLogs := make(map[string]*root.TransparencyLog) | ||
var timestampAuthorities []root.CertificateAuthority | ||
rekorTransparencyLogs := make(map[string]*root.TransparencyLog) | ||
|
||
if c.CertChain != "" { | ||
fulcioAuthority, err := parsePEMFile(c.CertChain) | ||
if err != nil { | ||
return err | ||
} | ||
fulcioCertAuthorities = append(fulcioCertAuthorities, *fulcioAuthority) | ||
} else if c.CARoots != "" { | ||
roots, err := parseCerts(c.CARoots) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
var intermediates []*x509.Certificate | ||
if c.CAIntermediates != "" { | ||
intermediates, err = parseCerts(c.CAIntermediates) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
// Here we're trying to "flatten" the x509.CertPool cosign was using | ||
// into a trusted root with a clear mapping between roots and | ||
// intermediates. Make a guess that if there are intermediates, there | ||
// is one per root. | ||
|
||
for i, rootCert := range roots { | ||
var fulcioAuthority root.CertificateAuthority | ||
fulcioAuthority.Root = rootCert | ||
if i < len(intermediates) { | ||
fulcioAuthority.Intermediates = []*x509.Certificate{intermediates[i]} | ||
} | ||
fulcioCertAuthorities = append(fulcioCertAuthorities, fulcioAuthority) | ||
} | ||
} | ||
|
||
if c.CtfeKeyPath != "" { | ||
ctLogPubKey, id, idBytes, err := getPubKey(c.CtfeKeyPath) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
ctLogs[id] = &root.TransparencyLog{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to set validity periods as well? Do we need to take in flags for start time, or default to now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we aren't setting Philisophically, I was thinking of this utility more as "enough of a trusted root to get you started" instead of a flexible tool to cover all use-cases. This will be a recurring theme in my responses! And of course once you have the skeleton of a trusted root file, the user can continue editing it and adding additional content. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed it shouldn't cover all cases, but if we're going to instruct users to go manually inspect it afterwards and update a set of values, providing flags to do that for them seems reasonable. Since validity period is a significant benefit of the trusted root, I'd prefer adding a flag for users to set the start time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the ability to specify the validity start time for keys |
||
HashFunc: crypto.SHA256, | ||
ID: idBytes, | ||
PublicKey: *ctLogPubKey, | ||
SignatureHashFunc: crypto.SHA256, | ||
} | ||
} | ||
|
||
if c.RekorKeyPath != "" { | ||
tlogPubKey, id, idBytes, err := getPubKey(c.RekorKeyPath) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
rekorTransparencyLogs[id] = &root.TransparencyLog{ | ||
HashFunc: crypto.SHA256, | ||
ID: idBytes, | ||
PublicKey: *tlogPubKey, | ||
SignatureHashFunc: crypto.SHA256, | ||
} | ||
} | ||
|
||
if c.TSACertChainPath != "" { | ||
timestampAuthority, err := parsePEMFile(c.TSACertChainPath) | ||
if err != nil { | ||
return err | ||
} | ||
timestampAuthorities = append(timestampAuthorities, *timestampAuthority) | ||
} | ||
|
||
newTrustedRoot, err := root.NewTrustedRoot(root.TrustedRootMediaType01, | ||
fulcioCertAuthorities, ctLogs, timestampAuthorities, | ||
rekorTransparencyLogs, | ||
) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
var trBytes []byte | ||
|
||
trBytes, err = newTrustedRoot.MarshalJSON() | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if c.Out != "" { | ||
err = os.WriteFile(c.Out, trBytes, 0600) | ||
if err != nil { | ||
return err | ||
} | ||
} else { | ||
fmt.Println(string(trBytes)) | ||
} | ||
|
||
return nil | ||
} | ||
|
||
func parsePEMFile(path string) (*root.CertificateAuthority, error) { | ||
certs, err := parseCerts(path) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
var ca root.CertificateAuthority | ||
ca.Root = certs[len(certs)-1] | ||
if len(certs) > 1 { | ||
ca.Intermediates = certs[:len(certs)-1] | ||
} | ||
|
||
return &ca, nil | ||
} | ||
|
||
func parseCerts(path string) ([]*x509.Certificate, error) { | ||
var certs []*x509.Certificate | ||
|
||
contents, err := os.ReadFile(path) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
for block, contents := pem.Decode(contents); ; block, contents = pem.Decode(contents) { | ||
cert, err := x509.ParseCertificate(block.Bytes) | ||
if err != nil { | ||
return nil, err | ||
} | ||
certs = append(certs, cert) | ||
|
||
if len(contents) == 0 { | ||
break | ||
} | ||
} | ||
|
||
if len(certs) == 0 { | ||
return nil, fmt.Errorf("no certificates in file %s", path) | ||
} | ||
|
||
return certs, nil | ||
} | ||
|
||
func getPubKey(path string) (*crypto.PublicKey, string, []byte, error) { | ||
pemBytes, err := os.ReadFile(path) | ||
if err != nil { | ||
return nil, "", []byte{}, err | ||
} | ||
|
||
pubKey, err := cryptoutils.UnmarshalPEMToPublicKey(pemBytes) | ||
if err != nil { | ||
return nil, "", []byte{}, err | ||
} | ||
|
||
keyID, err := cosign.GetTransparencyLogID(pubKey) | ||
if err != nil { | ||
return nil, "", []byte{}, err | ||
} | ||
|
||
idBytes, err := hex.DecodeString(keyID) | ||
if err != nil { | ||
return nil, "", []byte{}, err | ||
} | ||
|
||
return &pubKey, keyID, idBytes, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do cert pools work here? This seems incompatible with the trusted root, as you have to specify a chain.
I see below that you're flattening, assuming 1 intermediate per root and assuming these two lists are ordered. I think that's not always going to be accurate - i would assume a client would throw all their certs into a list without thinking about chain building.
I would recommend we drop these until we support pools in the trust root, and require that clients construct the chains themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially we were trying to mirror other
cosign
flags, but we ended up moving away from that, so how about we specify a single root and a single intermediate file (that can have one or more intermediates)?Again, this is in the spirit of "enough to get you started" and not a do-it-all utility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd lean towards just removing these flags entirely and only have --certificate-chain as an option. if you specify a root and its intermediates separately, you still have to know their order - might as well just use the chain flag at that point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only supporting chains sounds good to me! I updated this pull request to reflect that.