Prevent from wrongly interpreting cc: in a BOOTIF=mac as config yaml

[jjqq2013]

When a mac address contains cc, such as

01:02:03:04:cc:f4

and when the kernel command line is

.... BOOTIF=01:02:03:04:cc:f4 ...

then cloudinit will wrongly think that

f4 ...

are a cloudinit config yaml, then the yaml parser fails.

Normally this will not have a problem, but if the kernel command line embeds an important cloudinit instruction such as runcmd, then it will not be run, such as

... ds=nocloud cc: datasource_list: [NoCloud] end_cc cc: runcmd: [[echo,a]] end_cc BOOTIF=01:02:03:04:cc:f4

I have tested on a real UEFI PXE booted liveos.

Proposed Commit Message

summary: no more than 70 characters

A description of what the change being made is and why it is being
made, if the summary line is insufficient.  The blank line above is
required. This should be wrapped at 72 characters, but otherwise has
no particular length requirements.

If you need to write multiple paragraphs, feel free.

Fixes GH-NNNNN (GitHub Issue number. Remove line if irrelevant)
LP: #NNNNNN (Launchpad bug number. Remove line if irrelevant)

Additional Context

Test Steps

Checklist:

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

[igalic]

let's add a comment and/or a test case to ensure we know why this exists

[TheRealFalcon]

Since it is valid for the line to begin with cc:, this won't work in that case.

I'm tempted to replace this entire section with regex parsing. Is that something you'd be interested in doing?

[github-actions]

Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close.

If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon.

(If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.)

[TheRealFalcon]

@jjqq2013 do you have any interest in making the requested change? Otherwise, I can take the effort over

[holmanb]

Doesn't cc: get parsed over in ds-identify too? We might want to check if a related bug exists there too.

[jjqq2013]

@TheRealFalcon hi, sorry I don't have time to create a full PR, It would be great if you can take over this ticket.

[jjqq2013]

@holmanb

Doesn't cc: get parsed over in ds-identify too? We might want to check if a related bug exists there too.

When I test the case ... ds=nocloud cc: datasource_list: [NoCloud] end_cc cc: runcmd: [[echo,a]] end_cc BOOTIF=01:02:03:04:cc:f4, I did not find any error in ds-identity related output.
I remember that when ds-identify get called, cloud-init has finished the parsing of command line, so I think the issue does not affect in ds-identy itself.

[jjqq2013]

Thanks all for taking a look at this issue.

@TheRealFalcon

Since it is valid for the line to begin with cc:, this won't work in that case.
I'm tempted to replace this entire section with regex parsing. Is that something you'd be interested in doing?

The parser read the whole kernel command line, such as
case1:

... BOOTIF=01:02:03:04:cc:f4 ...

case2:

... cc: cloudinit_yml_here

case3:

cc: cloudinit_yml_here

You can see that case1 and case2 contains cc:, note the preceding space.

The case3 does not exist unless when someone insanely produced a kernel command line which starts with exactly cc:, I don't think that is possible in normal Linux boot protocol.

So the cc: works well (note the preceding space).

And since this issue is a rare issue, no much system pass critical thing such as runcmd via kernel command line,
I think it is not worthy putting much effort to fix this issue with more comprehensive method such as considering all <OTHER_WHITE_SPACE>cc:.

I.e., just inserting a single space fixes this issue pratically.

Just need someone to finish the whole process of a PR, such as passing tests.

[aciba90]

Let me take over this one.

[TheRealFalcon]

Thanks for the additional context @jjqq2013 .

@aciba90 , feel free!

[holmanb]

The case3 does not exist unless when someone insanely produced a kernel command line which starts with exactly cc:, I don't think that is possible in normal Linux boot protocol.

@jjqq2013 Not only is it possible, it is trivial. Consider:

root@me:~# sudo kexec -l /boot/vmlinuz-5.15.0-1017-kvm --append 'cc: cloudinit_yml_here cc_end root=PARTUUID=2fec7223-75cc-463c-a987-85456171d574 ro console=tty1 console=ttyS0 panic=-1' --initrd=/boot/initrd.img-5.15.0-1017-kvm
root@me:~# kexec -e

(Log back into the instance with the new running kernel)

root@me:~# cat /proc/cmdline 
cc: cloudinit_yml_here cc_end root=PARTUUID=2fec7223-75cc-463c-a987-85456171d574 ro console=tty1 console=ttyS0 panic=-1

Therefore, as @TheRealFalcon already said, it is valid for the line to begin with cc:. Lets just fix it the right way.

[jjqq2013]

Not only is it possible, it is trivial. Consider:
@holmanb thanks for pointing this. Yes, also possible in grub.cfg, normally the kernel command line begins with root=..., it is possible that if someone insert 'cc:....' before it, people are just fear of doing that.

linux   /boot/vmlinuz-5.4.0-139-generic root=UUID=8e0d54b3-93b0-46e2-99ff-c9f4fbbc9adf ro  console=tty0 console=ttyS0,115200

Anyway, it would be beautiful if we can cover all the cases.

[aciba90]

Anyway, it would be beautiful if we can cover all the cases.

This is being addressed in #4541.

Please, @jjqq2013, see #4541 (review). Thanks.

[TheRealFalcon]

Addressed in #4541 . Closing this one.