Skip to content

Commit

Permalink
tor+healthcheck: fix healthcheck for multiple services
Browse files Browse the repository at this point in the history
Fixes #6013.
This commit fixes the Tor healthcheck that would previously fail if
there were multiple hidden service registered.
In the controller, we only need to know that our service is contained in
the list of active services. But we can't do a string equality check
since there might be multiple services, comma separated.
  • Loading branch information
guggero committed Nov 23, 2021
1 parent ca0266d commit 5155ebc
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 10 deletions.
14 changes: 11 additions & 3 deletions docs/release-notes/release-notes-0.14.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@

## Bug Fixes

* [Fixed an inaccurate log message during a compaction failure](https://github.com/lightningnetwork/lnd/issues/5937)
* [A bug has been fixed in channeldb that uses the return value without checking error](https://github.com/lightningnetwork/lnd/pull/6012)
* [Fixed an inaccurate log message during a compaction
failure](https://github.com/lightningnetwork/lnd/pull/5961).

* [Fixed a bug in the Tor controller that would cause the health check to fail
if there was more than one hidden service
configured](https://github.com/lightningnetwork/lnd/pull/6016).

* [A bug has been fixed in channeldb that uses the return value without checking
the returned error first](https://github.com/lightningnetwork/lnd/pull/6012).

# Contributors (Alphabetical Order)

* Jamie Turley
* nayuta-ueno
* nayuta-ueno
* Oliver Gugger
2 changes: 1 addition & 1 deletion healthcheck/tor_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func CheckTorServiceStatus(tc *tor.Controller,
return restartTorController(tc, createService)

// If this is not a connection layer error, such as
// ErrServiceNotCreated or ErrServiceIDUnmatch, there's little we can
// ErrServiceNotCreated or ErrServiceIDMismatch, there's little we can
// do but to report the error to the user.
default:
return err
Expand Down
15 changes: 10 additions & 5 deletions tor/cmd_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@ package tor
import (
"errors"
"fmt"
"strings"
)

var (
// ErrServiceNotCreated is used when we want to query info on an onion
// service while it's not been created yet.
ErrServiceNotCreated = errors.New("onion service hasn't been created")

// ErrServiceIDUnmatch is used when the serviceID the controller has
// ErrServiceIDMismatch is used when the serviceID the controller has
// doesn't match the serviceID the Tor daemon has.
ErrServiceIDUnmatch = errors.New("onion serviceIDs not match")
ErrServiceIDMismatch = errors.New("onion serviceIDs don't match")

// ErrNoServiceFound is used when the Tor daemon replies no active
// onion services found for the current control connection while we
Expand Down Expand Up @@ -62,10 +63,14 @@ func (c *Controller) CheckOnionService() error {
}

// Check that our active service is indeed the service acknowledged by
// Tor daemon.
if c.activeServiceID != serviceID {
// Tor daemon. The controller is only aware of a single service but the
// Tor daemon might have multiple services registered (for example for
// the watchtower as well as the node p2p connections). So we just want
// to check that our current controller's ID is contained in the list of
// registered services.
if !strings.Contains(serviceID, c.activeServiceID) {
return fmt.Errorf("%w: controller has: %v, Tor daemon has: %v",
ErrServiceIDUnmatch, c.activeServiceID, serviceID)
ErrServiceIDMismatch, c.activeServiceID, serviceID)
}

return nil
Expand Down
25 changes: 24 additions & 1 deletion tor/cmd_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,30 @@ func TestCheckOnionServiceFailOnServiceIDNotMatch(t *testing.T) {
require.NoError(t, err, "server failed to write")

// Check the error returned from GetServiceInfo is expected.
require.ErrorIs(t, c.CheckOnionService(), ErrServiceIDUnmatch)
require.ErrorIs(t, c.CheckOnionService(), ErrServiceIDMismatch)
}

func TestCheckOnionServiceSucceedOnMultipleServices(t *testing.T) {
t.Parallel()

// Create mock server and client connection.
proxy := createTestProxy(t)
defer proxy.cleanUp()
server := proxy.serverConn

// Assign a fake service ID to the controller.
c := &Controller{conn: proxy.clientConn, activeServiceID: "fakeID"}

// Mock a response with a different serviceID.
serverResp := "250-onions/current=service1,fakeID,service2\n250 OK\n"

// Let the server mocks a given response.
_, err := server.Write([]byte(serverResp))
require.NoError(t, err, "server failed to write")

// No error is expected, the controller's ID is contained within the
// list of active services.
require.NoError(t, c.CheckOnionService())
}

func TestCheckOnionServiceFailOnClosedConnection(t *testing.T) {
Expand Down

0 comments on commit 5155ebc

Please sign in to comment.