buffrr / letsdane

🔒 Let's DANE is an experimental way to enable the use of DANE/TLSA in browsers and other apps using a lightweight proxy.
Apache License 2.0
111 stars 11 forks source link

Code to improve #32

Closed yagikota closed 10 months ago

yagikota commented 10 months ago

After performing static analysis using golangci-lint, there are some code to improve. Fixing these will make letsdane a better library.

golangci-lint run ./...
proxy/conn.go:48:10: Error return value of `io.Copy` is not checked (errcheck)
        io.Copy(src, dst)
               ^
proxy/conn_test.go:22:19: Error return value of `client.Handshake` is not checked (errcheck)
        client.Handshake()
                        ^
proxy/conn_test.go:40:14: Error return value of `(*crypto/tls.Conn).Handshake` is not checked (errcheck)
    }).Handshake()
                ^
proxy/proxy_test.go:18:10: Error return value of `w.Write` is not checked (errcheck)
        w.Write([]byte("hey"))
               ^
proxy/proxy_test.go:76:19: Error return value of `clientConn.Write` is not checked (errcheck)
        clientConn.Write([]byte("bar"))
                        ^
proxy/proxy_test.go:110:11: Error return value of `w.Write` is not checked (errcheck)
            w.Write([]byte("hello"))
                   ^
dialer_test.go:16:11: Error return value of `wr.Write` is not checked (errcheck)
        wr.Write([]byte("foo"))
                ^
dialer_test.go:38:12: Error return value of `conn.Write` is not checked (errcheck)
    conn.Write([]byte("GET / HTTP/1.1\nHost:example.org\n\r\n\r"))
              ^
tls.go:71:18: Error return value of `conn.SetDeadline` is not checked (errcheck)
    conn.SetDeadline(time.Now().Add(time.Second * 3))
                    ^
tls.go:81:21: Error return value of `clientTLS.Handshake` is not checked (errcheck)
    clientTLS.Handshake()
                       ^
tunnel.go:250:10: Error return value of `io.Copy` is not checked (errcheck)
        io.Copy(src, dst)
               ^
tunnel_test.go:43:10: Error return value of `w.Write` is not checked (errcheck)
        w.Write([]byte("foo"))
               ^
tunnel_test.go:104:14: Error return value of `conn.Write` is not checked (errcheck)
            conn.Write([]byte(fmt.Sprintf("GET %s HTTP/1.1\nHost:%s\nConnection:close\n\r\n\r", test.uri, test.host)))
                      ^
tunnel_test.go:132:11: Error return value of `wr.Write` is not checked (errcheck)
        wr.Write([]byte("foo"))
                ^
tunnel_test.go:141:14: Error return value of `conn.Write` is not checked (errcheck)
            conn.Write([]byte("ALPN TEST"))
                      ^
cmd/letsdane/main.go:86:14: Error return value of `pem.Encode` is not checked (errcheck)
            pem.Encode(certOut, &pem.Block{
                      ^
cmd/letsdane/main.go:92:14: Error return value of `pem.Encode` is not checked (errcheck)
            pem.Encode(privOut, &pem.Block{
                      ^
cmd/letsdane/main.go:103:14: Error return value of `kOut.Write` is not checked (errcheck)
            kOut.Write(privOut.Bytes())
                      ^
resolver/resolver_test.go:403:6: func `rrsEq` is unused (unused)
func rrsEq(slice1, slice2 []dns.RR) bool {
     ^
cert.go:34:2: field `getCertificate` is unused (unused)
    getCertificate func(*tls.ClientHelloInfo) (*tls.Certificate, error)
    ^
resolver/resolver_test.go:361:10: S1034: assigning the result of this type assertion to a variable (switch rr := rr.(type)) could eliminate type assertions in switch cases (gosimple)
        switch rr.(type) {
               ^
resolver/resolver_test.go:363:18: S1034(related information): could eliminate this type assertion (gosimple)
            f = append(f, rr.(*dns.TLSA))
                          ^
resolver/stub_test.go:82:5: testinggoroutine: call to (*T).Fatal from a non-test goroutine (govet)
                t.Fatal(err)
                ^
resolver/stub_test.go:87:5: testinggoroutine: call to (*T).Fatal from a non-test goroutine (govet)
                t.Fatal("dnssec-failed.org returned a valid response")
                ^
resolver/stub_test.go:92:5: testinggoroutine: call to (*T).Fatal from a non-test goroutine (govet)
                t.Fatal(err)
                ^
resolver/stub_test.go:96:5: testinggoroutine: call to (*T).Fatalf from a non-test goroutine (govet)
                t.Fatalf("got no ips")
                ^
resolver/stub_test.go:105:5: testinggoroutine: call to (*T).Fatalf from a non-test goroutine (govet)
                t.Fatalf("got no tlsa records from freebsd.org")
                ^
tunnel_test.go:373:10: ineffectual assignment to err (ineffassign)
            body, err := io.ReadAll(resp.Body)
                  ^
cert_test.go:89:2: SA4006: this value of `tlsc` is never used (staticcheck)
    tlsc, err = conf.GetCertificate(clientHello)
    ^
cert_test.go:96:2: SA4006: this value of `tlsc` is never used (staticcheck)
    tlsc, err = conf.GetCertificate(clientHello)
    ^
cmd/letsdane/main.go:124:5: SA1019: x509.IsEncryptedPEMBlock has been deprecated since Go 1.16 because it shouldn't be used: Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since it does not authenticate the ciphertext, it is vulnerable to padding oracle attacks that can let an attacker recover the plaintext. (staticcheck)
    if x509.IsEncryptedPEMBlock(block) {
       ^
cmd/letsdane/main.go:129:25: SA1019: x509.DecryptPEMBlock has been deprecated since Go 1.16 because it shouldn't be used: Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since it does not authenticate the ciphertext, it is vulnerable to padding oracle attacks that can let an attacker recover the plaintext. (staticcheck)
        decryptedBlock, err = x509.DecryptPEMBlock(block, []byte(*pass))
                              ^
resolver/stub_test.go:77:3: SA2002: the goroutine calls T.Fatal, which must be called in the same goroutine as the test (staticcheck)
        go func(proto string) {
        ^
buffrr commented 10 months ago

Many of the flagged issues are in tests or not directly applicable. Open to PRs for specific improvements though!