[Freeipmi-devel] Re: [llnl-devel] checking chksums, possible framework change needed??

[nancy . dirgo]

AB,

TECH SPECS!!!! :-)

Thanks.
Nancy

> I always kept changing the order of chksum and comp_code
> validation. And I decided to postpone it until disaster strikes again.
>
> Let us fix it now.
>
> Validity of the comp_code itself depends on the chksum. But location
> of chksum depends on comp_code. There seems to be a cyclic dependency.
>
> I think it is OK, not to validate the chksum if comp_code is not
> success. In that case we can check comp_code first.
> OR
> We can double-check the chksum and then validate comp_code
> like this
>
> if (pkt.chksum != ipmi_chksum((u_int8_t *) &pkt.msg.rs,
>                   (sizeof (pkt.msg.rs) + sizeof(pkt.data))))
> {
>   ipmi_err_rs_t err_rs;
>   memcpy (&err_rs, &pkt.msg.rs, sizeof (ipmi_err_rs_t));
>   if (err_rs.chksum != ipmi_chksum((u_int8_t *) &err_rs.msg.rs,
>                   (sizeof (err_rs.msg.rs) + sizeof(err_rs.data))))
>   {
>     REALLY BAD CHKSUM
>   }
> }
> AND THEN VALIDATE COMP_CODE
>
> where
>
> typedef struct ipmi_err_rs
> {
>   ipmi_session_t session;
>   ipmi_rs_msg_t msg;
>   struct {
>   // just for writing generic chksum macros
>   } data;
>   ipmi_chksum_t chksum;
> } ipmi_err_rs_t;
>
> There is also one more approach - using "c union", but too may nesting of
> structures aren't elegant.
>
> Happy Hacking
> -ab
>
> Albert Chu <address@hidden> writes:
>
>>Hey AB,
>>
>>In your ipmi-wrapper.c code, you do checksum checks like the following:
>>
>>if (pkt.chksum != ipmi_chksum((u_int8_t *) &pkt.msg.rs,
>>                  (sizeof (pkt.msg.rs) + sizeof(pkt.data))))
>>{
>>     // yell and scream about checksum errors
>>}
>>
>>However this is incorrect.  The reason is that on a completion code
>>error (for example, a bad username or bad password error), response data
>>may not be returned.  Thus, the packet received will be smaller than
>>expected.  Thus, the response packet's checksum field may not be copied
>>into the pkt.chksum field properly.  And the above chksum check bombs.
>>
>>For example, a get_session_challenge_response is 42 bytes (rmcp +
>>session + rs_msg + data + chksum).  However, if you passed a bad
>>username during the get_session_challenge_request command, the response
>>will only be 23 bytes (the only response data is the completion code, no
>>session_id or challenge string is returned).  Therefore, calling
>>ipmi_chksum using your above example will fail, when in fact, it
>>shouldn't fail.  The checksums are probably fine, its the completion
>>code that is the problem.
>>
>>I think the correct way is to do this is as follows:
>>
>>len = ipmi_recvfrom(fd, &rmcp_hdr, &ipmi_pkt, sizeof(ipmi_pkt));
>>if (len <= 0)
>>   // blah blah
>>
>>if (ipmi_response_chksum(&ipmi_pkt.msg.rs, len) == 0)
>>   // blah blah yell and scream
>>
>>I think the ipmi_recvfrom should return the length of data stored
>>in the ipmi_pkt buffer.  Then, the ipmi_chksum_response function
>>assumes the last byte of the buffer passed in is the checksum field.
>>
>>Let me know what you think ...
>>
>>Al
>>
>>--
>>Albert Chu
>>address@hidden
>>Lawrence Livermore National Laboratories
>>
>>
>>_______________________________________________
>>llnl-devel mailing list
>>address@hidden
>>http://californiadigital.com/cgi-bin/mailman/listinfo/llnl-devel
>
> --
> Anand Babu
> CaliforniaDigital.com
> Office# +1-510-687-7045
> Cell# +1-510-396-0717
> Home# +1-510-894-0586
>
> Free as in Freedom <www.gnu.org>
>
> _______________________________________________
> llnl-devel mailing list
> address@hidden
> http://californiadigital.com/cgi-bin/mailman/listinfo/llnl-devel
>