From 4bfe5353de4e8e6323de0325cfb4116f74863695 Mon Sep 17 00:00:00 2001 From: Zi Lin Date: Wed, 25 Jan 2017 13:23:08 -0800 Subject: [PATCH] fix unused value assignments caught by static checker (#718) - also update static checker's official import path - various bugfixes regarding bad error handling or bad logic. --- api/api_test.go | 12 ++++++------ api/crl/crl.go | 4 ++++ certdb/testdb/certstore_development.db | Bin 13312 -> 13312 bytes cli/cli_test.go | 2 +- cli/gencert/gencert.go | 4 ++++ cli/genkey/genkey.go | 3 +++ cli/printdefault/printdefault.go | 2 +- cli/selfsign/selfsign.go | 5 +++++ cli/sign/sign.go | 4 ++++ config/config_test.go | 4 ++++ csr/csr_test.go | 8 ++++---- helpers/helpers_test.go | 4 ++-- helpers/testsuite/testing_helpers.go | 5 +++++ initca/initca_test.go | 18 +++++++++--------- multiroot/config/config.go | 13 +++++++++++++ ocsp/ocsp_test.go | 4 ++++ signer/signer.go | 3 +++ test.sh | 2 +- 18 files changed, 73 insertions(+), 24 deletions(-) diff --git a/api/api_test.go b/api/api_test.go index c569c2708..fe2105154 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -120,7 +120,7 @@ func TestRigidHandle(t *testing.T) { obj = map[string]interface{}{} obj["critique"] = "it's OK" obj["compliment"] = "it's not bad" - resp, body = post(t, obj, ts) + resp, _ = post(t, obj, ts) if resp.StatusCode != http.StatusBadRequest { t.Errorf("Test expected 400, have %d", resp.StatusCode) @@ -128,14 +128,14 @@ func TestRigidHandle(t *testing.T) { // reject empty review obj = map[string]interface{}{} - resp, body = post(t, obj, ts) + resp, _ = post(t, obj, ts) if resp.StatusCode != http.StatusBadRequest { t.Errorf("Test expected 400, have %d", resp.StatusCode) } // reject GET - resp, body = get(t, ts) + resp, _ = get(t, ts) if resp.StatusCode != http.StatusMethodNotAllowed { t.Errorf("Test expected 405, have %d", resp.StatusCode) @@ -190,7 +190,7 @@ func TestCleverHandle(t *testing.T) { obj = map[string]interface{}{} obj["critique"] = "it's OK" obj["compliment"] = "it's not bad" - resp, body = post(t, obj, ts) + _, body = post(t, obj, ts) message = new(Response) err = json.Unmarshal(body, message) @@ -205,14 +205,14 @@ func TestCleverHandle(t *testing.T) { // reject empty review obj = map[string]interface{}{} - resp, body = post(t, obj, ts) + resp, _ = post(t, obj, ts) if resp.StatusCode != http.StatusBadRequest { t.Errorf("Test expected 400, have %d", resp.StatusCode) } // reject GET - resp, body = get(t, ts) + resp, _ = get(t, ts) if resp.StatusCode != http.StatusMethodNotAllowed { t.Errorf("Test expected 405, have %d", resp.StatusCode) diff --git a/api/crl/crl.go b/api/crl/crl.go index 9538aa5b3..cf1cb4660 100644 --- a/api/crl/crl.go +++ b/api/crl/crl.go @@ -87,6 +87,10 @@ func crlHandler(w http.ResponseWriter, r *http.Request) error { } result, err := cert.CreateCRL(rand.Reader, key, revokedCerts, time.Now(), newExpiryTime) + if err != nil { + log.Debug("Unable to create CRL: %v", err) + return err + } return api.SendResponse(w, result) } diff --git a/certdb/testdb/certstore_development.db b/certdb/testdb/certstore_development.db index b5f7ded9c20688e1c6e69fcc5ec9d5f624ac50df..874ae08f1b660bede9c59cc33d021171abf62c0a 100644 GIT binary patch delta 1444 zcmZWpTZkJ~7~X8s?e1)M+d^B{O0{^Qmu=0N$#p8Vo=N7GWRjh0GJ}FiCYfxmn@qAZ zJ6h6RDI%q%HVPILt3p9Uv|Ygmp;fWkr&7e~i};`@SWzFW2wKqDt}TngIdIPR|Nr+N zzW?wYsT`@C+&<~sJ2W&j@$dFNzdJna+k5WGv*V+uhPCCtaTE_n@F0#)18@KU44{$!q%1E?-t9>~`?LpHKQXa&^w6ObJ@zUu z-8i{RHd-y+tyoKnx+JSq0JyDgt(~{1?*s-5llS@D*COXP{q7xKKQVTj?);Wh-Z8h) z$jB2XL~(HS9)N-+@2iMs=**J$CHMHk$k_CVXWhE>z~k{Pd5^l#B3S(9H5!oUI2ar8 z%D&~Bca3i7$7=Cd43--Pv{(Wftjx=b1Zpu|N$R-KGV7*XOC{#jhLDI_ab8U%TN+!L zFQy?+QV79AOOVJE2WvD$Ws<0HFBFvs5zm|iG9*S80%IvN!pb}p()gh zjZj^W5xIO;Xy$W*!%_q&i3F$9yk%(rQ-N4fA#SJuax9&IEt-I-yc*KNj9SETEoR5^ z%q*BAwRXgxCT(CTw zvRatJrvrLDTmfAt7><;PP<dl-_QY zj0S;a1NoMz@_}|RvXFz*@d8r{5u8Bxb9SWS4v9b@q^o6#h;T5tGDi(*+(59onZBW)G}N=<9nm10Bg zsTkhnAfRJnn-$@EaoJf`5IMnV5A`Q?2NyO82MPk^w;J!HeGKwY?e;NQqx?sPGvzt79093F1Ja4 zZo10Hnp~#jX%4>k-#=h{`rz0@2PdaCuGk7du@Jxl!GW|}0C#Q}s-5v79?#kw*ExW8 zX7e2=46wn)2dDbSci`IPgXYy2KlpTN?)s%#=1r!jyhjl+V$SyCF8GOe;ZD=FMg6pe%Jm2#svSM`rG*O25`gs4;XaEf&{=@5KY_4l) zsB37UU}S7%Vrpe#sb^wpVPt4-U}C0iU|?lnU}RuuhNfilU4Hq=Gx&bxg}ORr_&Rfi zh51&vx;XmBGIDat@=7xX8*8Q|W~VA7W@i?qmgQ%srmz4_!LZ2G*u>1t!py`7r$wcC zsTBo4Gg1_iQ;SMA34CSd=9J}xnY~G55;JE9BO@DIY+r2f z1u|!{acZ+Mva+%pFfy_WFJNY3WLW5K;A+Tiu=$TT2hZgHnofKY{2-qi z8X8&{8Jn0+4lxLlgm4UiAZmdiT;9^ic=AOJiTbrx;-`4CpIUiKt$sYSU}4zq_Dvnz zLU(x8Dt>Eiir@Awk^S{KVHV$81{eGTMUN(Ro{N1s*-L1<$TKB>zcZgN`M5+sb*s8kkyGT25{>uAdmdg3WjS8bJndznWM^Enwzi zWQ6+=7y`IFJ$e1v^{!=M^EBB$8qX1~KCZ--C1$&|**=2zOYM(CflK#Ix$*R!y7?6C zgvGH2JNrZGbnK7rsNZgP|FFp6uN(IZ9Ix4}d`#naRi&$>mci3E4`yg=u6x!X>sDX) zYFQqCRd&Go*u-qPpY>Z;e!V>Rb=rd&5;N+Ksu{g@ODOROF5+SI!sTfTQv*u_b2Ag5 Wr*k&5=rFUeG6R!8*Jd7rFU$ayKLODI diff --git a/cli/cli_test.go b/cli/cli_test.go index 246e1a0a6..b836ffd87 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -97,7 +97,7 @@ func TestReadStdin(t *testing.T) { t.Fatal(err) } - fn, err = ReadStdin("-") + _, err = ReadStdin("-") if err != nil { t.Fatal(err) } diff --git a/cli/gencert/gencert.go b/cli/gencert/gencert.go index ea74cb702..45cdab35b 100644 --- a/cli/gencert/gencert.go +++ b/cli/gencert/gencert.go @@ -54,6 +54,10 @@ func gencertMain(args []string, c cli.Config) error { return err } + if len(args) > 0 { + return errors.New("only one argument is accepted, please check with usage") + } + csrJSONFileBytes, err := cli.ReadStdin(csrJSONFile) if err != nil { return err diff --git a/cli/genkey/genkey.go b/cli/genkey/genkey.go index 2d810e70e..f82122f1a 100644 --- a/cli/genkey/genkey.go +++ b/cli/genkey/genkey.go @@ -28,6 +28,9 @@ func genkeyMain(args []string, c cli.Config) (err error) { if err != nil { return } + if len(args) > 0 { + return errors.New("only one argument is accepted, please check with usage") + } csrFileBytes, err := cli.ReadStdin(csrFile) if err != nil { diff --git a/cli/printdefault/printdefault.go b/cli/printdefault/printdefault.go index fe7381ded..7a6a53d6f 100644 --- a/cli/printdefault/printdefault.go +++ b/cli/printdefault/printdefault.go @@ -22,7 +22,7 @@ func printAvailable() { } func printDefaults(args []string, c cli.Config) (err error) { - arg, args, err := cli.PopFirstArgument(args) + arg, _, err := cli.PopFirstArgument(args) if err != nil { return } diff --git a/cli/selfsign/selfsign.go b/cli/selfsign/selfsign.go index c4a595379..a4d8f7c44 100644 --- a/cli/selfsign/selfsign.go +++ b/cli/selfsign/selfsign.go @@ -3,6 +3,7 @@ package selfsign import ( "encoding/json" + "errors" "fmt" "os" "time" @@ -49,6 +50,10 @@ func selfSignMain(args []string, c cli.Config) (err error) { return } + if len(args) > 0 { + return errors.New("too many arguments are provided, please check with usage") + } + csrFileBytes, err := cli.ReadStdin(csrFile) if err != nil { return diff --git a/cli/sign/sign.go b/cli/sign/sign.go index 6aa1369f5..71384fcdd 100644 --- a/cli/sign/sign.go +++ b/cli/sign/sign.go @@ -3,6 +3,7 @@ package sign import ( "encoding/json" + "errors" "io/ioutil" "github.com/cloudflare/cfssl/certdb/dbconf" @@ -121,6 +122,9 @@ func signerMain(args []string, c cli.Config) (err error) { if err != nil { return } + if len(args) > 0 { + return errors.New("too many arguments are provided, please check with usage") + } var subjectJSON []byte subjectJSON, err = ioutil.ReadFile(subjectFile) diff --git a/config/config_test.go b/config/config_test.go index 5bc54b18c..1d1266469 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -447,6 +447,10 @@ func TestNeedLocalSigner(t *testing.T) { if localConfig.Signing.NeedsRemoteSigner() != false { t.Fatal("incorrect NeedsRemoteSigner().") } + + if err != nil { + t.Fatal(err) + } } func TestOverrideRemotes(t *testing.T) { diff --git a/csr/csr_test.go b/csr/csr_test.go index 7b9439767..b420fa403 100644 --- a/csr/csr_test.go +++ b/csr/csr_test.go @@ -634,7 +634,7 @@ func TestReGenerate(t *testing.T) { KeyRequest: &BasicKeyRequest{"ecdsa", 256}, } - csr, key, err := ParseRequest(req) + _, key, err := ParseRequest(req) if err != nil { t.Fatalf("%v", err) } @@ -644,7 +644,7 @@ func TestReGenerate(t *testing.T) { t.Fatalf("%v", err) } - csr, err = Generate(priv, req) + csr, err := Generate(priv, req) if err != nil { t.Fatalf("%v", err) } @@ -677,7 +677,7 @@ func TestBadReGenerate(t *testing.T) { KeyRequest: &BasicKeyRequest{"ecdsa", 256}, } - csr, key, err := ParseRequest(req) + _, key, err := ParseRequest(req) if err != nil { t.Fatalf("%v", err) } @@ -687,7 +687,7 @@ func TestBadReGenerate(t *testing.T) { t.Fatalf("%v", err) } - csr, err = Generate(priv, req) + csr, err := Generate(priv, req) if err != nil { t.Fatalf("%v", err) } diff --git a/helpers/helpers_test.go b/helpers/helpers_test.go index 9f1d6b146..1e039746c 100644 --- a/helpers/helpers_test.go +++ b/helpers/helpers_test.go @@ -428,12 +428,12 @@ const clientCertFile = "testdata/ca.pem" const clientKeyFile = "testdata/ca_key.pem" func TestClientCertParams(t *testing.T) { - cert, err := LoadClientCertificate(testCertFile, testPrivateRSAKey) + _, err := LoadClientCertificate(testCertFile, testPrivateRSAKey) if err == nil { t.Fatal("Unmatched cert/key should generate error") } - cert, err = LoadClientCertificate("", "") + cert, err := LoadClientCertificate("", "") if err != nil || cert != nil { t.Fatal("Certificate atempted to loaded with missing key and cert") } diff --git a/helpers/testsuite/testing_helpers.go b/helpers/testsuite/testing_helpers.go index f937540e9..9db0793e4 100644 --- a/helpers/testsuite/testing_helpers.go +++ b/helpers/testsuite/testing_helpers.go @@ -284,10 +284,15 @@ func SignCertificate(request csr.CertificateRequest, signerCert, signerKey []byt tempCSRFile, ) CLIOutput, err = command.CombinedOutput() + if err != nil { + return nil, nil, fmt.Errorf("%v - CLI output: %s", err, string(CLIOutput)) + } + err = checkCLIOutput(CLIOutput) if err != nil { return nil, nil, fmt.Errorf("%v - CLI output: %s", err, string(CLIOutput)) } + encodedCert, err = cleanCLIOutput(CLIOutput, "cert") if err != nil { return nil, nil, err diff --git a/initca/initca_test.go b/initca/initca_test.go index 922489968..57b814dc6 100644 --- a/initca/initca_test.go +++ b/initca/initca_test.go @@ -16,12 +16,12 @@ import ( ) var validKeyParams = []csr.BasicKeyRequest{ - {"rsa", 2048}, - {"rsa", 3072}, - {"rsa", 4096}, - {"ecdsa", 256}, - {"ecdsa", 384}, - {"ecdsa", 521}, + {A: "rsa", S: 2048}, + {A: "rsa", S: 3072}, + {A: "rsa", S: 4096}, + {A: "ecdsa", S: 256}, + {A: "ecdsa", S: 384}, + {A: "ecdsa", S: 521}, } var validCAConfigs = []csr.CAConfig{ @@ -54,10 +54,10 @@ var testECDSACAKeyFile = "testdata/5min-ecdsa-key.pem" var invalidCryptoParams = []csr.BasicKeyRequest{ // Weak Key - {"rsa", 1024}, + {A: "rsa", S: 1024}, // Bad param - {"rsaCrypto", 2048}, - {"ecdsa", 2000}, + {A: "rsaCrypto", S: 2048}, + {A: "ecdsa", S: 2000}, } func TestInitCA(t *testing.T) { diff --git a/multiroot/config/config.go b/multiroot/config/config.go index 797131a99..db9f7d381 100644 --- a/multiroot/config/config.go +++ b/multiroot/config/config.go @@ -242,6 +242,19 @@ func parsePrivateKeySpec(spec string, cfg map[string]string) (crypto.Signer, err return nil, err } + log.Debug("attempting to load PEM-encoded private key") + priv, err = helpers.ParsePrivateKeyPEM(in) + if err != nil { + log.Debug("file is not a PEM-encoded private key") + log.Debug("attempting to load DER-encoded private key") + priv, err = derhelpers.ParsePrivateKeyDER(in) + if err != nil { + return nil, err + } + } + + log.Debug("loaded private key") + return priv, nil default: return nil, ErrUnsupportedScheme diff --git a/ocsp/ocsp_test.go b/ocsp/ocsp_test.go index ed55c359e..3b1a06394 100644 --- a/ocsp/ocsp_test.go +++ b/ocsp/ocsp_test.go @@ -121,6 +121,10 @@ func TestSign(t *testing.T) { } sMismatch, err := NewSignerFromFile(wrongServerCertFile, otherCertFile, wrongServerKeyFile, dur) + if err != nil { + t.Fatal("NewSigner failed:", err) + } + _, err = sMismatch.Sign(req) if err == nil { t.Fatal("Signed a certificate from the wrong issuer") diff --git a/signer/signer.go b/signer/signer.go index fa7b91782..a90e3d217 100644 --- a/signer/signer.go +++ b/signer/signer.go @@ -233,6 +233,9 @@ func ComputeSKI(template *x509.Certificate) ([]byte, error) { // and SKI. func FillTemplate(template *x509.Certificate, defaultProfile, profile *config.SigningProfile) error { ski, err := ComputeSKI(template) + if err != nil { + return err + } var ( eku []x509.ExtKeyUsage diff --git a/test.sh b/test.sh index f4b66f934..65ea9db88 100755 --- a/test.sh +++ b/test.sh @@ -53,7 +53,7 @@ do done if ! command -v staticcheck > /dev/null ; then - go get github.com/dominikh/go-staticcheck/cmd/staticcheck + go get honnef.co/go/tools/cmd/staticcheck fi for package in $PACKAGES