Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strip-private: T6355: rework the strip-private filter #3475

Closed
wants to merge 1 commit into from

Conversation

dmbaturin
Copy link
Member

The original | strip-private has two issues:

  1. It removes way too much: replacing every IP address with xx.xx.10.20 makes it impossible to make any sense of what's going on. The script has options to only strip specific things, but they were never exposed in the CLI.
  2. It uses regexes, which can lead to false positives and unintentionally remove absolutely non-secret data.

In the future, "secret" will be a property of the node in the config tree and "strip private" will be a feature of the config rendered. But for now we can at least:

  • Only remove passwords and private keys.
  • Use an explicit list of secret paths.

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe): behavior change.

Component(s) name

strip-private filter.

Proposed changes

How to test

My test config:

vyos@vyos# show | strip-private 
high-availability {
    vrrp {
        group Foo {
            address 10.4.0.1/24 {
            }
            authentication {
                password "<PASSWORD REDACTED>"
                type "plaintext-password"
            }
            interface "eth0"
            vrid "100"
        }
    }
}
interfaces {
    ethernet eth0 {
        address "dhcp"
        hw-id "08:00:27:64:ce:12"
    }
    ethernet eth1 {
        hw-id "08:00:27:7b:d1:70"
    }
    ethernet eth2 {
        hw-id "08:00:27:0c:45:17"
    }
    loopback lo {
    }
}
pki {
    ca foo {
        certificate "MIIDnTCCAoWgAwIBAgIUFG50M50A+IY23eL0LbAWClg8nqEwDQYJKoZIhvcNAQELBQAwVzELMAkGA1UEBhMCR0IxEzARBgNVBAgMClNvbWUtU3RhdGUxEjAQBgNVBAcMCVNvbWUtQ2l0eTENMAsGA1UECgwEVnlPUzEQMA4GA1UEAwwHdnlvcy5pbzAeFw0yNDA1MTYyMjEzMjhaFw0yOTA1MTUyMjEzMjhaMFcxCzAJBgNVBAYTAkdCMRMwEQYDVQQIDApTb21lLVN0YXRlMRIwEAYDVQQHDAlTb21lLUNpdHkxDTALBgNVBAoMBFZ5T1MxEDAOBgNVBAMMB3Z5b3MuaW8wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDBaO/ljzvAhPpk56owvQJu0Cmz/1wxY/ap3zBofSBq00zXfaZwO6xSS5ekjp/xhq1vyJ5Nfv03cVlq3prW4yN8fo6vel1MdWmxXVGMtstHb+7DRgLK/hgfnk0M2qbl9XIonTMP9tX1c6N8vbtUgOcaHUNVYeygezC145kTjS5uo24RRYWeJXakITYcYIrGU5UUVNGAx+cJpAz4DPR3LJAT1WWhWMW0jkIDXuIj8BmrHgyvlrTXmj1XvXcBQ1j3M/vwbLYPhcJnzwfgjwD37H6Z/Al2RReynYDSsxN4UcZE5IE0sUeGWdfdqZmhpdFMol9Gv72IKXxGrBc4t7QyYXq5AgMBAAGjYTBfMA8GA1UdEwEB/wQFMAMBAf8wDgYDVR0PAQH/BAQDAgGGMB0GA1UdJQQWMBQGCCsGAQUFBwMCBggrBgEFBQcDATAdBgNVHQ4EFgQUBeNzDtl5bcr/CeGBGuFROABPpIkwDQYJKoZIhvcNAQELBQADggEBABMW74QnzBgmOHGmxtTpmehSAgBGSSpghgYw9mlLdY7cIJ4rRSp1jVcU0yaiBKjkzfMRTuuVzXV7O2Ux1J2HsmSQYHfpAL/H0CIu87is0DiAuFAhaBRyWirccipL27M4BikFGLuAfpo2gB+aBNYDXl6e7cAR4YeQvENp2K1iOe8iT5CheQgHUKpsXo0UYKv6+a+WxMpkAxY/a7onUjP4sh/8Swi0c4bYvJupbbSzgCYr09hgBCLRQjzA8tcWsmG3iCzvfs48h0/uXmS6e/qsAPykjTHF26pnzGr9/cyUnNURgQrOXGsQGmqMkUVy5tIMnUTE2bKPBQ///fTl6eGU6NI="
        private {
            key "<KEY DATA REDACTED>"
        }
    }
    key-pair test {
        private {
            key "<KEY DATA REDACTED>"
        }
        public {
            key "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAw1SxRlVk11uAMirhE3vlDXT8rtCDNDZW2k3qidZ8dvYiXkUlEhEntAGwQSTD7uH62jZm+NDipH4SlHES+vcFF9EnkKF/pDvIpt6hYHrkU+pylyH23QjLkV/OE05xhkMqw+eU47Te6MZj4YM0pIk3HdoYDmbYGCI4qXN04tV1IYrYNg0rWALRSI5vR680iamAUTWJAuCA7d0k3uenD+5qNAGqeXbD5+X8DzP/+dEu4zsBNC9zk/oN+uWBhC3OZElVAgMTDszvISimpffmV+QEWSO+1hc9Yl/WRMN6pwD0peqi4BMAp4UnpZLcRARBusQgXSdho4esXxizNTYsbe0kRQIDAQAB"
        }
    }
    openvpn {
        shared-secret ovpn-test {
            key "<KEY DATA REDACTED>"
            version "1"
        }
    }
}
protocols {
    isis {
        area-password {
            plaintext-password "<PASSWORD REDACTED>"
        }
        interface eth0 {
            password {
                plaintext-password "<PASSWORD REDACTED>"
            }
        }
        net "49.0001.1921.6800.1002.00"
    }
    rip {
        interface eth0 {
            authentication {
                plaintext-password "<PASSWORD REDACTED>"
            }
        }
    }
}
service {
    https {
        api {
            keys {
                id Foo {
                    key "<PASSWORD REDACTED>"
                }
            }
        }
    }
    monitoring {
        telegraf {
            prometheus-client {
                authentication {
                    password "<PASSWORD REDACTED>"
                    username "admin"
                }
            }
        }
    }
    ntp {
        allow-client {
            address "127.0.0.0/8"
            address "169.254.0.0/16"
            address "10.0.0.0/8"
            address "172.16.0.0/12"
            address "192.168.0.0/16"
            address "::1/128"
            address "fe80::/10"
            address "fc00::/7"
        }
        server time1.vyos.net {
        }
        server time2.vyos.net {
        }
        server time3.vyos.net {
        }
    }
    pppoe-server {
        authentication {
            local-users {
                username user {
                    password "<PASSWORD REDACTED>"
                }
            }
        }
        client-ip-pool Foo {
            range "10.4.0.0-10.4.0.10"
        }
        default-pool "Foo"
        interface eth1 {
        }
    }
    ssh
}
system {
    config-management {
        commit-revisions "100"
    }
    conntrack {
        modules {
            ftp
            h323
            nfs
            pptp
            sip
            sqlnet
            tftp
        }
    }
    console {
        device ttyS0 {
            speed "115200"
        }
    }
    host-name "vyos"
    login {
        user vyos {
            authentication {
                encrypted-password "<PASSWORD REDACTED>"
                plaintext-password ""
                public-keys ssh-test {
                    key "AAAAB3NzaC1yc2EAAAADAQABAAABAQCzWd3tvQlzPo68BFQzJ4iEDHH94D0x2c0MT5+9F0qPZuJyEV+ATyRf7Z80ijLxNHeoIE/bBkQVpL84a8FJO0DRqbzxt/oFi88ZPlrqhcjpj39q92a3ZAh2FGq5wEQ1HQ2zjSrLEKhSMqgSipr+iR8Kaazyv8tZ1m8yNchA0G33uxSf8dahJXfbmPA2vEsH0CiwuEpLpZXbNY2W3EFcwwuz+K5Lln+zWBlizytjbUSQGWmmcGFLKQ8YdSLtFz/PxRhBuk9ja74a36XZeZvC9E77Zmdbh1qsGEaW5SYozJS+jSOUCeY9MBEMPkGSC745Bmu6+yngmBxdYLPwkl0S1hOb"
                    type "ssh-rsa"
                }
                public-keys ssh-test-2 {
                    key "AAAAB3NzaC1yc2EAAAADAQABAAABAQCrohI5nQ3CDc6eFMZn2Zzrks1Yl4cz6DvJPD4vni8vXJCo89Oc8bdTTvkUTPVmMligqYzsYfTPv9WoD8dP5RT4FR/4ZUCAwWz2xwqwi07e3fu/qbMYOYzPbqHHwYsMzsKzgXgjSnFKyj8MAIJQs2QX4GbsxF71kQI/zukWNlArK10nCLOWZFCAllUvXOr9bbSezM18Q362nzhxuGucBPjyZJhL8WOMHTVWT85e5KYshT110oo3FWC5ahulfbi805BldJRZuu9loTOAgENai9Jl8ARxD9vC8pTJtakaDn3tnQd09F1OCOGni6aSvw0X0IipnfFzezMtcJitkL0FZRIb"
                    type "ssh-rsa"
                }
            }
        }
    }
    syslog {
        global {
            facility all {
                level "info"
            }
            facility local7 {
                level "debug"
            }
        }
    }
}
vpn {
    ipsec {
        authentication {
            psk Foo {
                id "192.0.2.1"
                secret "<PASSWORD REDACTED>"
            }
        }
        esp-group Foo {
            proposal 1 {
                encryption "3des"
                hash "md5"
            }
        }
        ike-group Foo {
            key-exchange "ikev2"
            proposal 1 {
                encryption "3des"
                hash "md5"
            }
        }
        remote-access {
            connection Bar {
                authentication {
                    local-users {
                        username dmb {
                            password "<PASSWORD REDACTED>"
                        }
                    }
                    pre-shared-secret "<PASSWORD REDACTED>"
                    server-mode "pre-shared-secret"
                }
                esp-group "Foo"
                ike-group "Foo"
                local-address "192.168.56.107"
            }
        }
    }
    l2tp {
        remote-access {
            authentication {
                local-users {
                    username dmbaturin {
                        password "<PASSWORD REDACTED>"
                    }
                }
            }
            client-ip-pool Foo {
                range "10.2.0.0-10.2.0.10"
            }
            default-pool "Foo"
            ipsec-settings {
                authentication {
                    mode "pre-shared-secret"
                    pre-shared-secret "<PASSWORD REDACTED>"
                }
                esp-group "Foo"
                ike-group "Foo"
            }
            outside-address "192.168.56.107"
        }
    }
    pptp {
        remote-access {
            authentication {
                local-users {
                    username dmbaturin {
                        password "<PASSWORD REDACTED>"
                    }
                }
            }
            client-ip-pool Foo {
                range "10.0.0.0-10.0.0.10"
            }
            default-pool "Foo"
        }
    }
}

Smoketest result

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@c-po c-po requested review from a team, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team May 17, 2024 15:31
@dmbaturin dmbaturin requested a review from a team as a code owner May 22, 2024 15:08
Copy link
Member

@c-po c-po left a comment

Choose a reason for hiding this comment

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

configuration commands

I can not call it to filter the configuration commands.

[email protected]# run show configuration commands | strip-private
Traceback (most recent call last):
  File "/usr/libexec/vyos/strip-private.py", line 156, in <module>
    stripped_config = strip_private(config_source)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/libexec/vyos/strip-private.py", line 115, in strip_private
    ct = vyos.configtree.ConfigTree(config_source)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/configtree.py", line 155, in __init__
    raise ValueError("Failed to parse config: {0}".format(msg))
ValueError: Failed to parse config: Syntax error on line 1, character 16: Invalid syntax.

VRF

I also do not see any path handling VRFs, but it should support also filtering the VRF defined configuration.

Example:

set protocols bgp neighbor 4.5.6.7 address-family ipv4-unicast
set protocols bgp neighbor 4.5.6.7 password 'baz'
set protocols bgp neighbor 4.5.6.7 remote-as '555'
set protocols bgp system-as '200'

set vrf name red protocols bgp neighbor 1.2.3.4 address-family ipv4-unicast
set vrf name red protocols bgp neighbor 1.2.3.4 password 'foobar123'
set vrf name red protocols bgp neighbor 1.2.3.4 remote-as '400'
set vrf name red protocols bgp system-as '200'
set vrf name red table '10000'

Result:

[email protected]# show | strip-private
...
protocols {
    bgp {
        neighbor 4.5.6.7 {
            address-family {
                ipv4-unicast
            }
            password "<PASSWORD REDACTED>"
            remote-as "555"
        }
        system-as "200"
    }
}

But for the VRF context I get:

vrf {
    name red {
        protocols {
            bgp {
                neighbor 1.2.3.4 {
                    address-family {
                        ipv4-unicast
                    }
                    password "foobar123"
                    remote-as "400"
                }
                system-as "200"
            }
        }
        table "10000"
    }
}

to only remove passwords and use an explicit list of secret paths
@c-po
Copy link
Member

c-po commented May 23, 2024

[email protected]:~$ show configuration commands | strip-private
strip-private filter error: could not parse the input as a VyOS config
Hint: if want to see your config as commands, run "show | strip-private | commands"

This used to work in 1.4.0-epa2 and also 1.3

It yet not masks the OSPF key

[email protected]# show | strip-private | commands | match md5-key
set protocols ospf interface eth0.201 authentication md5 key-id 10 md5-key 'ospfvyos'

@dmbaturin
Copy link
Member Author

We cannot implement a proper version until we figure out a way to pass the config path to the filter, and there seems to be no way to see if the config is at the top level or not.

The situation in question is like show protocols | strip-private — it will not strip anything because the filter doesn't know that every path in the config it got from stdin is relative to protocols.

The concept of secret nodes in the reference tree will make it all radically simpler, but only when we figure out UI for givine parameters to show, or make a custom shell that makes some functions look like filters even though they are not.

@dmbaturin dmbaturin closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants