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

[BUG] MAC address input not sanitized #37

Closed
synackd opened this issue Jul 1, 2024 · 1 comment · Fixed by #40
Closed

[BUG] MAC address input not sanitized #37

synackd opened this issue Jul 1, 2024 · 1 comment · Fixed by #40
Labels
bug Something isn't working

Comments

@synackd
Copy link
Collaborator

synackd commented Jul 1, 2024

Describe the bug
When making a POST request to BSS to add boot parameters, the list of MAC addresses is not checked for validity. This issue was discovered while using the Ochami CLI to add boot parameters via a YAML file. While BSS should definitely validate the format of the boot parameters payload, it would be helpful for the CLI to also validate before sending it.

To Reproduce
This bug can be reproduced by using ochami-cli to add boot parameters using a YAML file.

  1. Create a "bad" YAML config file, e.g. cluster-boot-config-mlnx.yaml:
    kernel: 'http://10.100.0.1:9000/boot-images/efi-images/compute/mlnx/vmlinuz-4.18.0-553.5.1.el8_10.x86_64'
    initrd: 'http://10.100.0.1:9000/boot-images/efi-images/compute/mlnx/initramfs-4.18.0-553.5.1.el8_10.x86_64.img'
    macs:
    - ec:e7:a7:05:9f:a0
      - ec:e7:a7:02:d9:80
      - ec:e7:a7:02:da:80
      - ec:e7:a7:05:93:88
      - ec:e7:a7:05:93:64
      - ec:e7:a7:02:d8:f0
      - ec:e7:a7:05:95:30
      - ec:e7:a7:05:9f:70
      - ec:e7:a7:05:9e:7c
    params: 'nomodeset ro root=live:http://10.100.0.1:9000/boot-images/compute/mlnx/rocky8.10-compute-mlnx-base-latest ip=dhcp overlayroot=tmpfs overlayroot_cfgdisk=disabled apparmor=0 selinux=0 console=tty0 console=ttyS0,115200 ip6=off ds=nocloud-net;s=http://10.100.0.1:8000/cloud-init/compute/mlnx/'
    Notice that the first MAC address in the macs list is not indented while the rest are indented.
  2. Attempt to add these boot parameters:
    ochami-cli bss --config cluster-boot-config-mlnx.yaml --add-bootparams
    
  3. Retrieve the boot parameters that BSS knows about:
    ochami-cli bss --get-bootparams --format yaml
    
    The MAC address list will be misformatted:
    - initrd: http://10.100.0.1:9000/boot-images/efi-images/compute/mlnx/initramfs-4.18.0-553.5.1.el8_10.x86_64.img
      kernel: http://10.100.0.1:9000/boot-images/efi-images/compute/mlnx/vmlinuz-4.18.0-553.5.1.el8_10.x86_64
      macs:
      - ec:e7:a7:05:9f:a0 - ec:e7:a7:02:d9:80 - ec:e7:a7:02:da:80 - ec:e7:a7:05:93:88
        - ec:e7:a7:05:93:64 - ec:e7:a7:02:d8:f0 - ec:e7:a7:05:95:30 - ec:e7:a7:05:9f:70
        - ec:e7:a7:05:9e:7c
      params: nomodeset ro root=live:http://10.100.0.1:9000/boot-images/compute/mlnx/rocky8.10-compute-mlnx-base-latest
        ip=dhcp overlayroot=tmpfs overlayroot_cfgdisk=disabled apparmor=0 selinux=0 console=tty0
        console=ttyS0,115200 ip6=off ds=nocloud-net;s=http://10.100.0.1:8000/cloud-init/compute/mlnx/

Expected behavior
Catch the misformatted MAC address list and return a 400 Bad Request.

Mitigations
If bad boot parameter data is added, the only way to delete it without restarting is to delete all nodes in BSS whose boot parameters match those from the bad data.

  1. Create the YAML with only the boot parameter data and no nodes, e.g. delete-mlnx-bootparams.yaml:
    kernel: 'http://10.100.0.1:9000/boot-images/efi-images/compute/mlnx/vmlinuz-4.18.0-553.5.1.el8_10.x86_64'
    initrd: 'http://10.100.0.1:9000/boot-images/efi-images/compute/mlnx/initramfs-4.18.0-553.5.1.el8_10.x86_64.img'
    params: 'nomodeset ro root=live:http://10.100.0.1:9000/boot-images/compute/mlnx/rocky8.10-compute-mlnx-base-latest ip=dhcp overlayroot=tmpfs overlayroot_cfgdisk=disabled apparmor=0 selinux=0 console=tty0 console=ttyS0,115200 ip6=off ds=nocloud-net;s=http://10.100.0.1:8000/cloud-init/compute/mlnx/'
  2. Use ochami-cli to delete:
    ochami-cli bss --config delete-mlnx-bootparams.yaml --delete-bootparams
    
@synackd synackd added the bug Something isn't working label Jul 1, 2024
@synackd
Copy link
Collaborator Author

synackd commented Jul 1, 2024

To be clear, is the expected case of an invalid MAC (or Xname or NID) be that the entire request fails?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant