[sru] wifi password is not obfuscated in /etc/netplan yaml files

[nikhil kshirsagar]

[IMPACT]

wifi SSID and password (in cleartext) is not obfuscated from /etc/netplan/UUID-yaml file when a sosreport is collected, or even cleaned using sos cleaner or mask.

[TEST PLAN]

Manually create /etc/netplan/9156e614.yaml using this content,

~~~
network:
  version: 2
  wifis:
    NM-9156e614-a3ef-4743-a642-a58ae63193e3:
      renderer: NetworkManager
      match:
        name: "wlp2s0"
      dhcp4: true
      dhcp6: true
      access-points:
        "My Cool Wireless Network SSID":
          auth:
            key-management: "psk"
            password: "MySecretPassword"
          networkmanager:
            uuid: "9156e614-a3ef-4743-a642-a58ae63193e3"
            name: "My Cool Wireless Network SSID"
            passthrough:
              ipv6.addr-gen-mode: "default"
              ipv6.ip6-privacy: "-1"
              proxy._: ""
      networkmanager:
        uuid: "9156e614-a3ef-4743-a642-a58ae63193e3"
        name: "My Cool Wireless Network SSID"
~~~

Collect a sosreport and check if the networking plugin obfuscates the password.

[WHERE PROBLEMS COULD OCCUR]
If due to some exception sosreport does not call the postproc() method on the plugin, leaving the password unobfuscated. I have ruled out this situation based on my analysis of the code, so this SRU seems a reasonably safe patch.

[Launchpad Janitor (janitor)]

This bug was fixed in the package sosreport - 4.5.6-0ubuntu2

---------------
sosreport (4.5.6-0ubuntu2) mantic; urgency=medium

  * d/tests/simple.sh:
    - Correct typo in test_mask to print ip address
      (LP: #2037873)

  * d/p/0002-obfuscate-netplan-ssid-password.patch:
    - Obfuscate SSID password in netplan/XX.yaml files
      (LP: #2037872)

 -- Nikhil Kshirsagar <email address hidden> Tue, 03 Oct 2023 05:43:23 +0000

[Timo Aaltonen (tjaalton)]

Hello nikhil, or anyone else affected,

Accepted sosreport into jammy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/sosreport/4.5.6-0ubuntu1~22.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-jammy to verification-done-jammy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-jammy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Timo Aaltonen (tjaalton)]

Hello nikhil, or anyone else affected,

Accepted sosreport into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/sosreport/4.5.6-0ubuntu1~20.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Timo Aaltonen (tjaalton)]

Hello nikhil, or anyone else affected,

Accepted sosreport into lunar-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/sosreport/4.5.6-0ubuntu1~23.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-lunar to verification-done-lunar. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-lunar. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted sosreport (4.5.6-0ubuntu1~20.04.2) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

sosreport/4.5.6-0ubuntu1~20.04.2 (arm64)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#sosreport

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[nikhil kshirsagar (nkshirsagar)]

Tested this SRU for focal jammy and lunar.

[nikhil kshirsagar (nkshirsagar)]

The autopkgtest failure on focal only on arm64 seems to be a false positive.

573s ######### DONE WITH --mask #########
573s !!! TEST FAILED: IP address not obfuscated in all places !!!
573s /tmp/sosreport_test/sos_commands/logs/journalctl_--no-pager:Oct 06 14:23:25 host0 systemd-hostnamed[571]: Changed host name to 'host-10-43-136-14'
573s /tmp/sosreport_test/sos_commands/logs/journalctl_--no-pager:Oct 06 14:24:33 host0 systemd-hostnamed[454]: Changed host name to 'host-10-43-136-14'
573s /tmp/sosreport_test/sos_commands/logs/journalctl_--no-pager:Oct 06 14:26:47 host0 systemd-hostnamed[453]: Changed host name to 'host-10-43-136-14'
573s /tmp/sosreport_test/sos_commands/logs/journalctl_--no-pager_--boot_-1:Oct 06 14:24:33 host0 systemd-hostnamed[454]: Changed host name to 'host-10-43-136-14'
573s /tmp/sosreport_test/sos_commands/logs/journalctl_--no-pager_--boot:Oct 06 14:26:47 host0 systemd-hostnamed[453]: Changed host name to 'host-10-43-136-14'
573s /tmp/sosreport_test/var/log/syslog:Oct 6 14:23:25 adt-focal-arm64-sosreport-20231006-142159-juju-4d1272-prod-prop systemd-hostnamed[571]: Changed host name to 'host-10-43-136-14'
573s /tmp/sosreport_test/var/log/syslog:Oct 6 14:24:33 adt-focal-arm64-sosreport-20231006-142159-juju-4d1272-prod-prop systemd-hostnamed[454]: Changed host name to 'host-10-43-136-14'
573s /tmp/sosreport_test/var/log/syslog:Oct 6 14:26:47 adt-focal-arm64-sosreport-20231006-142159-juju-4d1272-prod-prop systemd-hostnamed[453]: Changed host name to 'host-10-43-136-14'

When systemd-hotnamed changes the hostname to one that has the ip address (though with dashes not dots) , i.e 'host-10-43-136-14', the grep returns true since it thinks it matches the ip address 10.43.136.14 because we don't escape the dots with backslash.

nikhil@nikhil-Lenovo-Legion-Y540-15IRH-PG0:~/Downloads/test$ cat testgrep
573s /tmp/sosreport_test/sos_commands/logs/journalctl_--no-pager:Oct 06 14:23:25 host0 systemd-hostnamed[571]: Changed host name to 'host-10-43-136-14'
nikhil@nikhil-Lenovo-Legion-Y540-15IRH-PG0:~/Downloads/test$ grep -rI "10.43.136.14" .
./testgrep:573s /tmp/sosreport_test/sos_commands/logs/journalctl_--no-pager:Oct 06 14:23:25 host0 systemd-hostnamed[571]: Changed host name to 'host-10-43-136-14'
nikhil@nikhil-Lenovo-Legion-Y540-15IRH-PG0:~/Downloads/test$ grep -rI "10\.43\.136\.14" .
nikhil@nikhil-Lenovo-Legion-Y540-15IRH-PG0:~/Downloads/test$

 I don't see this as a blocker for this SRU.

[Łukasz Zemczak (sil2100)]

Hey Nikhil! It all makes sense, but I have 2 questions re: this autopkgtest failure:
1) I guess previous version, 4.5.6-0ubuntu1~20.04.1, did not have this failure. What happened that now the test is failing?
2) The IP address per-se is not leaked, but since the host name is changed to be 1:1 the IP address, doesn't this mean we actually *do* leak the IP address, but implicitly? Doesn't this actually surface an issue? Is this a new issue?

[nikhil kshirsagar (nkshirsagar)]

Hi Lukasz,

The previous version was indeed reporting this kind of issue for focal-ppc64el (https://bugs.launchpad.net/ubuntu/+source/sosreport/+bug/2028327/comments/7) but due to the typo issue (Please see https://bugs.launchpad.net/ubuntu/+source/sosreport/+bug/2037873 which is also an SRU on 4.5.6 and should be part of the same release as this bug) the grep results were not printed so we could not be sure it was this exact situation then, i.e of the hostname incorporating the IP address in it using dashes, due to systemd-hostnamed using that as a hotname.

Since it happens only in focal on arm64 here, I did not think it had anything to do with the native sosreport code. I'm not sure who instructs systemd-hostnamed to set the hostname this way, but I reasoned to myself that somehow the autopkgtest infra uses the ipaddress and then sets the hostname of the server using it but with dashes. This made sense because I cannot see why sosreport would ever set hostname, indeed sosreport promises not to make ANY changes to the server its run on.

So I think its definitely not being leaked by sosreport code, i.e I did not think there's something in sosreport that is taking the ip address of the server and setting the hostname of the local test server that autopkgtests run on like what we see in the logs that the echo prints,

573s /tmp/sosreport_test/var/log/syslog:Oct 6 14:24:33 adt-focal-arm64-sosreport-20231006-142159-juju-4d1272-prod-prop systemd-hostnamed[454]: Changed host name to 'host-10-43-136-14'

The sosreport cleaner code will check for ip addresses which contain dots, and not dashes. It would not go to the extent of extracting each number in the ip address and checking whether that string still exists in the sosreport. It's just our autopkgtest grep that treats it as a regex rather than a string, so the dash passes the . regex.

        if [ "$(grep -rI $ip_addr /tmp/sosreport_test/*)" ]; then <---
            add_failure "IP address not obfuscated in all places"
            echo "$(grep -rI $ip_addr /tmp/sosreport/_test/*)"

Please let me know what you think and whether you think its a blocker for this SRU.

[Chris Halse Rogers (raof)]

It seems like this is accidentally exposing a real IP leak? I mean, you're no longer including the literal dotted-decimal representation of the IP, but it seems that hostnamed will, under some default circumstances, set the hostname to $FOO-dashed-decimal-ip¹. If sosreport is meant to redact IP addresses from the report, then it seems reasonable to treat a default hostnamed behaviour as a meaningful IP address leak and thus the fact that the new autopkgtest catches this because the fixed autopkgtest is catching a real bug.

If you don't want to fix it now, that's fine; we can mark this version as bad-test (and the *next* upload will fail autopkgtests unless this is fixed). If you *don't* intend to fix it, then I think you should fix the autopkgtest to only consider dotted-decimal IPs as leaks.

If you intend to fix, it, though, I think the SRU team would mildly prefer it to be fixed in *this* update, so we don't need to push two separate updates to users.

So, please select from one of the following options:
1) We don't want to fix it now, but acknowledge that it's a bug.
  - Please file a bug to track this bug
  - We will badtest this version, so packages which trigger sosreport's autopkgtest won't be blocked
    + Note that this means that sosreport's autopkgtest will no longer catch breakage caused by any changes in your dependencies!
  - We will release this SRU into -updates

2) We don't regard this as a bug
  - Please do a new upload to -proposed that fixes the autopkgtest to not trigger on host-$DASHED_DECIMAL_IP

3) We'll fix it now
  - Please upload a new version to -proposed that fixes the IP leak

Without knowledge of sosreport internals I can't judge the difficulty of (3), but that would be my mild preference.

¹: https://www.freedesktop.org/wiki/Software/systemd/hostnamed/

[nikhil kshirsagar (nkshirsagar)]

Hi Chris,

thank you for the review and the detailed comment.

The sos cleaner regex for IP address is , looking at https://github.com/sosreport/sos/blob/main/sos/cleaner/parsers/ip_parser.py#L15C1-L22C6 ,

class SoSIPParser(SoSCleanerParser):
    """Handles parsing for IP addresses"""

    name = 'IP Parser'
    regex_patterns = [
        # IPv4 with or without CIDR
        r'((?<!(-|\.|\d))([0-9]{1,3}\.){3}([0-9]){1,3}(\/([0-9]{1,2}))?)'
    ]

Changing the regex search in the sosreport cleaner module to search ip addresses by extracting the numerical parts and checking those with dashes instead of dots may require a discussion upstream, simply because the argument could be made why stop just at dashes, and why not underscores, for eg, or any other way the numerical parts may be leaked.

Given the urgency of this SRU (a wifi SSID password is being leaked in cleartext), I feel its better to push this SRU to updates, and we will fix the autopkgtest in the next sos release to only check for actual IP address as a full string. That would be 4.6.2 (https://warthogs.atlassian.net/browse/SET-181). I can open a bug on ubuntu sosreport for that in LP if that sounds OK.

The alternative approach is to badtest this version and I could then open an upstream issue and start a discussion upstream about the regex used to detect IP addresses, and check if upstream would accept changing the regex, in which case we leave our autopkgtest as it is. That would also work (option 1).

Regards,
Nikhil.

[nikhil kshirsagar (nkshirsagar)]

Hello,

upon further discussion, I've opened an upstream issue for this in sos - https://github.com/sosreport/sos/issues/3388

Can we please mark this particular autopkgtest failure as badtest for this SRU so it can be pushed to updates, and we will fix the sos cleaner in the next release to handle such leaked ip addresses where dhcp seems to set the hostname this way.

Regards,
Nikhil.

[Launchpad Janitor (janitor)]

This bug was fixed in the package sosreport - 4.5.6-0ubuntu1~20.04.2

---------------
sosreport (4.5.6-0ubuntu1~20.04.2) focal; urgency=medium

  * d/tests/simple.sh:
    - Correct typo in test_mask to print ip address
      (LP: #2037873)

  * d/p/0002-obfuscate-netplan-ssid-password.patch:
    - Obfuscate SSID password in netplan/XX.yaml files
      (LP: #2037872)

 -- Nikhil Kshirsagar <email address hidden> Wed, 04 Oct 2023 03:41:30 +0000

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for sosreport has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package sosreport - 4.5.6-0ubuntu1~22.04.2

---------------
sosreport (4.5.6-0ubuntu1~22.04.2) jammy; urgency=medium

  * d/tests/simple.sh:
    - Correct typo in test_mask to print ip address
      (LP: #2037873)

  * d/p/0002-obfuscate-netplan-ssid-password.patch:
    - Obfuscate SSID password in netplan/XX.yaml files
      (LP: #2037872)

 -- Nikhil Kshirsagar <email address hidden> Wed, 04 Oct 2023 03:35:14 +0000

[Launchpad Janitor (janitor)]

This bug was fixed in the package sosreport - 4.5.6-0ubuntu1~23.04.2

---------------
sosreport (4.5.6-0ubuntu1~23.04.2) lunar; urgency=medium

  * d/tests/simple.sh:
    - Correct typo in test_mask to print ip address
      (LP: #2037873)

  * d/p/0002-obfuscate-netplan-ssid-password.patch:
    - Obfuscate SSID password in netplan/XX.yaml files
      (LP: #2037872)

 -- Nikhil Kshirsagar <email address hidden> Wed, 04 Oct 2023 03:12:42 +0000

[Łukasz Zemczak (sil2100)]

Released and badtested the failure. But please make sure this is handled properly in a follow up SRU.