Wireshark · Wireshark-commits: [Wireshark-commits] master 2944d8b: ssl: clarify meaning of StringInfo, cleanup PRFs, master_secret

[Wireshark code review <code-review-do-not-reply@xxxxxxxxxxxxx>]

URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=2944d8b97ce4e046acea39589623c91b0e9bcf4e
Submitter: Alexis La Goutte (alexis.lagoutte@xxxxxxxxx)
Changed: branch: master
Repository: wireshark

Commits:

2944d8b by Peter Wu (peter@xxxxxxxxxxxxx):

    ssl: clarify meaning of StringInfo, cleanup PRFs, master_secret
    
    It was not clear whether the data_len member of StringInfo refers to
    the allocated memory (as was done for session_ticket) or the length
    of the actual data. This is clarified in a comment. To keep the
    invariant "data_len refers to the length of meaningful data", some
    code has been moved just in case some intermediate code fails:
    
     - Setting session_ticket.data_len vs tvb_memcpy to session_ticket.data.
     - PRF functions would expect the data length as input to a paramter
       named "out". This is highly confusing, so another parameter has been
       added to signify the requested length, "out_len". This also helps
       holding up the invariant.
     - For prf() calls, out.data_len does not need to be initialized but
       passed as parameter.
    
    Other PRF-related changes:
    
     - Change the PRF functions to return a boolean instead of an int.
     - tls_hash: return void as it cannot fail and remove related error
       handling from callers. Fix a memleak of label_seed if tls_hash was
       successful.
     - tls_hash: add comments to clarify its functionality, whitespace.
     - ssl3_generate_export_iv could not fail, so make it void. Also added
       an out_len param to pass the target length.
     - In prf(), replaced if-conditions for SSL version by a switch.
     - In ssl_generate_keyring_material, the scope of some variable has been
       tightened.
     - ssl_session_init: explicitly set data_len to 0. This is strictly not
       necessary as the callers have already zeroed out the memory, but that
       has not been documented.
    
    Other changes related to master_secret (ssl_save_session[_ticket]):
    
     - Initialize master_secret.data_len to 0 in ssl_session_init as the
       master_secret is unusable at that point.
     - Remove the hack that tests whether master_secret.data is non-empty.
     - Replace hardcoded master_secret length (48) from wmem_alloc0().
     - Introduce macro for master secret length, use this in
       SslDecryptSession, for parsing from keyfile and converting pre-master
       secret to master secret (prf).
     - Use (master_secret + 1) to refer to the part after the struct rather
       than adding the size manually to a gchar-casted master_secret.
    
    Change-Id: Ie1ea448db54e828b904568224486147a3d962522
    Reviewed-on: https://code.wireshark.org/review/3030
    Reviewed-by: Peter Wu <peter@xxxxxxxxxxxxx>
    Reviewed-by: Alexis La Goutte <alexis.lagoutte@xxxxxxxxx>
    

Actions performed:

    from  fc983cf   ssl,dtls: move Finished dissection to ssl-utils
    adds  2944d8b   ssl: clarify meaning of StringInfo, cleanup PRFs, master_secret


Summary of changes:
 epan/dissectors/packet-ssl-utils.c |  322 ++++++++++++++++++------------------
 epan/dissectors/packet-ssl-utils.h |    9 +-
 2 files changed, 166 insertions(+), 165 deletions(-)