Commit Graph

293 Commits

Author SHA1 Message Date
Peter Wu 4680c8b429 Revert "Report an error if we fail to open the keylog file."
This reverts commit d1fcb7dd34.

Warning the user multiple times about an invalid ssl.keylog_file every
time a SSL stream is encountered is an annoyance (in tshark), but
crashing in GTK+/Qt during live captures is even worse.

Disable the warning for now. Maybe detect it once at startup? That would
not cover removed files though.

Bug: 11488
Change-Id: I56b2eba1df0cff2309584a745b55ada238999fc4
Reviewed-on: https://code.wireshark.org/review/9687
Reviewed-by: Michael Mann <mmann78@netscape.net>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-09-21 14:58:28 +00:00
Alex Badea 5e635ad714 ssl: determine DTLS by positively checking for UDP
TLS can be tunnelled over other protocols (e.g. TLS over EAP
over 802.1x), which are neither TCP nor UDP.  In this case,
we would assume DTLS, which is typically wrong.  Assume TLS
instead.

Change-Id: I45d70789f7fa793861297fc2e7a5f2be311bbbb1
Reviewed-on: https://code.wireshark.org/review/10416
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
2015-09-21 14:57:03 +00:00
Alexis La Goutte a0cfeab7f6 SSL: Fix Dead Store (Dead assignement/Dead increment) warning found by Clang
Change-Id: Ice4523786238f17250961a85988a195f2df8e888
Reviewed-on: https://code.wireshark.org/review/10507
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Olaf Bergmann <bergmann@tzi.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-09-14 09:10:04 +00:00
Olaf Bergmann a6360b6cc0 SSL: bugfix for raw public keys in certificate message
RFC 7250 has changed the format of the Certificate structure from
RFC 5246 to the following:

opaque ASN.1Cert<1..2^24-1>;

struct {
    select(certificate_type) {

        // certificate type defined in RFC 7250
        case RawPublicKey:
           opaque ASN.1_subjectPublicKeyInfo<1..2^24-1>;

        // X.509 certificate defined in RFC 5246
        case X.509:
           ASN.1Cert certificate_list<0..2^24-1>;
    };
} Certificate;

Thus, ssl_dissect_hnd_cert() must parse subjectPublicKeyInfo
immediately when the message's certificate type is
SSL_HND_CERT_TYPE_RAW_PUBLIC_KEY. Otherwise, the message will
contain a certificate_list.

This modification first determines the certificate type and then
handles both cases independently. For raw public keys, no subtree
is created to reflect the flat structure of the certificate
message.

Bug: 11480
Change-Id: I1c55eca361c4e40fcbff5bc32bfc8de3576bdfbf
Reviewed-on: https://code.wireshark.org/review/10272
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-09-07 21:56:42 +00:00
Timo Warns 7d599251f6 SSL: refine KEX handling and fix _anon KEX dissecting
Dissecting client and server KEX messages requires to precisely distinguish KEX
algos. For example, Server KEX for DH_anon do not contain a signature, while
DHE_DSS and DHE_RSA do. The patch introduces KEX distinction with full
precision and fixes dissecting _anon KEX messages.

Change-Id: I0bcd5e2bf899ba9cac79476d5b7a1ffb3accf0db
Reviewed-on: https://code.wireshark.org/review/9836
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2015-08-21 08:57:16 +00:00
Peter Wu 5038efd13f ssl-utils: do not check for empty keylog filenames
If ssl.keylog_file is not configured, an empty string is set. In that
case, do not attempt to open the keylog file.

Change-Id: I2ba4b9dbc7cfb5009d2623c49a129e98734df80f
Reviewed-on: https://code.wireshark.org/review/9688
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2015-07-17 18:28:55 +00:00
Guy Harris d1fcb7dd34 Report an error if we fail to open the keylog file.
That way, we don't just silently fail.

Change-Id: I924f4387f6efdc342f6b02ed29796802567c1884
Reviewed-on: https://code.wireshark.org/review/9683
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2015-07-17 09:02:24 +00:00
Peter Wu b99f10bea7 ssl-utils: fix RSA keys with GnuTLS 2.12.23
Apparently GnuTLS 2.12.23 as used on Ubuntu 14.04 produces different
outputs for the u parameter as observed in gdb. GnuTLS 3.4.2 on Arch
Linux works fine. Workaround this issue by unconditionally calculating
the inverse.

Change-Id: I8406352f8c570b355ea774cafc903662d06888ac
Fixes: v1.99.8rc0-417-g85f8a99
Bug: 11371
Reviewed-on: https://code.wireshark.org/review/9666
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-07-16 23:13:33 +00:00
Peter Wu 85f8a99f35 ssl-utils: fix failing decryption for some RSA keys
Reported at
https://ask.wireshark.org/questions/43788/struggling-to-decrypt-ssl

"u" requirement is documented at
https://www.gnupg.org/documentation/manuals/gcrypt/RSA-key-parameters.html#RSA-key-parameters

Add regression test (key is generated manually with p and q swapped and
qInv recalculated).

Change-Id: I5505ddcdb54bb47d7a58867b8c3e53fcc0f66dde
Reviewed-on: https://code.wireshark.org/review/9573
Tested-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-07-15 21:31:44 +00:00
Peter Wu 79be8312da ssl-utils: remove unused, broken libgcrypt code
Remove unused SSL_FAST code. That approach cannot work in modern
libgcrypt anyway since the symbols were renamed and private to
libgcrypt. The RSA decryption routine is not even a hot path, it is only
called for decrypting the encrypted pre-master secret.

While at it, expand the SSL_PRIVATE_KEY macro and remove its definition.

Change-Id: Ied556d18501ea6cbac5fb27218364b3479ad62ce
Reviewed-on: https://code.wireshark.org/review/9572
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-07-15 21:30:40 +00:00
Peter Wu 3ad976896a ssl,dtls: split init/cleanup routines
Minor functional change: instead of an empty hash table, now the
ssl_session_hash and ssl_crandom_hash structures point will be set to
NULL when files are closed.

API change: drop the ssl_keylog_file parameter from ssl_common_init,
add a new ssl_common_cleanup parameter instead.

Change-Id: I65efe71f8347fe9685359f8ed70cfb9673712421
Reviewed-on: https://code.wireshark.org/review/9226
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-07-03 23:38:26 +00:00
Guy Harris 63a78d45bc Use ws_fstat64() to determine the size of an open file.
fseek() to the end, followed by ftell(), is a bit of an odd way to get
the file size.  Use ws_fstat64() instead.

Check that the file is a regular file, while we're at it.  This means we
don't have to check before opening.

Bug: 11268
Change-Id: I31ee20dd5568d10541375cf97b286abfc1384d1c
Reviewed-on: https://code.wireshark.org/review/9230
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2015-06-30 02:34:54 +00:00
Peter Wu 1e511d830e ssl: test for file type before reading key file
ftell() is undefined for directories. In practice, it will report
0x7fffffffffffffff on an ext4 filesystem. Ensure that the given key file
is not a directory.

By the way, this is the only user of ftell that is affected.

Bug: 11268
Change-Id: Iaecd42c9b60da9e7945703a794601773749f2d97
Reviewed-on: https://code.wireshark.org/review/9213
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-06-29 22:57:55 +00:00
Adam Pridgen c392db5b08 TLS Decryption is now possible with a user provided PMS and Client Random values
Bug: 11263
Change-Id: I1476948ed823fc34de2cecdeb1bddafccbb8ec39
Reviewed-on: https://code.wireshark.org/review/8803
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2015-06-16 08:31:51 +00:00
Gerald Combs e08bc0dd62 Don't warn about overwriting filename preferences.
When specifying a filename preference (e.g. the SSL pre-master secret
log filename) don't warn about overwriting the file. Most of the time
we're reading the file and when we're not (e.g. for the SSL debug log)
overwriting it is kind of the point.

Preference descriptions are plain text. We display them in tooltips as
rich text. Convert them accordingly.

Fixup some of the SSL preference descriptions.

Bug: 11010
Change-Id: I4f1b1f3dd270c01648d9dd52ae20381c3c0d2e37
Reviewed-on: https://code.wireshark.org/review/8665
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>
2015-05-28 02:21:33 +00:00
Michael Mann c6b0a61ab3 Remove proto_tree_add_text from packet-ssl-utils.c
Change-Id: I64998e93e8d72faa76e0e7809abfd9ccae10ab36
Reviewed-on: https://code.wireshark.org/review/8653
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2015-05-26 15:14:23 +00:00
Guy Harris 8045cd6481 Do not assume the data field of an address structure is an aligned pointer.
There is *no* guarantee that it's aligned on a 4-byte boundary, and
there is *no* guarantee that you can safely dereference an unaligned
pointer.  See bug 11172 for a crash on Solaris/SPARC caused by those
assumptions both being false.

Change-Id: I30d97aebd42283545f5b8f6d50fa09c5b476ec47
Reviewed-on: https://code.wireshark.org/review/8412
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2015-05-11 22:11:21 +00:00
Alexis La Goutte c3c8316d2e SSL/TLS: Add support of TLS Fallback Signaling Cipher Suite Value (SCSV) for Preventing Protocol Downgrade Attacks (RFC 7507)
Update comment about TLS_FALLBACK_SCSV
Add new alert Inappropriate Fallback (86)

Change-Id: I17385b0a9ad71f3623ff4fdb4a9c588e46ba8d58
Reviewed-on: https://code.wireshark.org/review/8203
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2015-04-27 04:51:38 +00:00
Peter Wu 410b40d371 Export CLIENT_RANDOM with Export SSL Session Keys
This allows for exporting the SSL session keys for captures which were
decrypted using a RSA certificate, but where the server does not support
session resumption.

To avoid frequent reallocations, the expected length is used as initial
string size.

Tested against a nginx server with ssl_session_cache off.

Note that all keys loaded via ssl.keylog_file are exported, not just the
displayed ones!

Change-Id: Ie3a93d3692885502f46442953fa53303d16672d7
Reviewed-on: https://code.wireshark.org/review/7175
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2015-03-12 16:27:10 +00:00
Peter Wu 70d34eee2b ssl,dtls: fix CertificateVerify dissection for TLSv1.2
TLSv1.2 gained an additional SignatureAndHashAlgorithm field for fields
marked with the digitally-signed attribute. This was already implemented
before for ServerKeyExchange, let's reuse that.

Note that the SignatureAndHashAlgorithm tree and fields (hash algo,
signature algo) are repurposed in a different context, but since the
structure is the same it is kept like this.

By the way, add support for DTLSv1.2 too. RFC 6347 section 4.2.6
suggests that the implementation is the same (as far as the dissector is
concerned).

Also update the comments and remove the additional "Signature with
client's private key" subtree since the CertificateVerify message has no
other items.

Bug: 11045
Change-Id: I025901b85e607f04d60357ff14187cc13db2ae5d
Reviewed-on: https://code.wireshark.org/review/7650
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Tested-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-03-12 12:48:00 +00:00
Peter Wu d488d6392a ssl-utils: refactor keyfile matching
Merge the three separate regexes into a single pattern and use named
groups. This avoid magic numbers (group positions) and removes a
possible error source when the ht array gets out of sync with the
patterns array (by human error).

This is supposed to be more readable and allows for easier extension of
the regex.

Change-Id: I816525f358cdb89ff9f8ebc1211139b1f8c23840
Reviewed-on: https://code.wireshark.org/review/7245
Reviewed-by: Evan Huus <eapache@gmail.com>
Petri-Dish: Evan Huus <eapache@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2015-02-23 11:46:57 +00:00
Jeff Morriss a355daf328 Give users a more helpful error message if they enter an SSL protocol we don't
know.

First, if we know the protocol (by filter name) tell them that the dissector
just isn't set up to run over SSL but it could be--if they contact the Wireshark
developers.

Second, don't tell them that the dissectors which have called
ssl_dissector_add() are the only ones that are valid; those are just commonly
used ones.

Change-Id: I1b72bccd4c96c21c73a19fa2d87fe2c0b875a0fa
Reviewed-on: https://code.wireshark.org/review/7185
Reviewed-by: Richard Sharpe <realrichardsharpe@gmail.com>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-02-17 14:08:40 +00:00
Peter Wu 93ed72642b ssl,dtls,ssl-utils: Prepare for STARTTLS handling
All STARTTLS-like dissectors (protocols which can switch to SSL/TLS
after a protocol command) currently fail to get called after decryption.
The reason for this is that the port is not registered for SSL
dissection via ssl_dissector_add. Besides this, the MySQL dissector
breaks in the event of multiple segments because it does not properly
set desegmentation.

The call path TCP | App | SSL | App is a bad, error-prone pattern which
requires duplication of required functionality in dissectors. This patch
enables to bypass the App (TCP | SSL | App) by registering a SSL as
conversation dissector after a STARTTLS switch.

Logical overview of changes:

 - Move srv_addr, srv_ptype and srv_port to SslSession and adjust the
   users. This allows passing SslSession around which will never be null
   unlike SslDecryptSession. This is needed for looking up the packet
   direction (server or client) before calling a subdissector.
 - Add app_handle to store the dissector and last_nontls_frame the
   frame that initiated STARTTLS.
 - The same app_handle is now used to store the dissector handle from
   a ssl association.
 - Moved conversation data (SslDecryptSession) to ssl-utils to avoid
   code duplication. Merge ssl_session_init into it. The new
   ssl_session_get() is needed for STARTTLS frame/handle storage.
 - Introduce new "ssl_starttls_ack" function to signal the last non-TLS
   packet.
 - Ensure that match_uint is set before calling the conversation
   dissector. This ensures that dissectors using match_uint to check
   the direction of a packet (client vs. server) see the TCP port
   instead of the IP proto. At least the MySQL and SMTP dissectors
   require such special treatment.
 - Move epan/conversation.h outside HAVE_LIBGNUTLS, remove from dtls
   (as it is already included by ssl-utils).
 - Various comment/debug string updates. Remove outdated comment before
   SSL association lookup.

Besides setting match_uint and caching the app_handle, existing
dissectors should not be affected by this patch. Follow-up patches
will update existing dissectors to use the new ssl_starttls_ack
interface.

Bug: 9515
Change-Id: I795d16b6a901e672a5d89e922adc7e5bbcda0333
Reviewed-on: https://code.wireshark.org/review/6872
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-02-09 14:00:38 +00:00
Peter Wu 3222cd1df6 ssl-utils: use ALPN hint for improved spdy/http2 recognition
This patch improves detection of a SPDY/3.1 in SSL capture. While at it,
add other protocols from the RFC/drafts.

spdy was tested against a private capture from spdy/3.1 communication
between Chromium 40 and ssl.gstatic.com.
http2 was tested against http2-16-ssl.pcapng from
http://wiki.wireshark.org/SampleCaptures#SSL_with_decryption_keys

Change-Id: I111efae34d614b7d8e37eaaa686b391d332753dd
Reviewed-on: https://code.wireshark.org/review/7000
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-02-09 12:58:18 +00:00
Michael Mann ada1eec702 SE_COPY_ADDRESS -> WMEM_COPY_ADDRESS
Copy addresses with wmem-scope instead of (forced) seasonal scope.  All existing instances were converted to wmem_file_scope, but the flexibility is there for other scopes.

Change-Id: I8e58837b9ef574ec7dd87e278470d7063ae8c1c2
Reviewed-on: https://code.wireshark.org/review/6564
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-01-16 15:25:18 +00:00
Guy Harris bc23f79729 UAT error string pointers should not be const pointers.
UAT error strings are usually allocated by g_strdup() or
g_strdup_printf(), and must ultimately be freed by the caller.

Make the pointer-to-error-string-pointer arguments to various functions
be "char **", not "const char **".

Fix cases that finds where a raw string was being used, as that won't
work if you try to free it; g_strdup() it instead.

Add a missing free of an error string.

Remove some no-longer-necessary casts.

Remove some unnecessary g_strdup()s (the string being handed to it was
already g_malloc()ated).

Change some variable declarations to match.

Put in XXX comments for some cases where the error string is just freed,
without being shown to the user.

Change-Id: I40297746a2ef729c56763baeddbb0842386fa0d0
Reviewed-on: https://code.wireshark.org/review/6525
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2015-01-13 20:20:03 +00:00
Michael Mann 7967ef9510 Remove emem APIs from UAT functionality.
Change-Id: I009c09f25d170e5c9aaaef713eaacb3252817856
Reviewed-on: https://code.wireshark.org/review/6460
Petri-Dish: Michael Mann <mmann78@netscape.net>
Reviewed-by: Evan Huus <eapache@gmail.com>
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-01-12 14:25:12 +00:00
Michael Mann f5c4d06dba Replace some "low hanging fruit" uses of emem.
Most of the remaining ep_ uses are grouped with specific functionality.

Change-Id: I8fa64a17acc6bcdcf6891b2d28715ac0c58f1a4a
Reviewed-on: https://code.wireshark.org/review/6484
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-01-11 00:27:57 +00:00
Michael Mann 4a5ca5c76e bytes_to_ep_str -> bytes_to_str
Change-Id: Ifcda8328dedec0ef4104c3a124d6246f99493750
Reviewed-on: https://code.wireshark.org/review/6389
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2015-01-08 05:43:51 +00:00
Michael Mann 98d3b1494b Replace bytes_to_ep_str_punct with wmem equivalent.
Change-Id: I8aa7d7374db94685fd875cbf358c3bfbc83f3255
Reviewed-on: https://code.wireshark.org/review/6370
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-01-07 18:12:35 +00:00
Gerald Combs d3581aecda Make sure we don't underrun a buffer when decrypting SSL.
Discovered by Noam Rathaus.

Change-Id: Ia0275601b2a825ba616656064d9a6eca109e34fa
Reviewed-on: https://code.wireshark.org/review/6256
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Evan Huus <eapache@gmail.com>
2015-01-04 18:43:41 +00:00
Martin Kaiser 97f5f39c96 free the temporary buffers allocated by gnutls_x509_privkey_export_rsa_raw()
Bug: 10740
Change-Id: Idd4afab1bca6204d17c4bd4345661ec821f209e0
Reviewed-on: https://code.wireshark.org/review/6145
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Tested-by: Martin Kaiser <wireshark@kaiser.cx>
2014-12-30 18:34:40 +00:00
Bill Meier b68fb9b659 Fix some dissectors doing termio (fprintf(stderr,..), g_warning()).
- Use report_...failure() (in most cases).
- Also: Do some misc fixes in certain disectors
  - re-arrange order of #includes
  - Fixup preferences help text

Change-Id: I385f6f97257f365f53ce611df02f57f9257dc5f9
Reviewed-on: https://code.wireshark.org/review/6039
Petri-Dish: Bill Meier <wmeier@newsguy.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2014-12-26 20:55:25 +00:00
Bill Meier b5d7b7ab6e Cleanup use of #includes in non-generated epan/dissector/*.c
Specifically:
- Set packet.h to be the first wireshark #include after
   config.h and "system" #includes.
   packet.h added as an #include in some cases when missing.
- Remove some #includes included (directly/indirectly) in
   packet.h. E.g., glib.h.
   (Done only for those files including packet.h).
- As needed, move "system" #includes to be after config.h and
   before wireshark #includes.
- Rework various #include file specifications for consistency.
- Misc.

Change-Id: Ifaa1a14b50b69fbad38ea4838a49dfe595c54c95
Reviewed-on: https://code.wireshark.org/review/5923
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Bill Meier <wmeier@newsguy.com>
2014-12-21 05:46:22 +00:00
Dave Tapuska 6dbb97da10 SSL: Implement Extended Master Secret
Store all handshake mesages in a buffer so that we can hash them
correctly when generating the master secret.

This change does not work correctly for DTLS retransmitted packets; that
are in the handshake as they will be hashed twice; which is bad. Looking
for ideas to implement this.

Bug: 10686
Change-Id: Ied01d4cc37b4270f325070a8d1630d3123577a0d
Reviewed-on: https://code.wireshark.org/review/5168
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
2014-11-24 09:22:12 +00:00
Martin Kaiser c33deaa92f SSL 3.0 and TLS ciphersuite values are two bytes long
Change-Id: I3ea2846fd4273efe654358ad4a317cb9b3670181
Reviewed-on: https://code.wireshark.org/review/4942
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Tested-by: Martin Kaiser <wireshark@kaiser.cx>
2014-10-26 22:32:06 +00:00
Martin Kaiser 3ac46ed6d7 add TLS_FALLBACK_SCSV
Change-Id: I02ef08b6726f935d79cd3ca4f10ead5f1d18ee20
Reviewed-on: https://code.wireshark.org/review/4941
Reviewed-by: Martin Kaiser <wireshark@kaiser.cx>
Tested-by: Martin Kaiser <wireshark@kaiser.cx>
2014-10-26 22:31:40 +00:00
Guy Harris 0b9eb9f4b7 Get rid of unnecessary includes of ctype.h.
Change-Id: I2cf49f808558147ce77e7d086558966cfb2defca
Reviewed-on: https://code.wireshark.org/review/4850
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2014-10-20 06:02:51 +00:00
Michael Mann 5e4e17ca5e Remove unnecessary tvb_ensure_bytes_exist calls.
All of the calls removed are followed by proto_tree_add_xxx calls of the same offset/length of the tvb_ensure_bytes_exist call.  The proto_tree_add_xxx calls should throw the exception, so we don't need the "double check".
There are probably more calls that can be removed, these were just obvious as first glance, spurred mostly by noticing the (ab)use in packet-wsp.c

Change-Id: I37cee347c8cf8ab0559e21562c802d3b37f4871e
Reviewed-on: https://code.wireshark.org/review/4833
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
2014-10-19 15:26:43 +00:00
Evan Huus 2154e2346e ssl: allocate regexes with glib
they're stored in a static variable so we only ever need compile them once and
they can just hang around forever

Change-Id: Icf43745ad80f4984443a67af21c979625363fc6f
Ping-Bug: 10474
Reviewed-on: https://code.wireshark.org/review/4139
Reviewed-by: Evan Huus <eapache@gmail.com>
2014-09-20 18:09:28 +00:00
Michael Mann 80407a46df Eliminate proto_tree_add_text from some of the dissectors.
Other minor cleanups while in the area.

Change-Id: I99096ade9c69a4c148962d45bb6b0bd775040ba1
Reviewed-on: https://code.wireshark.org/review/4020
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2014-09-09 01:25:36 +00:00
Jamil Nimeh f2a7a6d503 TLS: fix dissection of status_request_v2 hello extension
Bug: 10416
Change-Id: I58a3faef227f7eafd61942cafa6e38a17557ee61
Reviewed-on: https://code.wireshark.org/review/3865
Reviewed-by: Evan Huus <eapache@gmail.com>
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com>
2014-09-01 15:00:22 +00:00
Peter Wu 06ba328fec Fix uninitialized session ticket
If the tvb contained too few data,
ssl_dissect_hnd_hello_ext_session_ticket would then allocate
session_ticket, but not initialize the contents. Fix this by adding a
check for the TVB length.

The same is done for ssl_dissect_hnd_new_ses_ticket. That might, or
might not, be necessary as proto_tree_add_item() is called with the
range. When tree is NULL, ssl is usually NULL too. For clarity (and to
avoid surprises in the future), add it anyway.

Bug: 10330
Change-Id: I469e97542542aaef4cbd660086bedf92ba1c0b6e
Reviewed-on: https://code.wireshark.org/review/3309
Reviewed-by: Evan Huus <eapache@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2014-08-03 07:41:07 +00:00
Peter Wu a69a63f5d1 ssl: fix SSL keylog file live-capture use case
Patch "ssl,dtls: simplify keyfile handling" did not account for the use
case where packets are captured and decrypted on the fly using
SSLKEYLOGFILE.

This patch restores that functionality by reading additional lines from
the keylog file when needed (to preserve the benefit of not having to
read the full file) and by watching the open file for deletions.

"Deletion" is detected by comparing st_dev and st_ino. Since these may
be useless on Windows, the size is also checked.

Change-Id: Ieadaef1426a9270587293db28f4dda33b3d17334
Reviewed-on: https://code.wireshark.org/review/3190
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Evan Huus <eapache@gmail.com>
Petri-Dish: Evan Huus <eapache@gmail.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2014-07-31 11:09:07 +00:00
Peter Wu 7939d32ce2 ssl,dtls: simplify keyfile handling
Previously, the keylog file would be fully parsed when an encrypted
pre-master secret is encountered or in the ChangeCipherSpec stage. There
was also a lot of duplication in the key logfile parsing.

This patch simplifies the key logfile parsing by using regular
expressions. Rather than scanning the key logfile for a specific key,
do this scan once at ssl init and save the results to a hashtable. The
map for session ID/tickets to master keys already existed, another one
for client random to master key and encrypted pre-master to pre-master
was added. This could later also be wired to the "Export SSL Keys"
menu item for improved reliability (when no session ID or tickets are
available, the client random could be used).

The ssl_{save,restore}_session{,_ticket} functions have been converted
to a single function that looks up a key (sid / client random / encr.
pre-master) to a (pre-)master secret.

Other minor changes: return booleans for some functions that can only
fail/pass. Remove some functions from the ssl-utils header that have
become private a few commits ago. Remove some outstanding issues
from the comments in packet-ssl as they are already done, add myself
to the ssl-utils header.

These changes pass the test suite and the sample Session Ticket-enabled
capture from https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5963

On-the-fly decryption are broken with this patch since keylog files are
read once at the start of a capture. This will be solved in a future
patch.

Change-Id: Idb343abe161950b5f3ff61bee093d0f4ef9655bd
Reviewed-on: https://code.wireshark.org/review/3057
Reviewed-by: Evan Huus <eapache@gmail.com>
Petri-Dish: Evan Huus <eapache@gmail.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2014-07-31 11:08:18 +00:00
Peter Wu 9ed85d1de9 Fix use of uninitialized field
ssl_print_string uses out->data_len to determine the length of the
printed data, but this was not set. Use ssl_data_set for that and add an
additional DISSECTOR_ASSERT just in case we change something here.

Reported by Alexis La Goutte, found by Clang static analyzer.

Change-Id: I630a9193ff1ece86a0a46924dd86591fedf5c595
Reviewed-on: https://code.wireshark.org/review/3261
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2014-07-30 16:25:33 +00:00
Peter Wu 2944d8b97c 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@lekensteyn.nl>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
2014-07-24 05:33:50 +00:00
Peter Wu fc983cfca2 ssl,dtls: move Finished dissection to ssl-utils
Change-Id: Ib4bd5712cb85cd2671f67fe035747b88d5b4f186
Reviewed-on: https://code.wireshark.org/review/3034
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Hauke Mehrtens <hauke@hauke-m.de>
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
2014-07-24 05:29:52 +00:00
Peter Wu 5e3b04449a ssl,dtls: move Session Ticket to ssl-utils
Changes:

 - dtls: also support saving session tickets.
 - Drop the length check and let proto_tree_add_item throw exceptions
   on length errors.
 - Use proto_tree_add_item instead of proto_tree_add_uint.
 - Drop "TLS" from header field descriptions, the RFC does not name it
   as such and DTLS can also use it (a draft is in progress that extends
   DTLS with Session Tickets,
   draft-hummen-dtls-extended-session-resumption-01).

Change-Id: I11195217368b7200821d11289b1c5870a1ffe637
Reviewed-on: https://code.wireshark.org/review/3029
Reviewed-by: Evan Huus <eapache@gmail.com>
2014-07-23 20:50:38 +00:00
Peter Wu 057ded827d ssl-utils: stop exporting some symbols
Client/Server hello and Hello extensions are now dissected inside
ssl-utils, no need to export them for the SSL or DTLS dissectors.

Change-Id: I8f2405199f21616743fe74959f07cfa839565527
Reviewed-on: https://code.wireshark.org/review/3022
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Evan Huus <eapache@gmail.com>
2014-07-23 17:09:25 +00:00