Code review comment for ~ddstreet/maas:lp1916860-v2

Revision history for this message
Lee Trager (ltrager) wrote :

I agree there were other things wrong with the original code. While this fixes one of them it doesn't check for gaps in supported ciphers. Look at the unit test I added in my branch[1]. Lets say ipmitool returns the following for supported ciphers and their settings

RMCP+ Cipher Suites : 1,2,3,6,7,8,11,12,15,16,17
Cipher Suite Priv Max : aaaXXXXXXXXXXaa

This means the BMC doesn't IPMI ciphers 0, 4, 5, 9, 10, 13, and 14. It has options for all supported ciphers plus 4 vendor ciphers which are not listed. ciphers 1, 2, 3 and two vendor ciphers are currently enabled while the others are disabled.

This branch will result push the settings string "XXXXXXXXXXXXXXX" which disables all ciphers making remote access impossible. The correct cipher string is "XXaXXXXXXXaXXaa" which means disable the insecure ciphers 1 and 2, keep 3 and the two vendor ciphers as is, and enable cipher 17 for MAAS to use.

[1] https://code.launchpad.net/~ltrager/maas/+git/maas/+merge/399134

« Back to merge proposal