[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] [Code Review] added AES_ENCRYPT and AES_DECRYPT
From:       "Mark Michelson" <mmichelson () digium ! com>
Date:       2009-01-27 22:20:42
Message-ID: 20090127222042.15881.81435 () hotblack ! digium ! internal
[Download RAW message or body]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/128/#review342
-----------------------------------------------------------

Ship it!


I'm marking this as "ship it" because I am confident that with the below two fixes, \
this code will be ready to merge.

Sorry I didn't find these earlier :(


/trunk/funcs/func_aes.c
<http://reviewboard.digium.com/r/128/#comment803>

    After thinking about this more, I realized that this memcpy is a bit superfluous \
here and could be replaced just as easily with ast_copy_string since args.data is \
null-terminated.  
    ast_copy_string(tmp, args.data, len);
    
    Sorry for directing you down the memcpy route the first time. I wasn't thinking \
correctly and the fact that args.data is a normal string escaped me.



/trunk/funcs/func_aes.c
<http://reviewboard.digium.com/r/128/#comment804>

    I started thinking about this and I'm thinking that the code in this else block \
is incorrect.  
    Based on what Tilghman was saying earlier, AES-encrypted text could contain null \
bytes. If this is the case, then when we decode from base64, tmp may actually appear \
to "end" before we reach the end of the encrypted string due to an embedded null \
byte. This means that calling strlen with tmp as the argument could result in \
data_len being smaller than it should be.  
    Luckily, this is a simple fix. Just directly set data_len to what \
ast_base64decode returns.


- Mark


On 2009-01-27 15:56:30, dvossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/128/
> -----------------------------------------------------------
> 
> (Updated 2009-01-27 15:56:30)
> 
> 
> Review request for Asterisk Developers, Russell Bryant and Mark Michelson.
> 
> 
> Summary
> -------
> 
> Allows data to be encrypted and decrypted using AES in the dialplan.  
> 
> 
> This addresses bug 0014301.
> http://bugs.digium.com/view.php?id=0014301
> 
> 
> Diffs
> -----
> 
> /trunk/funcs/func_aes.c PRE-CREATION 
> 
> Diff: http://reviewboard.digium.com/r/128/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> dvossel
> 
> 


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.com--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev


[prev in list] [next in list] [prev in thread] [next in thread]