From c6c62b947208ae9373b7f50f126b945ca4808ef8 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Fri, 29 Nov 2019 18:12:11 +0100 Subject: [PATCH] multi: fix all linter errors --- .golangci.yml | 3 +++ auth/authenticator.go | 2 +- auth/authenticator_test.go | 7 ----- challenger.go | 2 +- config.go | 2 +- freebie/mem_store.go | 8 +++--- kirin.go | 3 ++- mint/mint.go | 8 +++--- mint/mock_test.go | 4 +-- proxy/log.go | 10 +++---- proxy/proxy.go | 8 +++--- proxy/proxy_test.go | 55 +++++++++++++++++++++++--------------- proxy/service.go | 8 +++--- services.go | 4 +-- 14 files changed, 68 insertions(+), 56 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5f0d58c..a4a52f6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -33,3 +33,6 @@ linters: # We have long functions, especially in tests. Moving or renaming those # would trigger funlen problems that we may not want to solve at that time. - funlen + + # Gosec is outdated and reports false positives. + - gosec \ No newline at end of file diff --git a/auth/authenticator.go b/auth/authenticator.go index 6e8bc1b..69dbc26 100644 --- a/auth/authenticator.go +++ b/auth/authenticator.go @@ -203,7 +203,7 @@ func FromHeader(header *http.Header) (*macaroon.Macaroon, lntypes.Preimage, erro // SetHeader sets the provided authentication elements as the default/standard // HTTP header for the LSAT protocol. func SetHeader(header *http.Header, mac *macaroon.Macaroon, - preimage lntypes.Preimage) error { + preimage fmt.Stringer) error { macBytes, err := mac.MarshalBinary() if err != nil { diff --git a/auth/authenticator_test.go b/auth/authenticator_test.go index 4467892..63e3783 100644 --- a/auth/authenticator_test.go +++ b/auth/authenticator_test.go @@ -8,16 +8,9 @@ import ( "github.com/lightninglabs/kirin/auth" "github.com/lightninglabs/loop/lsat" - "github.com/lightningnetwork/lnd/lntypes" "gopkg.in/macaroon.v2" ) -type mockChallenger struct{} - -func (c *mockChallenger) NewChallenge() (string, lntypes.Hash, error) { - return "lnt1xxxx", lntypes.ZeroHash, nil -} - // createDummyMacHex creates a valid macaroon with dummy content for our tests. func createDummyMacHex(preimage string) string { dummyMac, err := macaroon.New( diff --git a/challenger.go b/challenger.go index 97cfc50..5a7bfef 100644 --- a/challenger.go +++ b/challenger.go @@ -35,7 +35,7 @@ func NewLndChallenger(cfg *authConfig, genInvoiceReq InvoiceRequestGenerator) ( } client, err := lndclient.NewBasicClient( - cfg.LndHost, cfg.TlsPath, cfg.MacDir, cfg.Network, + cfg.LndHost, cfg.TLSPath, cfg.MacDir, cfg.Network, ) if err != nil { return nil, err diff --git a/config.go b/config.go index 83031c9..50e2894 100644 --- a/config.go +++ b/config.go @@ -26,7 +26,7 @@ type authConfig struct { // LndHost is the hostname of the LND instance to connect to. LndHost string `long:"lndhost" description:"Hostname of the LND instance to connect to"` - TlsPath string `long:"tlspath"` + TLSPath string `long:"tlspath"` MacDir string `long:"macdir"` diff --git a/freebie/mem_store.go b/freebie/mem_store.go index cf80f74..4a97b98 100644 --- a/freebie/mem_store.go +++ b/freebie/mem_store.go @@ -6,7 +6,7 @@ import ( ) var ( - defaultIpMask = net.IPv4Mask(0xff, 0xff, 0xff, 0x00) + defaultIPMask = net.IPv4Mask(0xff, 0xff, 0xff, 0x00) ) type Count uint16 @@ -17,7 +17,7 @@ type memStore struct { } func (m *memStore) getKey(ip net.IP) string { - return ip.Mask(defaultIpMask).String() + return ip.Mask(defaultIPMask).String() } func (m *memStore) currentCount(ip net.IP) Count { @@ -38,11 +38,11 @@ func (m *memStore) TallyFreebie(r *http.Request, ip net.IP) (bool, error) { return true, nil } -// NewMemIpMaskStore creates a new in-memory freebie store that masks the last +// NewMemIPMaskStore creates a new in-memory freebie store that masks the last // byte of an IP address to keep track of free requests. The last byte of the // address is discarded for the mapping to reduce risk of abuse by users that // have a whole range of IPs at their disposal. -func NewMemIpMaskStore(numFreebies Count) DB { +func NewMemIPMaskStore(numFreebies Count) DB { return &memStore{ numFreebies: numFreebies, freebieCounter: make(map[string]Count), diff --git a/kirin.go b/kirin.go index 25cd175..59fa641 100644 --- a/kirin.go +++ b/kirin.go @@ -2,6 +2,7 @@ package kirin import ( "fmt" + "io" "io/ioutil" "net/http" "os" @@ -171,7 +172,7 @@ func createProxy(cfg *config, genInvoiceReq InvoiceRequestGenerator, } // cleanup closes the given server and shuts down the log rotator. -func cleanup(etcdClient *clientv3.Client, server *http.Server) { +func cleanup(etcdClient io.Closer, server io.Closer) { if err := etcdClient.Close(); err != nil { log.Errorf("Error terminating etcd client: %v", err) } diff --git a/mint/mint.go b/mint/mint.go index 6089c06..d8ee2d3 100644 --- a/mint/mint.go +++ b/mint/mint.go @@ -113,7 +113,7 @@ func (m *Mint) MintLSAT(ctx context.Context, if err != nil { return nil, "", err } - macaroon, err := macaroon.New( + mac, err := macaroon.New( secret[:], id, "lsat", macaroon.LatestVersion, ) if err != nil { @@ -134,13 +134,13 @@ func (m *Mint) MintLSAT(ctx context.Context, return nil, "", err } } - if err := lsat.AddFirstPartyCaveats(macaroon, caveats...); err != nil { + if err := lsat.AddFirstPartyCaveats(mac, caveats...); err != nil { // Attempt to revoke the secret to save space. _ = m.cfg.Secrets.RevokeSecret(ctx, idHash) return nil, "", err } - return macaroon, paymentRequest, nil + return mac, paymentRequest, nil } // createUniqueIdentifier creates a new LSAT identifier bound to a payment hash @@ -240,7 +240,7 @@ func (m *Mint) VerifyLSAT(ctx context.Context, params *VerificationParams) error // With the LSAT verified, we'll now inspect its caveats to ensure the // target service is authorized. - var caveats []lsat.Caveat + caveats := make([]lsat.Caveat, 0, len(rawCaveats)) for _, rawCaveat := range rawCaveats { // LSATs can contain third-party caveats that we're not aware // of, so just skip those. diff --git a/mint/mock_test.go b/mint/mock_test.go index 7dddc18..bcbfaf7 100644 --- a/mint/mock_test.go +++ b/mint/mock_test.go @@ -87,7 +87,7 @@ func newMockServiceLimiter() *mockServiceLimiter { func (l *mockServiceLimiter) ServiceCapabilities(ctx context.Context, services ...lsat.Service) ([]lsat.Caveat, error) { - var res []lsat.Caveat + res := make([]lsat.Caveat, 0, len(services)) for _, service := range services { capabilities, ok := l.capabilities[service] if !ok { @@ -101,7 +101,7 @@ func (l *mockServiceLimiter) ServiceCapabilities(ctx context.Context, func (l *mockServiceLimiter) ServiceConstraints(ctx context.Context, services ...lsat.Service) ([]lsat.Caveat, error) { - var res []lsat.Caveat + res := make([]lsat.Caveat, 0, len(services)) for _, service := range services { constraints, ok := l.constraints[service] if !ok { diff --git a/proxy/log.go b/proxy/log.go index 89bc0cb..1395752 100644 --- a/proxy/log.go +++ b/proxy/log.go @@ -46,13 +46,13 @@ func NewRemoteIPPrefixLog(logger btclog.Logger, remoteAddr string) (net.IP, if err != nil { remoteHost = "0.0.0.0" } - remoteIp := net.ParseIP(remoteHost) - if remoteIp == nil { - remoteIp = net.IPv4zero + remoteIP := net.ParseIP(remoteHost) + if remoteIP == nil { + remoteIP = net.IPv4zero } - return remoteIp, &PrefixLog{ + return remoteIP, &PrefixLog{ logger: logger, - prefix: remoteIp.String(), + prefix: remoteIP.String(), } } diff --git a/proxy/proxy.go b/proxy/proxy.go index b1921d7..6f12762 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -62,7 +62,7 @@ func New(auth auth.Authenticator, services []*Service, staticRoot string) ( func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { // Parse and log the remote IP address. We also need the parsed IP // address for the freebie count. - remoteIp, prefixLog := NewRemoteIPPrefixLog(log, r.RemoteAddr) + remoteIP, prefixLog := NewRemoteIPPrefixLog(log, r.RemoteAddr) logRequest := func() { prefixLog.Infof(formatPattern, r.Method, r.RequestURI, r.Proto, r.Referer(), r.UserAgent()) @@ -99,11 +99,12 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { p.handlePaymentRequired(w, r, target.Name) return } + case authLevel.IsFreebie(): // We only need to respect the freebie counter if the user // is not authenticated at all. if !p.authenticator.Accept(&r.Header, target.Name) { - ok, err := target.freebieDb.CanPass(r, remoteIp) + ok, err := target.freebieDb.CanPass(r, remoteIP) if err != nil { prefixLog.Errorf("Error querying freebie db: "+ "%v", err) @@ -117,7 +118,7 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { p.handlePaymentRequired(w, r, target.Name) return } - _, err = target.freebieDb.TallyFreebie(r, remoteIp) + _, err = target.freebieDb.TallyFreebie(r, remoteIP) if err != nil { prefixLog.Errorf("Error updating freebie db: "+ "%v", err) @@ -128,7 +129,6 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } } - case authLevel.IsOff(): } // If we got here, it means everything is OK to pass the request to the diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index 9fd1fc4..fa04eb9 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "io" "io/ioutil" "net" "net/http" @@ -39,7 +40,7 @@ type helloServer struct{} // SayHello returns a simple string that also contains a string from the // request. -func (s *helloServer) SayHello(ctx context.Context, +func (s *helloServer) SayHello(_ context.Context, req *proxytest.HelloRequest) (*proxytest.HelloReply, error) { return &proxytest.HelloReply{ @@ -80,13 +81,13 @@ func TestProxyHTTP(t *testing.T) { Addr: testProxyAddr, Handler: http.HandlerFunc(p.ServeHTTP), } - go server.ListenAndServe() - defer server.Close() + go func() { _ = server.ListenAndServe() }() + defer closeOrFail(t, server) // Start the target backend service. backendService := &http.Server{Addr: testTargetServiceAddress} - go startBackendHTTP(backendService) - defer backendService.Close() + go func() { _ = startBackendHTTP(backendService) }() + defer closeOrFail(t, backendService) // Wait for servers to start. time.Sleep(100 * time.Millisecond) @@ -109,6 +110,7 @@ func TestProxyHTTP(t *testing.T) { t.Fatalf("expected partial LSAT in response header, got: %v", authHeader) } + _ = resp.Body.Close() // Make sure that if the Auth header is set, the client's request is // proxied to the backend service. @@ -128,7 +130,7 @@ func TestProxyHTTP(t *testing.T) { } // Ensure that we got the response body we expect. - defer resp.Body.Close() + defer closeOrFail(t, resp.Body) bodyBytes, err := ioutil.ReadAll(resp.Body) if err != nil { t.Fatalf("failed to read response body: %v", err) @@ -179,8 +181,8 @@ func TestProxyGRPC(t *testing.T) { InsecureSkipVerify: true, }, } - go server.ListenAndServeTLS(certFile, keyFile) - defer server.Close() + go func() { _ = server.ListenAndServeTLS(certFile, keyFile) }() + defer closeOrFail(t, server) // Start the target backend service also on TLS. tlsConf := cert.TLSConfFromCert(certData) @@ -188,7 +190,7 @@ func TestProxyGRPC(t *testing.T) { grpc.Creds(credentials.NewTLS(tlsConf)), } backendService := grpc.NewServer(serverOpts...) - go startBackendGRPC(backendService) + go func() { _ = startBackendGRPC(backendService) }() defer backendService.Stop() // Dial to the proxy now, without any authentication. @@ -202,7 +204,7 @@ func TestProxyGRPC(t *testing.T) { // Make request without authentication. We expect an error that can // be parsed by gRPC. req := &proxytest.HelloRequest{Name: "foo"} - res, err := client.SayHello( + _, err = client.SayHello( context.Background(), req, grpc.WaitForReady(true), ) if err == nil { @@ -225,6 +227,9 @@ func TestProxyGRPC(t *testing.T) { dummyMac, err := macaroon.New( []byte("key"), []byte("id"), "loc", macaroon.LatestVersion, ) + if err != nil { + t.Fatalf("unable to create dummy macaroon: %v", err) + } opts = []grpc.DialOption{ grpc.WithTransportCredentials(creds), grpc.WithPerRPCCredentials(macaroons.NewMacaroonCredential( @@ -239,7 +244,7 @@ func TestProxyGRPC(t *testing.T) { // Make the request. This time no error should be returned. req = &proxytest.HelloRequest{Name: "foo"} - res, err = client.SayHello(context.Background(), req) + res, err := client.SayHello(context.Background(), req) if err != nil { t.Fatalf("unable to call service: %v", err) } @@ -274,13 +279,13 @@ func TestWhitelistHTTP(t *testing.T) { Addr: testProxyAddr, Handler: http.HandlerFunc(p.ServeHTTP), } - go server.ListenAndServe() - defer server.Close() + go func() { _ = server.ListenAndServe() }() + defer closeOrFail(t, server) // Start the target backend service. backendService := &http.Server{Addr: testTargetServiceAddress} - go startBackendHTTP(backendService) - defer backendService.Close() + go func() { _ = startBackendHTTP(backendService) }() + defer closeOrFail(t, backendService) // Wait for servers to start. time.Sleep(100 * time.Millisecond) @@ -301,6 +306,7 @@ func TestWhitelistHTTP(t *testing.T) { t.Fatalf("expected partial LSAT in response header, got: %v", authHeader) } + _ = resp.Body.Close() // Make sure that if we query an URL that is on the whitelist, we don't // get the 402 response. @@ -318,7 +324,7 @@ func TestWhitelistHTTP(t *testing.T) { } // Ensure that we got the response body we expect. - defer resp.Body.Close() + defer closeOrFail(t, resp.Body) bodyBytes, err := ioutil.ReadAll(resp.Body) if err != nil { t.Fatalf("failed to read response body: %v", err) @@ -374,8 +380,8 @@ func TestWhitelistGRPC(t *testing.T) { InsecureSkipVerify: true, }, } - go server.ListenAndServeTLS(certFile, keyFile) - defer server.Close() + go func() { _ = server.ListenAndServeTLS(certFile, keyFile) }() + defer closeOrFail(t, server) // Start the target backend service also on TLS. tlsConf := cert.TLSConfFromCert(certData) @@ -383,7 +389,7 @@ func TestWhitelistGRPC(t *testing.T) { grpc.Creds(credentials.NewTLS(tlsConf)), } backendService := grpc.NewServer(serverOpts...) - go startBackendGRPC(backendService) + go func() { _ = startBackendGRPC(backendService) }() defer backendService.Stop() // Dial to the proxy now, without any authentication. @@ -396,7 +402,7 @@ func TestWhitelistGRPC(t *testing.T) { // Test making a request to the backend service to an URL where // authentication is enabled. req := &proxytest.HelloRequest{Name: "foo"} - res, err := client.SayHello( + _, err = client.SayHello( context.Background(), req, grpc.WaitForReady(true), ) if err == nil { @@ -425,7 +431,7 @@ func TestWhitelistGRPC(t *testing.T) { // Make the request. This time no error should be returned. req = &proxytest.HelloRequest{Name: "foo"} - res, err = client.SayHelloNoAuth(context.Background(), req) + res, err := client.SayHelloNoAuth(context.Background(), req) if err != nil { t.Fatalf("unable to call service: %v", err) } @@ -492,3 +498,10 @@ func genCertPair(certFile, keyFile string) (*x509.CertPool, } return cp, creds, crt, nil } + +func closeOrFail(t *testing.T, c io.Closer) { + err := c.Close() + if err != nil { + t.Fatal(err) + } +} diff --git a/proxy/service.go b/proxy/service.go index 0c54266..cdb0ca2 100644 --- a/proxy/service.go +++ b/proxy/service.go @@ -102,7 +102,7 @@ func prepareServices(services []*Service) error { for _, service := range services { // Each freebie enabled service gets its own store. if service.Auth.IsFreebie() { - service.freebieDb = freebie.NewMemIpMaskStore( + service.freebieDb = freebie.NewMemIPMaskStore( service.Auth.FreebieCount(), ) } @@ -149,8 +149,10 @@ func prepareServices(services []*Service) error { // not only when the request happens. for _, entry := range service.AuthWhitelistPaths { _, err := regexp.Compile(entry) - return fmt.Errorf("error validating auth whitelist: %v", - err) + if err != nil { + return fmt.Errorf("error validating auth "+ + "whitelist: %v", err) + } } } return nil diff --git a/services.go b/services.go index 8e068c1..969dd4d 100644 --- a/services.go +++ b/services.go @@ -48,7 +48,7 @@ func newStaticServiceLimiter(proxyServices []*proxy.Service) *staticServiceLimit func (l *staticServiceLimiter) ServiceCapabilities(ctx context.Context, services ...lsat.Service) ([]lsat.Caveat, error) { - var res []lsat.Caveat + res := make([]lsat.Caveat, 0, len(services)) for _, service := range services { capabilities, ok := l.capabilities[service] if !ok { @@ -65,7 +65,7 @@ func (l *staticServiceLimiter) ServiceCapabilities(ctx context.Context, func (l *staticServiceLimiter) ServiceConstraints(ctx context.Context, services ...lsat.Service) ([]lsat.Caveat, error) { - var res []lsat.Caveat + res := make([]lsat.Caveat, 0, len(services)) for _, service := range services { constraints, ok := l.constraints[service] if !ok {