From b97495786805453dd2e6692f41ed7262072294d7 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Tue, 19 Feb 2019 19:45:52 -0800 Subject: [PATCH 1/3] Add certificate information to logs. Fixes smallstep/ca-component#147 --- api/api.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ api/api_test.go | 5 +++-- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/api/api.go b/api/api.go index 4bdf1b09..edd21237 100644 --- a/api/api.go +++ b/api/api.go @@ -1,10 +1,15 @@ package api import ( + "crypto/ecdsa" + "crypto/rsa" "crypto/tls" "crypto/x509" + "encoding/asn1" + "encoding/base64" "encoding/json" "encoding/pem" + "fmt" "net/http" "strconv" "strings" @@ -13,6 +18,7 @@ import ( "github.com/go-chi/chi" "github.com/pkg/errors" "github.com/smallstep/certificates/authority" + "github.com/smallstep/certificates/logging" "github.com/smallstep/cli/crypto/tlsutil" ) @@ -275,6 +281,7 @@ func (h *caHandler) Sign(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusCreated) + logCertificate(w, cert, body.OTT) JSON(w, &SignResponse{ ServerPEM: Certificate{cert}, CaPEM: Certificate{root}, @@ -297,6 +304,7 @@ func (h *caHandler) Renew(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusCreated) + logCertificate(w, cert, "") JSON(w, &SignResponse{ ServerPEM: Certificate{cert}, CaPEM: Certificate{root}, @@ -372,6 +380,43 @@ func (h *caHandler) Federation(w http.ResponseWriter, r *http.Request) { }) } +var oidStepProvisioner = asn1.ObjectIdentifier{1, 3, 6, 1, 4, 1, 37476, 9000, 64, 1} + +type stepProvisioner struct { + Type int + Name []byte + CredentialID []byte +} + +func logCertificate(w http.ResponseWriter, cert *x509.Certificate, token string) { + if rl, ok := w.(logging.ResponseLogger); ok { + m := map[string]interface{}{ + "serial": cert.SerialNumber, + "subject": cert.Subject.CommonName, + "issuer": cert.Issuer.CommonName, + "valid-from": cert.NotBefore.Format(time.RFC3339), + "valid-to": cert.NotAfter.Format(time.RFC3339), + "public-key": fmtPublicKey(cert), + "certificate": base64.StdEncoding.EncodeToString(cert.Raw), + } + if len(token) > 0 { + m["ott"] = token + } + for _, ext := range cert.Extensions { + if ext.Id.Equal(oidStepProvisioner) { + val := &stepProvisioner{} + rest, err := asn1.Unmarshal(ext.Value, val) + if err != nil || len(rest) > 0 { + break + } + m["provisioner"] = fmt.Sprintf("%s (%s)", val.Name, val.CredentialID) + break + } + } + rl.WithFields(m) + } +} + func parseCursor(r *http.Request) (cursor string, limit int, err error) { q := r.URL.Query() cursor = q.Get("cursor") @@ -383,3 +428,17 @@ func parseCursor(r *http.Request) (cursor string, limit int, err error) { } return } + +// TODO: add support for Ed25519 once it's supported +func fmtPublicKey(cert *x509.Certificate) string { + var params string + switch pk := cert.PublicKey.(type) { + case *ecdsa.PublicKey: + params = pk.Curve.Params().Name + case *rsa.PublicKey: + params = strconv.Itoa(pk.Size()) + default: + params = "unknown" + } + return fmt.Sprintf("%s %s", cert.PublicKeyAlgorithm, params) +} diff --git a/api/api_test.go b/api/api_test.go index 988b46dd..b2db1796 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -18,6 +18,7 @@ import ( "github.com/go-chi/chi" "github.com/smallstep/certificates/authority" + "github.com/smallstep/certificates/logging" "github.com/smallstep/cli/crypto/tlsutil" "github.com/smallstep/cli/jose" ) @@ -598,7 +599,7 @@ func Test_caHandler_Sign(t *testing.T) { }).(*caHandler) req := httptest.NewRequest("POST", "http://example.com/sign", strings.NewReader(tt.input)) w := httptest.NewRecorder() - h.Sign(w, req) + h.Sign(logging.NewResponseLogger(w), req) res := w.Result() if res.StatusCode != tt.statusCode { @@ -650,7 +651,7 @@ func Test_caHandler_Renew(t *testing.T) { req := httptest.NewRequest("POST", "http://example.com/renew", nil) req.TLS = tt.tls w := httptest.NewRecorder() - h.Renew(w, req) + h.Renew(logging.NewResponseLogger(w), req) res := w.Result() if res.StatusCode != tt.statusCode { From adbc496b40301d63c155dac8955c37cbb73a7131 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 20 Feb 2019 12:18:13 -0800 Subject: [PATCH 2/3] Improve tests --- api/api.go | 5 ++- api/api_test.go | 117 ++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 112 insertions(+), 10 deletions(-) diff --git a/api/api.go b/api/api.go index edd21237..37a151d6 100644 --- a/api/api.go +++ b/api/api.go @@ -1,6 +1,7 @@ package api import ( + "crypto/dsa" "crypto/ecdsa" "crypto/rsa" "crypto/tls" @@ -436,7 +437,9 @@ func fmtPublicKey(cert *x509.Certificate) string { case *ecdsa.PublicKey: params = pk.Curve.Params().Name case *rsa.PublicKey: - params = strconv.Itoa(pk.Size()) + params = strconv.Itoa(pk.Size() * 8) + case *dsa.PublicKey: + params = strconv.Itoa(pk.Q.BitLen() * 8) default: params = "unknown" } diff --git a/api/api_test.go b/api/api_test.go index b2db1796..c4907b8d 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -3,12 +3,19 @@ package api import ( "bytes" "context" + "crypto/dsa" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/rsa" "crypto/tls" "crypto/x509" + "crypto/x509/pkix" "encoding/json" "encoding/pem" "fmt" "io/ioutil" + "math/big" "net/http" "net/http/httptest" "reflect" @@ -99,6 +106,23 @@ Q7vMNPBWrJWu+A++vHY61WGET+h4lY3GFr2I8OE4IiHPQi1D7Y0+fwOmStwuRPM4 DCbKzWTW8lqVdp9Kyf7XEhhc2R8C5w== -----END CERTIFICATE REQUEST-----` + stepCertPEM = `-----BEGIN CERTIFICATE----- +MIIChTCCAiugAwIBAgIRAJ3O5T28Rdj2lr/UPjf+GAUwCgYIKoZIzj0EAwIwJDEi +MCAGA1UEAxMZU21hbGxzdGVwIEludGVybWVkaWF0ZSBDQTAeFw0xOTAyMjAyMDE1 +NDNaFw0xOTAyMjEyMDE1NDNaMHExCzAJBgNVBAYTAlVTMQswCQYDVQQIEwJDQTEW +MBQGA1UEBxMNU2FuIEZyYW5jaXNjbzEcMBoGA1UEChMTU21hbGxzdGVwIExhYnMg +SW5jLjEfMB0GA1UEAxMWaW50ZXJuYWwuc21hbGxzdGVwLmNvbTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABC0aKrTNl+gXFuNkXisqX4/foLO3VMt+Kphngziim+fz +aJhiS9JU+oFYLTNW6HWGUD8CNzfwrmWlVsAmiJwHKlKjgfAwge0wDgYDVR0PAQH/ +BAQDAgWgMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAdBgNVHQ4EFgQU +JheKvlZqNv1IcgaC8WOS1Zg0i1QwHwYDVR0jBBgwFoAUu97PaFQPfuyKOeew7Hg4 +5WFIAVMwIQYDVR0RBBowGIIWaW50ZXJuYWwuc21hbGxzdGVwLmNvbTBZBgwrBgEE +AYKkZMYoQAEESTBHAgEBBBVtYXJpYW5vQHNtYWxsc3RlcC5jb20EK2pPMzdkdERi +a3UtUW5hYnM1VlIwWXc2WUZGdjl3ZUExOGRwM2h0dmRFanMwCgYIKoZIzj0EAwID +SAAwRQIhAIrn17fP5CBrGtKuhyPiq6eSwryBCf8ki+k17u5a+E/LAiB24Y2E0Put +nIHOI54lAqDeF7A0y73fPRVCiJEWmuxz0g== +-----END CERTIFICATE-----` + pubKey = `{ "use": "sig", "kty": "EC", @@ -567,6 +591,9 @@ func Test_caHandler_Sign(t *testing.T) { t.Fatal(err) } + expected1 := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"}`) + expected2 := []byte(`{"crt":"` + strings.Replace(stepCertPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"}`) + tests := []struct { name string input string @@ -576,16 +603,16 @@ func Test_caHandler_Sign(t *testing.T) { root *x509.Certificate signErr error statusCode int + expected []byte }{ - {"ok", string(valid), nil, nil, parseCertificate(certPEM), parseCertificate(rootPEM), nil, http.StatusCreated}, - {"json read error", "{", nil, nil, nil, nil, nil, http.StatusBadRequest}, - {"validate error", string(invalid), nil, nil, nil, nil, nil, http.StatusBadRequest}, - {"authorize error", string(valid), nil, fmt.Errorf("an error"), nil, nil, nil, http.StatusUnauthorized}, - {"sign error", string(valid), nil, nil, nil, nil, fmt.Errorf("an error"), http.StatusForbidden}, + {"ok", string(valid), nil, nil, parseCertificate(certPEM), parseCertificate(rootPEM), nil, http.StatusCreated, expected1}, + {"ok with Provisioner", string(valid), nil, nil, parseCertificate(stepCertPEM), parseCertificate(rootPEM), nil, http.StatusCreated, expected2}, + {"json read error", "{", nil, nil, nil, nil, nil, http.StatusBadRequest, nil}, + {"validate error", string(invalid), nil, nil, nil, nil, nil, http.StatusBadRequest, nil}, + {"authorize error", string(valid), nil, fmt.Errorf("an error"), nil, nil, nil, http.StatusUnauthorized, nil}, + {"sign error", string(valid), nil, nil, nil, nil, fmt.Errorf("an error"), http.StatusForbidden, nil}, } - expected := []byte(`{"crt":"` + strings.Replace(certPEM, "\n", `\n`, -1) + `\n","ca":"` + strings.Replace(rootPEM, "\n", `\n`, -1) + `\n"}`) - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { h := New(&mockAuthority{ @@ -612,8 +639,8 @@ func Test_caHandler_Sign(t *testing.T) { t.Errorf("caHandler.Root unexpected error = %v", err) } if tt.statusCode < http.StatusBadRequest { - if !bytes.Equal(bytes.TrimSpace(body), expected) { - t.Errorf("caHandler.Root Body = %s, wants %s", body, expected) + if !bytes.Equal(bytes.TrimSpace(body), tt.expected) { + t.Errorf("caHandler.Root Body = %s, wants %s", body, tt.expected) } } }) @@ -921,3 +948,75 @@ func Test_caHandler_Federation(t *testing.T) { }) } } + +func Test_fmtPublicKey(t *testing.T) { + p256, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + rsa1024, err := rsa.GenerateKey(rand.Reader, 1024) + if err != nil { + t.Fatal(err) + } + var dsa2048 dsa.PrivateKey + if err := dsa.GenerateParameters(&dsa2048.Parameters, rand.Reader, dsa.L2048N256); err != nil { + t.Fatal(err) + } + if err := dsa.GenerateKey(&dsa2048, rand.Reader); err != nil { + t.Fatal(err) + } + + type args struct { + pub, priv interface{} + cert *x509.Certificate + } + tests := []struct { + name string + args args + want string + }{ + {"p256", args{p256.Public(), p256, nil}, "ECDSA P-256"}, + {"rsa1024", args{rsa1024.Public(), rsa1024, nil}, "RSA 1024"}, + {"dsa2048", args{cert: &x509.Certificate{PublicKeyAlgorithm: x509.DSA, PublicKey: &dsa2048.PublicKey}}, "DSA 2048"}, + {"unknown", args{cert: &x509.Certificate{PublicKeyAlgorithm: x509.ECDSA, PublicKey: []byte("12345678")}}, "ECDSA unknown"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var cert *x509.Certificate + if tt.args.cert != nil { + cert = tt.args.cert + } else { + cert = mustCertificate(t, tt.args.pub, tt.args.priv) + } + if got := fmtPublicKey(cert); got != tt.want { + t.Errorf("fmtPublicKey() = %v, want %v", got, tt.want) + } + }) + } +} + +func mustCertificate(t *testing.T, pub, priv interface{}) *x509.Certificate { + template := x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{ + Organization: []string{"Acme Co"}, + }, + NotBefore: time.Now(), + NotAfter: time.Now().Add(24 * time.Hour), + + KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, + BasicConstraintsValid: true, + } + + der, err := x509.CreateCertificate(rand.Reader, &template, &template, pub, priv) + if err != nil { + t.Fatal(err) + } + + cert, err := x509.ParseCertificate(der) + if err != nil { + t.Fatal(err) + } + return cert +} From 1c7155298b9c39da424c959543a674ef61180ceb Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 20 Feb 2019 12:34:40 -0800 Subject: [PATCH 3/3] Log always the token, even on errors. --- api/api.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/api/api.go b/api/api.go index 37a151d6..f8d11ff5 100644 --- a/api/api.go +++ b/api/api.go @@ -259,6 +259,8 @@ func (h *caHandler) Sign(w http.ResponseWriter, r *http.Request) { WriteError(w, BadRequest(errors.Wrap(err, "error reading request body"))) return } + + logOtt(w, body.OTT) if err := body.Validate(); err != nil { WriteError(w, err) return @@ -282,7 +284,7 @@ func (h *caHandler) Sign(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusCreated) - logCertificate(w, cert, body.OTT) + logCertificate(w, cert) JSON(w, &SignResponse{ ServerPEM: Certificate{cert}, CaPEM: Certificate{root}, @@ -305,7 +307,7 @@ func (h *caHandler) Renew(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusCreated) - logCertificate(w, cert, "") + logCertificate(w, cert) JSON(w, &SignResponse{ ServerPEM: Certificate{cert}, CaPEM: Certificate{root}, @@ -389,7 +391,15 @@ type stepProvisioner struct { CredentialID []byte } -func logCertificate(w http.ResponseWriter, cert *x509.Certificate, token string) { +func logOtt(w http.ResponseWriter, token string) { + if rl, ok := w.(logging.ResponseLogger); ok { + rl.WithFields(map[string]interface{}{ + "ott": token, + }) + } +} + +func logCertificate(w http.ResponseWriter, cert *x509.Certificate) { if rl, ok := w.(logging.ResponseLogger); ok { m := map[string]interface{}{ "serial": cert.SerialNumber, @@ -400,9 +410,6 @@ func logCertificate(w http.ResponseWriter, cert *x509.Certificate, token string) "public-key": fmtPublicKey(cert), "certificate": base64.StdEncoding.EncodeToString(cert.Raw), } - if len(token) > 0 { - m["ott"] = token - } for _, ext := range cert.Extensions { if ext.Id.Equal(oidStepProvisioner) { val := &stepProvisioner{}