IPMItool / Bugs (use GitHub instead) / #480 ipmitool coredumps in EVP_CIPHER_CTX

Zdenek Styblik - 2017-03-12

status: open --> open-accepted

assigned_to: Zdenek Styblik

Group: version-cvs --> version-1.8.19
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Zdenek Styblik - 2017-03-12

Hello,

thank you for bug report and I'm sorry for inconvenience. I should've paid more attention to changes as some things just really cannot work. I'm looking into fix, although I'm afraid I have no way to test whether the fix works or not.

Zdenek

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Zdenek Styblik - 2017-03-12
  
  Patch is attached. Please, can you test it? I've tried to connect to non-IPMI host and I didn't see any segfault, but it's not an IPMI capable host after all. In other words, it's possible I didn't get to SSL part.
  
  Thank you very much,
  Zdenek
  
  0001-ID-480-ipmitool-coredumps-in-EVP_CIPHER_CTX_init.patch
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - Keisuke MORI - 2017-03-23
    
    Thank you for providing a patch, and sorry for my late response.
    
    Yes, the patch works perfectly. I have tested it with connecting to a real IPMI LAN machine and ipmitool no longer coredumps now.
    
    Only my question by looking into the patch is that the ctx is not freed; Isn't it necessary?
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Zdenek Styblik - 2017-03-25

Hello,

thank you for testing it.

As for your question, while I was working on the fix, I got the impression EVP_CIPHER_CTX_cleanup() and EVP_CIPHER_CTX_free() are more or less same. However, EVP_CIPHER_CTX_cleanup() has been deprecated and replaced with EVP_CIPHER_CTX_free().
Is it necessary? I don't know. Sooner or later it might be. It shouldn't be a problem to write a patch which replaces cleanup with free.

Best regards,
Zdenek

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Keisuke MORI - 2017-03-27

In my understanding, EVP_CIPHER_CTX_new() and EVP_CIPHER_CTX_free() are in a pair as same as malloc() and free().

In the previous code, the memory of ctx was allocated on the stack by EVP_CIPHER_CTX ctx and so EVP_CIPHER_CTX_init() and EVP_CIPHER_CTX_cleanup() do not handle memory allocation for the ctx . In the newer code by contrast, EVP_CIPHER_CTX *ctx only allocates the pointer, and not the entity of ctx, so we'd need to manage the memory allocation for it by ourselfs, that's EVP_CIPHER_CTX_new() and EVP_CIPHER_CTX_free() are for. So an absent of EVP_CIPHER_CTX_free() might cause a memory leakage.

I've also looked into the source code of OpenSSL to make sure about it:

EVP_CIPHER_CTX_new() does: malloc and init
https://github.com/openssl/openssl/blob/OpenSSL_1_0_1-stable/crypto/evp/evp_enc.c#L86

EVP_CIPHER_CTX_free() does: cleanup and free
https://github.com/openssl/openssl/blob/OpenSSL_1_0_1-stable/crypto/evp/evp_enc.c#L534

Thanks,
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Zdenek Styblik - 2017-03-27
  
  Thank you for digging into this. I'm busy until Wednesday, but I will create patch then, if you're willing to test it :)
  
  Best regards,
  Zdenek
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Holger Liebig - 2017-03-29

Patch proposal to add the missing EVP_CIPHER_CTX_free() which also calls EVP_CIPHER_CTX_cleanup().

patch.diff

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Zdenek Styblik - 2017-03-30
  
  Hello,
  
  the patch looks good to me. I have the following comments:
  
  please, generate patch with % git format;
  
  please, EVP_CIPHER_CTX* ctx -> EVP_CIPHER_CTX *ctx
  
  feature request - log debug message when ctx == NULL to make debugging easier
  
  Thank you,
  Zdenek
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
  - Holger Liebig - 2017-03-30
    
    Hi Zdenek,
    
    patch was created with 'git diff' from the project root, not sure what you
    mean by % git format.
    
    Thanks & Best Regards,
    
    Holger
    
    From: Zdenek Styblik [mailto:stybla@users.sf.net]
    Sent: Thursday, March 30, 2017 2:00 PM
    To: [ipmitool:bugs]
    Subject: [ipmitool:bugs] Re: #480 ipmitool coredumps in EVP_CIPHER_CTX_init
    
    Hello,
    
    the patch looks good to me. I have the following comments:
    
    please, generate patch with % git format;
    
    please, EVP_CIPHER_CTX ctx -> EVP_CIPHER_CTX ctx
    
    feature request - log debug message when ctx == NULL to make
    debugging easier
    
    Thank you,
    Zdenek
    
    [bugs:#480] https://sourceforge.net/p/ipmitool/bugs/480/ ipmitool
    coredumps in EVP_CIPHER_CTX_init
    
    Status: open-accepted
    Group: version-1.8.19
    Created: Tue Mar 07, 2017 05:43 AM UTC by Keisuke MORI
    Last Updated: Wed Mar 29, 2017 07:16 AM UTC
    Owner: Zdenek Styblik
    
    ipmitool built from the latest repository 497f77
    https://sourceforge.net/p/ipmitool/source/ci/497f7767cd8e80ad67d08680ae1652 71441017fc/ dumps core in the encryption of lanplus as the backtrace
    below:
    
    Core was generated by `./src/ipmitool -vvv -I lanplus -H xx.xx.xx.xx -U
    XXXXXX -P XXXXXXX power status'.
    Program terminated with signal 11, Segmentation fault.
    
    0 0x00000031b84ebee3 in EVP_CIPHER_CTX_init () from
    
    /usr/lib64/libcrypto.so.10
    
    1 0x000000000046bdfa in lanplus_encrypt_aes_cbc_128 (...) at
    
    lanplus_crypt_impl.c:168
    
    Apparently the changes for ID:461 are related:
    
    ID:461 - Make compiler happier about changes related to OpenSSL 1.1
    https://sourceforge.net/p/ipmitool/source/ci/77fe5635037ebaf411cae46cf5045c a819b5c145/
    
    ID:461 - OpenSSL 1.1 compatibility -
    https://sourceforge.net/p/ipmitool/source/ci/b57487e360916ab3eaa50aa6d021c7 3b6337a4a0/ "error: storage size of 'ctx' isn't known"
    
    I'm not familiar with the OpenSSL API, but if my understanding is correct,
    EVP_CIPHER_CTX *ctx needs to be allocated and freed manually by using
    EVP_CIPHER_CTX_new() and EVP_CIPHER_CTX_free() .
    
    I'm using CentOS 7.2 and OpenSSL 1.0.1e bundled with it.
    
    Thanks,
    
    Sent from sourceforge.net because you indicated interest in
    https://sourceforge.net/p/ipmitool/bugs/480/
    
    To unsubscribe from further messages, please visit
    https://sourceforge.net/auth/subscriptions/
    
    alternate
    
    smime.p7s
    
    If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
    - Zdenek Styblik - 2017-03-30
      
      Hello Holger,
      
      please, give a read to "Write a patch" at https://sourceforge.net/p/ipmitool/wiki/Home/ . I hope it helps and is easy to understand. If not, please, let me know.
      
      Thanks,
      Zdenek
      
      If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Holger Liebig - 2017-03-30

Updated patch

patch_v2.diff

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
- Zdenek Styblik - 2017-04-04
  
  Hello Holger,
  
  the patch is in under your name now :)
  
  Best regards,
  Zdenek
  
  If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Zdenek Styblik - 2017-04-04

status: open-accepted --> closed-fixed
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

Zdenek Styblik - 2017-04-04

I'm closing this as fixed since it seems to me all issues have been addressed. Thank you guys.

Best regards,
Zdenek

If you would like to refer to this comment somewhere else in this project, copy and paste the following link:

ipmitool coredumps in EVP_CIPHER_CTX_init

Group

Searches

Help

#480 ipmitool coredumps in EVP_CIPHER_CTX_init

Discussion

0 0x00000031b84ebee3 in EVP_CIPHER_CTX_init () from

1 0x000000000046bdfa in lanplus_encrypt_aes_cbc_128 (...) at