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

Switch to calling gRPC-based fulcio v2 APIs #1762

Closed
wants to merge 18 commits into from
Closed

Switch to calling gRPC-based fulcio v2 APIs #1762

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
0a5c2bc
move to using gRPC interface of fulcio
bobcallaway Apr 14, 2022
f77123c
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Apr 14, 2022
849a2cb
add TLS support if https requested
bobcallaway Apr 14, 2022
00ce9be
ran hack/update-codegen.sh
bobcallaway Apr 14, 2022
198e72e
handle default port case
bobcallaway Apr 15, 2022
abd9991
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Apr 15, 2022
e06f099
update to latest fulcio API
bobcallaway Apr 30, 2022
26809be
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Sep 27, 2022
927a7f2
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Nov 2, 2022
fd89a89
remove 3p license files
bobcallaway Nov 2, 2022
c4a2bff
change the way fulcio URL is parsed
bobcallaway Nov 7, 2022
8911066
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Nov 7, 2022
bec1727
fix lint error
bobcallaway Nov 7, 2022
88a2b75
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Nov 8, 2022
c143e25
bump version of scaffolding to test grpc
bobcallaway Nov 9, 2022
163bf2a
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Nov 9, 2022
92458c4
deal with http URL with insecure flag
bobcallaway Nov 10, 2022
2cdefb5
Merge remote-tracking branch 'upstream/main' into fulcio_grpc
bobcallaway Nov 10, 2022
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
6 changes: 3 additions & 3 deletions cmd/cosign/cli/attest/attest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ type tlogUploadFn func(*client.Rekor, []byte) (*models.LogEntryAnon, error)
func uploadToTlog(ctx context.Context, sv *sign.SignerVerifier, rekorURL string, upload tlogUploadFn) (*cbundle.RekorBundle, error) {
var rekorBytes []byte
// Upload the cert or the public key, depending on what we have
if sv.Cert != nil {
rekorBytes = sv.Cert
if sv.Cert != "" {
rekorBytes = []byte(sv.Cert)
} else {
pemBytes, err := sigs.PublicKeyPem(sv, signatureoptions.WithContext(ctx))
if err != nil {
Expand Down Expand Up @@ -157,7 +157,7 @@ func AttestCmd(ctx context.Context, ko sign.KeyOpts, regOpts options.RegistryOpt
}

opts := []static.Option{static.WithLayerMediaType(types.DssePayloadType)}
if sv.Cert != nil {
if sv.Cert != "" {
opts = append(opts, static.WithCertChain(sv.Cert, sv.Chain))
}

Expand Down
80 changes: 58 additions & 22 deletions cmd/cosign/cli/fulcio/fulcio.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,22 @@ import (
"crypto/ecdsa"
"crypto/rand"
"crypto/sha256"
"crypto/tls"
"crypto/x509"
"fmt"
"net/url"
"os"

"github.com/pkg/errors"
"golang.org/x/term"
"google.golang.org/grpc"
"google.golang.org/grpc/credentials"
"google.golang.org/grpc/credentials/insecure"

"github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioroots"
clioptions "github.com/sigstore/cosign/cmd/cosign/cli/options"
"github.com/sigstore/cosign/pkg/cosign"
"github.com/sigstore/fulcio/pkg/api"
fulciopb "github.com/sigstore/fulcio/pkg/generated/protobuf"
"github.com/sigstore/sigstore/pkg/cryptoutils"
"github.com/sigstore/sigstore/pkg/oauthflow"
"github.com/sigstore/sigstore/pkg/signature"
)
Expand All @@ -55,8 +59,8 @@ func (rf *realConnector) OIDConnect(url, clientID, secret, redirectURL string) (
return oauthflow.OIDConnect(url, clientID, secret, redirectURL, rf.flow)
}

func getCertForOauthID(priv *ecdsa.PrivateKey, fc api.Client, connector oidcConnector, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string) (*api.CertificateResponse, error) {
pubBytes, err := x509.MarshalPKIXPublicKey(&priv.PublicKey)
func getCertForOauthID(ctx context.Context, priv *ecdsa.PrivateKey, fc fulciopb.CAClient, connector oidcConnector, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string) (*fulciopb.SigningCertificate, error) {
pubPEM, err := cryptoutils.MarshalPublicKeyToPEM(&priv.PublicKey)
if err != nil {
return nil, err
}
Expand All @@ -73,19 +77,28 @@ func getCertForOauthID(priv *ecdsa.PrivateKey, fc api.Client, connector oidcConn
return nil, err
}

cr := api.CertificateRequest{
PublicKey: api.Key{
Algorithm: "ecdsa",
Content: pubBytes,
cscr := &fulciopb.CreateSigningCertificateRequest{
Key: &fulciopb.CreateSigningCertificateRequest_PublicKeyRequest{
PublicKeyRequest: &fulciopb.PublicKeyRequest{
PublicKey: &fulciopb.PublicKey{
Algorithm: fulciopb.PublicKeyAlgorithm_ECDSA,
bobcallaway marked this conversation as resolved.
Show resolved Hide resolved
Content: string(pubPEM),
},
ProofOfPossession: proof,
},
},
Credentials: &fulciopb.Credentials{
Credentials: &fulciopb.Credentials_OidcIdentityToken{
OidcIdentityToken: tok.RawString,
},
},
SignedEmailAddress: proof,
}

return fc.SigningCert(cr, tok.RawString)
return fc.CreateSigningCertificate(ctx, cscr)
}

// GetCert returns the PEM-encoded signature of the OIDC identity returned as part of an interactive oauth2 flow plus the PEM-encoded cert chain.
func GetCert(ctx context.Context, priv *ecdsa.PrivateKey, idToken, flow, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string, fClient api.Client) (*api.CertificateResponse, error) {
func GetCert(ctx context.Context, priv *ecdsa.PrivateKey, idToken, flow, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string, fClient fulciopb.CAClient) (*fulciopb.SigningCertificate, error) {
c := &realConnector{}
switch flow {
case FlowDevice:
Expand All @@ -99,18 +112,18 @@ func GetCert(ctx context.Context, priv *ecdsa.PrivateKey, idToken, flow, oidcIss
return nil, fmt.Errorf("unsupported oauth flow: %s", flow)
}

return getCertForOauthID(priv, fClient, c, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL)
return getCertForOauthID(ctx, priv, fClient, c, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL)
}

type Signer struct {
Cert []byte
Chain []byte
Cert string
Chain []string
SCT []byte
pub *ecdsa.PublicKey
*signature.ECDSASignerVerifier
}

func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string, fClient api.Client) (*Signer, error) {
func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string, fClient fulciopb.CAClient) (*Signer, error) {
priv, err := cosign.GeneratePrivateKey()
if err != nil {
return nil, errors.Wrap(err, "generating cert")
Expand All @@ -131,17 +144,23 @@ func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClien
default:
flow = FlowNormal
}
Resp, err := GetCert(ctx, priv, idToken, flow, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL, fClient) // TODO, use the chain.
resp, err := GetCert(ctx, priv, idToken, flow, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL, fClient) // TODO, use the chain.
bobcallaway marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, errors.Wrap(err, "retrieving cert")
}

f := &Signer{
pub: &priv.PublicKey,
ECDSASignerVerifier: signer,
Cert: Resp.CertPEM,
Chain: Resp.ChainPEM,
SCT: Resp.SCT,
}
switch csc := resp.Certificate.(type) {
case *fulciopb.SigningCertificate_SignedCertificateDetachedSct:
f.SCT = csc.SignedCertificateDetachedSct.SignedCertificateTimestamp
f.Cert = csc.SignedCertificateDetachedSct.Chain.Certificates[0]
f.Chain = csc.SignedCertificateDetachedSct.Chain.Certificates[1:]
case *fulciopb.SigningCertificate_SignedCertificateEmbeddedSct:
f.Cert = csc.SignedCertificateEmbeddedSct.Chain.Certificates[0]
f.Chain = csc.SignedCertificateEmbeddedSct.Chain.Certificates[1:]
}

return f, nil
Expand All @@ -161,11 +180,28 @@ func GetIntermediates() *x509.CertPool {
return fulcioroots.GetIntermediates()
}

func NewClient(fulcioURL string) (api.Client, error) {
func NewClient(fulcioURL string) (fulciopb.CAClient, error) {
var opts []grpc.DialOption
fulcioServer, err := url.Parse(fulcioURL)
if err != nil {
return nil, err
}
fClient := api.NewClient(fulcioServer, api.WithUserAgent(clioptions.UserAgent()))
return fClient, nil
host := fulcioServer.Host
switch fulcioServer.Scheme {
case "https":
Copy link
Contributor

Choose a reason for hiding this comment

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

I've also seen dns:///foo.example.com:443 as a supported scheme for gRPC.

I also think we should allow scheme-less and match on port.

So either:

  • scheme=https
  • port=443
    should use a TLS connection

otherwise, an insecure connection

opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{MinVersion: tls.VersionTLS12})))
if fulcioServer.Port() == "" {
host = fmt.Sprintf("%s:443", host)
}
default:
bobcallaway marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an open issue to add an insecure flag - #2290

We should only default to a TLS-less connection with that flag set. Revising the above comment, if we match on port and or scheme:

  • scheme=https => TLS
  • port=443 => TLS
  • insecure flag set => no TLS

We could just ignore port 80, and in testing it's unlikely you'd be using port 80 anyways, probably 8000+

opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials()))
Copy link
Contributor

Choose a reason for hiding this comment

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

TODO: add a retry policy?

Copy link
Member Author

Choose a reason for hiding this comment

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

let's do this in a separate PR, because it should also apply to calling other external services too

if fulcioServer.Port() == "" {
host = fmt.Sprintf("%s:80", host)
}
}
conn, err := grpc.Dial(host, opts...)
if err != nil {
return nil, err
}
return fulciopb.NewCAClient(conn), nil
}
45 changes: 24 additions & 21 deletions cmd/cosign/cli/fulcio/fulcio_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package fulcio

import (
"context"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rand"
Expand All @@ -25,9 +26,9 @@ import (
"net/http/httptest"
"testing"

"github.com/sigstore/cosign/cmd/cosign/cli/options"
"github.com/sigstore/fulcio/pkg/api"
fulciopb "github.com/sigstore/fulcio/pkg/generated/protobuf"
"github.com/sigstore/sigstore/pkg/oauthflow"
"google.golang.org/grpc"
)

type testFlow struct {
Expand All @@ -44,18 +45,18 @@ func (tf *testFlow) OIDConnect(url, clientID, secret, redirectURL string) (*oaut
}

type testClient struct {
payload api.CertificateResponse
rootResp api.RootResponse
payload fulciopb.SigningCertificate
rootResp fulciopb.TrustBundle
err error
}

var _ api.Client = (*testClient)(nil)
var _ fulciopb.CAClient = (*testClient)(nil)

func (p *testClient) SigningCert(cr api.CertificateRequest, token string) (*api.CertificateResponse, error) {
func (p *testClient) CreateSigningCertificate(_ context.Context, _ *fulciopb.CreateSigningCertificateRequest, _ ...grpc.CallOption) (*fulciopb.SigningCertificate, error) {
return &p.payload, p.err
}

func (p *testClient) RootCert() (*api.RootResponse, error) {
func (p *testClient) GetTrustBundle(_ context.Context, _ *fulciopb.GetTrustBundleRequest, _ ...grpc.CallOption) (*fulciopb.TrustBundle, error) {
return &p.rootResp, p.err
}

Expand Down Expand Up @@ -103,9 +104,14 @@ func TestGetCertForOauthID(t *testing.T) {
expectedCertBytes := pem.EncodeToMemory(expectedCertPem)
expectedExtraBytes := []byte("0123456789abcdef")
tscp := &testClient{
payload: api.CertificateResponse{
CertPEM: expectedCertBytes,
ChainPEM: expectedExtraBytes,
payload: fulciopb.SigningCertificate{
Certificate: &fulciopb.SigningCertificate_SignedCertificateDetachedSct{
SignedCertificateDetachedSct: &fulciopb.SigningCertificateDetachedSCT{
Chain: &fulciopb.CertificateChain{
Certificates: []string{string(expectedCertBytes), string(expectedExtraBytes)},
},
},
},
},
err: tc.signingCertErr,
}
Expand All @@ -118,25 +124,27 @@ func TestGetCertForOauthID(t *testing.T) {
err: tc.tokenGetterErr,
}

resp, err := getCertForOauthID(testKey, tscp, &tf, "", "", "", "")

resp, err := getCertForOauthID(context.Background(), testKey, tscp, &tf, "", "", "", "")
if err != nil {
if !tc.expectErr {
t.Fatalf("getCertForOauthID returned error: %v", err)
}
return
}

leaf := resp.GetSignedCertificateDetachedSct().Chain.Certificates[0]
extra := resp.GetSignedCertificateDetachedSct().Chain.Certificates[1]
if tc.expectErr {
t.Fatalf("getCertForOauthID got: %q, %q wanted error", resp.CertPEM, resp.ChainPEM)
t.Fatalf("getCertForOauthID got: %q, %q wanted error", leaf, extra)
}

expectedCert := string(expectedCertBytes)
actualCert := string(resp.CertPEM)
actualCert := leaf
if actualCert != expectedCert {
t.Errorf("getCertForOauthID returned cert %q, wanted %q", actualCert, expectedCert)
}
expectedChain := string(expectedExtraBytes)
actualChain := string(resp.ChainPEM)
actualChain := extra
if actualChain != expectedChain {
t.Errorf("getCertForOauthID returned chain %q, wanted %q", actualChain, expectedChain)
}
Expand All @@ -146,17 +154,12 @@ func TestGetCertForOauthID(t *testing.T) {

func TestNewClient(t *testing.T) {
t.Parallel()
expectedUserAgent := options.UserAgent()
requestReceived := false
testServer := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
requestReceived = true
file := []byte{}

got := r.UserAgent()
if got != expectedUserAgent {
t.Errorf("wanted User-Agent %q, got %q", expectedUserAgent, got)
}
w.WriteHeader(http.StatusOK)
_, _ = w.Write(file)
}))
Expand All @@ -167,7 +170,7 @@ func TestNewClient(t *testing.T) {
t.Error(err)
}

_, _ = client.SigningCert(api.CertificateRequest{}, "")
_, _ = client.CreateSigningCertificate(context.Background(), &fulciopb.CreateSigningCertificateRequest{})

if !requestReceived {
t.Fatal("no requests were received")
Expand Down
7 changes: 4 additions & 3 deletions cmd/cosign/cli/fulcio/fulcioverifier/fulcioverifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,23 @@ import (
"context"
"fmt"
"os"
"strings"

"github.com/pkg/errors"

"github.com/sigstore/cosign/cmd/cosign/cli/fulcio"
"github.com/sigstore/cosign/cmd/cosign/cli/fulcio/fulcioverifier/ctl"
"github.com/sigstore/fulcio/pkg/api"
fulciopb "github.com/sigstore/fulcio/pkg/generated/protobuf"
)

func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string, fClient api.Client) (*fulcio.Signer, error) {
func NewSigner(ctx context.Context, idToken, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL string, fClient fulciopb.CAClient) (*fulcio.Signer, error) {
fs, err := fulcio.NewSigner(ctx, idToken, oidcIssuer, oidcClientID, oidcClientSecret, oidcRedirectURL, fClient)
if err != nil {
return nil, err
}

// verify the sct
if err := ctl.VerifySCT(ctx, fs.Cert, fs.Chain, fs.SCT); err != nil {
if err := ctl.VerifySCT(ctx, []byte(fs.Cert), []byte(strings.Join(fs.Chain, "\n")), fs.SCT); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Do you want to add a TODO here to change VerifySCT to take the list of certs rather than a single PEM-encoded chain? No need to change it now, but it would be better than converting back and forth going forward

return nil, errors.Wrap(err, "verifying SCT")
}
fmt.Fprintln(os.Stderr, "Successfully verified SCT...")
Expand Down
6 changes: 3 additions & 3 deletions cmd/cosign/cli/policy_init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func signPolicy() *cobra.Command {
}
defer sv.Close()

certs, err := cryptoutils.LoadCertificatesFromPEM(bytes.NewReader(sv.Cert))
certs, err := cryptoutils.LoadCertificatesFromPEM(strings.NewReader(sv.Cert))
if err != nil {
return err
}
Expand Down Expand Up @@ -247,7 +247,7 @@ func signPolicy() *cobra.Command {
}
signature := tuf.Signature{
Signature: base64.StdEncoding.EncodeToString(sig),
Cert: base64.StdEncoding.EncodeToString(sv.Cert),
Cert: base64.StdEncoding.EncodeToString([]byte(sv.Cert)),
}
if err := signed.AddOrUpdateSignature(key, signature); err != nil {
return err
Expand All @@ -261,7 +261,7 @@ func signPolicy() *cobra.Command {
if err != nil {
return err
}
entry, err := cosign.TLogUpload(ctx, rekorClient, sig, signed.Signed, rekorBytes)
entry, err := cosign.TLogUpload(ctx, rekorClient, sig, signed.Signed, []byte(rekorBytes))
if err != nil {
return err
}
Expand Down
Loading