Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions autopilotserver/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,11 @@ func (c *Client) ListFeatures(ctx context.Context) (map[string]*Feature,
perms := unmarshalPermissions(feature.PermissionsList)
rules := unmarshalRules(feature.Rules)
features[i] = &Feature{
Name: feature.Name,
Description: feature.Description,
Permissions: perms,
Rules: rules,
Name: feature.Name,
Description: feature.Description,
Permissions: perms,
Rules: rules,
Configuration: feature.Configuration,
}
}

Expand Down
3 changes: 3 additions & 0 deletions autopilotserver/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ type Feature struct {
// Rules is a list of all the firewall that must be specified for this
// feature.
Rules map[string]*RuleValues

// Configuration is a JSON-serialized configuration of the feature.
Configuration []byte
}

// RuleValues holds the default value along with the sane max and min values
Expand Down
199 changes: 105 additions & 94 deletions autopilotserverrpc/autopilotserver.pb.go

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions autopilotserverrpc/autopilotserver.proto
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ message Feature {
A list of the permissions that the feature will require to operate.
*/
repeated Permissions permissions_list = 4;

/*
The JSON-marshaled representation of a feature's configuration.
*/
bytes configuration = 5;
}

message Rule {
Expand Down
27 changes: 25 additions & 2 deletions cmd/litcli/autopilot.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package main
import (
"context"
"encoding/hex"
"fmt"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -65,6 +66,12 @@ var addAutopilotSessionCmd = cli.Command{
"perform actions on. In the " +
"form of: peerID1,peerID2,...",
},
cli.StringSliceFlag{
Name: "configuration",
Usage: `per feature JSON-serialized configuration, ` +
`expected format: {"version":0,"option1":` +
`"parameter1","option2":"parameter2",...}`,
},
},
}

Expand Down Expand Up @@ -216,11 +223,27 @@ func initAutopilotSession(ctx *cli.Context) error {
}
}

configs := ctx.StringSlice("configuration")
features := ctx.StringSlice("feature")

if len(configs) > 0 && len(features) != len(configs) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we just add a comment here explaining that as soon as one config flag is set, we need all of them set as else we dont know which config goes with which feature

return fmt.Errorf("number of features and configurations " +
"must match")
}

featureMap := make(map[string]*litrpc.FeatureConfig)
for _, feature := range ctx.StringSlice("feature") {
for i, feature := range ctx.StringSlice("feature") {
var config []byte

// We allow empty configs, to signal the usage of the default
// configuration when the session is registered.
Comment on lines +238 to +239
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome 😎

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add this to the flag description too? also should mention the importance of the order used when specifying the config flags

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw - I'd say this is a breaking change since users now have to set the new flag. Not a bad thing since I think most users dont register for autopilot sessions through the CLI but we should at least mention this in the release notes.

perhaps worth adding a tracking issue for things like this so that we can just post a comment there if we want to remind ourselves about anything that should be added to the readme or release notes for the next release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added all the suggestions, hope we have a better description now :). is it a breaking change? we can leave out all config flags and it will use the defaults

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah - you are right. Sorry about that 🙈

if len(configs) > 0 && configs[i] != "{}" {
config = []byte(configs[i])
}

featureMap[feature] = &litrpc.FeatureConfig{
Rules: ruleMap,
Config: nil,
Config: config,
}
}

Expand Down
40 changes: 40 additions & 0 deletions firewall/privacy_mapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"errors"
"fmt"
"math/big"
"regexp"
"strings"
"time"

"github.com/lightninglabs/lightning-terminal/firewalldb"
Expand Down Expand Up @@ -833,3 +835,41 @@ func CryptoRandIntn(n int) (int, error) {

return int(nBig.Int64()), nil
}

// ObfuscateConfig alters the config string by replacing pubkeys with random
// values and updates the privacy pair map.
func ObfuscateConfig(privacyPairs map[string]string, configB []byte) ([]byte,
Comment on lines +839 to +841
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very cool! should we also match on channel IDs/outpoints?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, will add that 👍!

error) {

// The config is a JSON blob, so we interpret it as a string to detect
// any patterns we want to replace.
config := string(configB)

// Replace all pubkeys with a pseudo-random string.
pubKeyRegex := regexp.MustCompile(`[a-fA-F0-9]+`)
matches := pubKeyRegex.FindAllString(config, -1)

for _, match := range matches {
// A pubkey is 66 characters long.
if len(match) != 66 {
continue
}

if _, ok := privacyPairs[match]; ok {
continue
}

replacement, err := firewalldb.NewPseudoStr(len(match))
if err != nil {
return nil, err
}
privacyPairs[match] = replacement
}

// Replace all occurances.
for k, v := range privacyPairs {
config = strings.ReplaceAll(config, k, v)
}

return []byte(config), nil
}
62 changes: 62 additions & 0 deletions firewall/privacy_mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,68 @@ func TestHideBool(t *testing.T) {
require.False(t, val)
}

// TestObfuscateConfig tests that we substitute substrings in the config
// correctly.
func TestObfuscateConfig(t *testing.T) {
tests := []struct {
name string
config []byte
expectedPubKeys int
}{
{
name: "empty",
},
{
name: "pubkeys",
config: []byte(`{"version":1,"pubkeys":` +
`"d23da57575cdcb878ac191e1e0c8a5c4f061b11cfdc7a8ec5c9d495270de66fdbf,` +
`0e092708c9e737115ff14a85b65466561280d77c1b8cd666bc655536ad81ccca85,` +
`DEAD2708c9e737115ff14a85b65466561280d77c1b8cd666bc655536ad81ccca85,` +
`586b59212da4623c40dcc68c4573da1719e5893630790c9f2db8940fff3efd8cd4"}`),
expectedPubKeys: 4,
},
{
name: "too short pubkey",
config: []byte(`{"version":1,"pubkeys":` +
`"d23da57575cdcb878ac191e1e0c8a5c4f061b11"}`),
expectedPubKeys: 0,
},
{
name: "too long pubkey",
config: []byte(`{"version":1,"pubkeys":` +
`"586b59212da4623c40dcc68c4573da1719e5893630790c9f2db8940fff3efd8cd4dead"}`),
expectedPubKeys: 0,
},
{
name: "nonhex",
config: []byte(`{"version":1,"pubkeys":` +
`"x86b59212da4623c40dcc68c4573da1719e5893630790c9f2db8940fff3efd8cd4"}`),
expectedPubKeys: 0,
},
}

for _, tt := range tests {
tt := tt

t.Run(tt.name, func(t *testing.T) {
privacyPairMap := make(map[string]string)
config, err := ObfuscateConfig(
privacyPairMap, tt.config,
)
require.NoError(t, err)

// We expect the config to be obfuscated only if there
// are pubkeys.
if tt.expectedPubKeys > 0 {
require.NotEqual(t, config, tt.config)
}
require.Equal(t, len(tt.config), len(config))
require.Equal(t, tt.expectedPubKeys,
len(privacyPairMap))
})
}
}

// mean computes the mean of the given slice of numbers.
func mean(numbers []uint64) uint64 {
sum := uint64(0)
Expand Down
115 changes: 63 additions & 52 deletions litrpc/lit-autopilot.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion litrpc/lit-autopilot.proto
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ message Feature {
feature rules set contains a rule that Litd is unaware of.
*/
bool requires_upgrade = 5;

/*
Represents the JSON-serialized configuration of a feature.
*/
string configuration = 6;
}

message RuleValues {
Expand Down Expand Up @@ -191,4 +196,4 @@ message Permissions {
A list of the permissions required for this method.
*/
repeated MacaroonPermission operations = 2;
}
}
11 changes: 11 additions & 0 deletions litrpc/lit-autopilot.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@
"requires_upgrade": {
"type": "boolean",
"description": "A boolean indicating if the user would need to upgrade their Litd version in\norder to subscribe to the Autopilot feature. This will be true if the\nfeature rules set contains a rule that Litd is unaware of."
},
"configuration": {
"type": "string",
"description": "Represents the JSON-serialized configuration of a feature."
}
}
},
Expand Down Expand Up @@ -578,6 +582,13 @@
"type": "string",
"format": "uint64",
"description": "The unix timestamp indicating the time at which the session was revoked.\nNote that this field has not been around since the beginning and so it\ncould be the case that a session has been revoked but that this field\nwill not have been set for that session. Therefore, it is suggested that\nreaders should not assume that if this field is zero that the session is\nnot revoked. Readers should instead first check the session_state field."
},
"feature_configs": {
"type": "object",
"additionalProperties": {
"type": "string"
},
"description": "Configurations for each individual feature mapping from the feature name to\na JSON-serialized configuration."
}
}
},
Expand Down
Loading