bugGNU FreeIPMI - Bugs: bug #38790, Invalid "Confidentiality Pad...

 
 

bug #38790: Invalid "Confidentiality Pad Length" value in encrypted RMCP+ packets

Submitted by:  Rob Swindell <rswindell>
Submitted on:  Sun 21 Apr 2013 12:23:28 AM UTC  
 
Category: libfreeipmiSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Improper Behaviour
Status: NonePrivacy: Public
Assigned to: Albert Chu <chu11>Open/Closed: Closed
Operating System: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Thu 25 Apr 2013 06:27:22 PM UTC, comment #2:

I was able to reproduce with --set-asset-tag=12313 (not sure why it did not with your example). I'm surprised this bug lingered so long. I guess most vendors do not check the pad.

I did a different patch though, and found another location in the code that needed to be changed. Here's the patch I commited for 1.2.6. I've of course given credit to you & Broadcom in the ChangeLog for finding the bug and the fix location.

===================================================================
--- libfreeipmi/interface/ipmi-rmcpplus-interface.c (revision 9608)
+++ libfreeipmi/interface/ipmi-rmcpplus-interface.c (working copy)
@@ -739,7 +739,7 @@
uint8_t iv[IPMI_CRYPT_AES_CBC_128_IV_LENGTH];
int iv_len;
uint8_t payload_buf[IPMI_MAX_PAYLOAD_LENGTH];
- uint8_t pad_len;
+ uint8_t pad_len, pad_tmp;
int payload_len, cipher_keylen, cipher_blocklen, encrypt_len;

/* Note: Confidentiality Key for AES_CBS_128 is K2 */
@@ -808,7 +808,11 @@
/* Pad the data appropriately */

/* +1 is for the pad length field */
- pad_len = IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH - ((payload_len + 1) % IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH);
+ pad_tmp = ((payload_len + 1) % IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH);
+ if (pad_tmp)
+ pad_len = IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH - pad_tmp;
+ else
+ pad_len = 0;

if ((payload_len + pad_len + 1) > IPMI_MAX_PAYLOAD_LENGTH)
{
@@ -821,8 +825,8 @@
unsigned int i;
for (i = 0; i < pad_len; i++)
payload_buf[payload_len + i] = i + 1;
- payload_buf[payload_len + pad_len] = pad_len;
}
+ payload_buf[payload_len + pad_len] = pad_len;

/* +1 for pad length field */
if ((encrypt_len = crypt_cipher_encrypt (IPMI_CRYPT_CIPHER_AES,

Albert Chu <chu11>
Project AdministratorIn charge of this item.
Thu 25 Apr 2013 01:31:09 AM UTC, comment #1:

Here's the patch:

Index: libfreeipmi/interface/ipmi-rmcpplus-interface.c
===================================================================
--- libfreeipmi/interface/ipmi-rmcpplus-interface.c (revision 9608)
+++ libfreeipmi/interface/ipmi-rmcpplus-interface.c (working copy)
@@ -808,7 +808,7 @@
/* Pad the data appropriately */

/* +1 is for the pad length field */
- pad_len = IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH - ((payload_len + 1) % IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH);
+ pad_len = (IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH - ((payload_len + 1) % IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH)) & (IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH-1);

if ((payload_len + pad_len + 1) > IPMI_MAX_PAYLOAD_LENGTH)
{

Rob Swindell <rswindell>
Sun 21 Apr 2013 12:23:28 AM UTC, original submission:

Example:

ipmi-dcmi -h 192.168.1.19 --set-asset-tag=123456789012345678901 -Dlan20 -I3 --debug

...

192.168.1.19: [ BYTE ARRAY ... ] = confidentiality_trailer[17B]
192.168.1.19: [ 01h 02h 03h 04h 05h 06h 07h 08h ]
192.168.1.19: [ 09h 0Ah 0Bh 0Ch 0Dh 0Eh 0Fh 10h ]
192.168.1.19: [ 10h ]

The last byte there is the "Confidentiality Pad Length" (10h) which should not exceed 15 (0x0f). See IPMI 2.0 Table 13-20 for details.

This bug causes the affected packets to be rejected/ignored by strictly-conforming IPMI 2.0 servers/BMCs.

The fix is to change the following line in ipmi-rmcpplus-interface.c:

From this:
pad_len = IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH - ((payload_len + 1) % IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH);

To this:
pad_len = (IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH - ((payload_len + 1) % IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH)) & (IPMI_CRYPT_AES_CBC_128_BLOCK_LENGTH-1);

This limits the "Confidentiality Pad Length" to the range of 0 to 15 (the legal range). The resulting packet (in the failing cases) is now 16 byte shorter and conforms to IPMI 2.0.

-Rob Swindell

Rob Swindell <rswindell>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by chu11 (Posted a comment)
  • -unavailable- added by rswindell (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 2 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 25 Apr 2013 06:40:15 PM UTCchu11Open/ClosedOpen=>Closed
    Thu 25 Apr 2013 06:27:22 PM UTCchu11Assigned toNone=>chu11

    Back to the top


    Powered by Savane 3.1-cleanup1