Commit graph

477 commits

Author SHA1 Message Date
Jouni Malinen
63fc84acd7 WPS: Clean up http_client_tx_ready()
Calculate the send() buffer length only once to make this a bit more
readable.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-08-29 13:42:32 +03:00
Jouni Malinen
24a5e528ce WPS: Remove duplicated isgraph() loop in HTTP header parsing
The hbp pointer is moved to the next space already earlier in this code
path, so the while loop here did not really do anything.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-08-28 23:59:28 +03:00
Jouni Malinen
f79a19746a WPS: Merge common error paths in HTTP server
There is no need to maintain three separate "goto fail" cases.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-08-28 20:33:22 +03:00
Jouni Malinen
2ce741fe0f WPS: Fix HTTP body length check
Commit 7da4f4b499 ('WPS: Check maximum
HTTP body length earlier in the process') added too strict check for
body length allocation. The comparison of new_alloc_nbytes against
h->max_bytes did not take into account that HTTPREAD_BODYBUF_DELTA was
added to previous allocation even if that ended up going beyond
h->max_bytes. This ended up rejecting some valid HTTP operations, e.g.,
when checking AP response to WPS ER setting selected registrar.

Fix this by taking HTTPREAD_BODYBUF_DELTA into account.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-08-25 00:17:00 +03:00
Jouni Malinen
56906d06c7 WPS ER: Fix SSDP CACHE-CONTROL line parser
Incorrect number of bytes was skipped from the beginning of the line
which resulted in the loop skipping spaces doing nothing. However, the
following operation was simply looking for the max-age parameter with
os_strstr(), so this did not have any effect on functionality. Fix the
number of bytes to skip and remove the unneeded loop to skip spaces.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-08-15 22:24:15 +03:00
Jouni Malinen
7e97d813ee WPS: Reject AP settings with invalid network key (PSK/passphrase)
This is similar to the earlier commit
b363121a20 ('WPS: Reject invalid
credential more cleanly'), but for the AP cases where AP settings are
being replaced. Previously, the new settings were taken into use even if
the invalid PSK/passphrase had to be removed. Now, the settings are
rejected with such an invalid configuration.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-08-06 20:47:25 +03:00
Jouni Malinen
c7068f106f WPS: Avoid bogus static analyzer warning in ndef_parse_record()
Use a local variable and check the record payload length validity before
writing it into record->payload_length in hopes of getting rid of a
bogus static analyzer warning. The negative return value was sufficient
to avoid record->payload_length being used, but that seems to be too
complex for some analyzers. (CID 122668)

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-07-17 20:43:17 +03:00
Jouni Malinen
2456264fad NFC: Add a hardcoded limit on maximum NDEF payload length
While this is already enforced in practice due to the limits on the
maximum control interface command length and total_length bounds
checking here, this explicit check on payload_length value may help
static analyzers understand the code better. (CID 122668)

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-07-08 17:03:34 +03:00
Jouni Malinen
df9079e727 NFC: Fix payload length validation in NDEF record parser
It was possible for the 32-bit record->total_length value to end up
wrapping around due to integer overflow if the longer form of payload
length field is used and record->payload_length gets a value close to
2^32. This could result in ndef_parse_record() accepting a too large
payload length value and the record type filter reading up to about 20
bytes beyond the end of the buffer and potentially killing the process.
This could also result in an attempt to allocate close to 2^32 bytes of
heap memory and if that were to succeed, a buffer read overflow of the
same length which would most likely result in the process termination.
In case of record->total_length ending up getting the value 0, there
would be no buffer read overflow, but record parsing would result in an
infinite loop in ndef_parse_records().

Any of these error cases could potentially be used for denial of service
attacks over NFC by using a malformed NDEF record on an NFC Tag or
sending them during NFC connection handover if the application providing
the NDEF message to hostapd/wpa_supplicant did no validation of the
received records. While such validation is likely done in the NFC stack
that needs to parse the NFC messages before further processing,
hostapd/wpa_supplicant better be prepared for any data being included
here.

Fix this by validating record->payload_length value in a way that
detects integer overflow. (CID 122668)

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-07-08 16:52:25 +03:00
Jouni Malinen
fc880b11ed NFC: Avoid misaligned read of an NDEF field
The 32-bit version of payload length field may not be 32-bit aligned in
the message buffer, so use WPA_GET_BE32() to read it instead of ntohl().

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-07-07 16:25:06 +03:00
Jouni Malinen
8640cf7f8f WPS: Add more debug prints to httpread
These can be helpful when debugging HTTP error cases.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-05-03 18:26:50 +03:00
Jouni Malinen
1bd0d578a9 WPS: Replace the httpread_debug design with standard debug prints
The debug information from httpread can be helpful in figuring out error
cases in general and as such, should be enabled by default. Get rid of
the hardcoded httpread_debug value that would require source code
changes to enable.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-05-03 18:26:50 +03:00
Jouni Malinen
7da4f4b499 WPS: Check maximum HTTP body length earlier in the process
There is no need to continue processing a HTTP body when it becomes
clear that the end result would be over the maximum length.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-05-03 18:26:50 +03:00
Jouni Malinen
af185d0b57 WPS: Extra validation step for HTTP reader
Verify that ncopy parameter to memcpy is not negative. While this is not
supposed to be needed, it is a good additional protection against
unknown implementation issues.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-05-03 18:26:50 +03:00
Jouni Malinen
5acd23f458 WPS: Fix HTTP chunked transfer encoding parser
strtoul() return value may end up overflowing the int h->chunk_size and
resulting in a negative value to be stored as the chunk_size. This could
result in the following memcpy operation using a very large length
argument which would result in a buffer overflow and segmentation fault.

This could have been used to cause a denial service by any device that
has been authorized for network access (either wireless or wired). This
would affect both the WPS UPnP functionality in a WPS AP (hostapd with
upnp_iface parameter set in the configuration) and WPS ER
(wpa_supplicant with WPS_ER_START control interface command used).

Validate the parsed chunk length value to avoid this. In addition to
rejecting negative values, we can also reject chunk size that would be
larger than the maximum configured body length.

Thanks to Kostya Kortchinsky of Google security team for discovering and
reporting this issue.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-05-03 18:26:50 +03:00
Hamad Kadmany
01a025937c WPS: Add support for 60 GHz band
Handling of WPS RF band for 60 GHz was missing. Add it in all relevant
places and also map "AES" as the cipher to GCMP instead of CCMP when
operating on the 60 GHz band.

Signed-off-by: Hamad Kadmany <qca_hkadmany@qca.qualcomm.com>
2015-04-28 00:22:16 +03:00
Mikael Kanstrup
8b423edbd3 Declare all read only data structures as const
By analysing objdump output some read only structures were found in
.data section. To help compiler further optimize code declare these
as const.

Signed-off-by: Mikael Kanstrup <mikael.kanstrup@sonymobile.com>
2015-04-25 17:33:06 +03:00
Jouni Malinen
ca68a8b561 WPS: Explicitly reject Public Key attribute with unexpected length
There is no need to try to derive DH shared key with a peer that tries
to use too short or too long DH Public Key. Previously, such cases ended
up implicitly getting rejected by the DH operations failing to produce
matching results. That is unnecessarily, so simply reject the message
completely if it does not have a Public Key with valid length. Accept
couple of octets shorter value to be used to avoid interoperability
issues if there are implementations that do not use zero-padding
properly.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-04-22 22:05:12 +03:00
Jouni Malinen
6b94f71dcd WPS: Truncate variable length string attributes to maximum length
This enforces variable length strings Manufacturer, Model Name, Model
Number, and Serial Number to be within the maximum length defined in the
WSC specification. While none of the existing users for these within
hostapd/wpa_supplicant had problems with longer strings, it is good to
ensure the strings are not longer to avoid potential issues at higher
layer components.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-04-22 22:05:12 +03:00
Jouni Malinen
a65d7495b5 tests: Add p2p-fuzzer
This program can be used to run fuzzing tests for areas related to P2P
message parsing and processing. p2p-fuzzer allows data files to be used
to inject Probe Response and Action frames for processing by the P2P
module.

Signed-off-by: Jouni Malinen <j@w1.fi>
2015-04-22 11:44:19 +03:00
Jouni Malinen
0f5acfba06 Use common is_ctrl_char() helper function
This modifies couple of code segments that replaced control characters
in strings with '_' to use a common helper function.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-04-22 11:44:19 +03:00
Jouni Malinen
5a041acc0a WPS: Ignore too long SSID attribute
While it looks like all the users of this parsed attribute were able to
handle longer SSID values, there is no valid use case for these and to
avoid any potential future issues, enforce maximum length (32 bytes) on
the SSID during parsing.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-04-22 11:44:19 +03:00
Jouni Malinen
d9d1b9527a Use SSID_MAX_LEN define instead of value 32 when comparing SSID length
This makes the implementation easier to understand.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-04-22 11:44:18 +03:00
Jouni Malinen
65b10253d6 WPS: Ignore too long Device Name attribute
While it looks like all the users of this parsed attribute were able to
handle longer Device Name values, there is no valid use case for these
and to avoid any potential issues in upper layer components, enforce
maximum length (32 bytes) on the Device Name during parsing.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-04-22 11:44:18 +03:00
Jouni Malinen
cc6f24380c Add WPS_DEV_NAME_MAX_LEN define and use it when comparing length
This make code easier to understand.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-04-22 11:44:18 +03:00
Brian Gix
4f88fc0464 P2PS: WPS changes needed for P2PS default PIN
This provides additional WPS definitions and rules for negotiating use
of P2PS default PIN configuration method.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-02-02 16:55:45 +02:00
Jouni Malinen
6b46bfa751 WPS: Re-fix an interoperability issue with mixed mode and AP Settings
Commit ce7b56afab ('WPS: Fix an
interoperability issue with mixed mode and AP Settings') added code to
filter M7 Authentication/Encryption Type attributes into a single bit
value in mixed mode (WPA+WPA2) cases to work around issues with Windows
7. This workaround was lost in commit
d7a15d5953 ('WPS: Indicate current AP
settings in M7 in unconfigurated state') that fixed unconfigured state
values in AP Settings, but did not take into account the earlier
workaround for mixed mode.

Re-introduce filtering of Authentication/Encryption Type attributes for
M7 based on the current AP configuration. In other words, merge those
two earlier commits together to include both the earlier workaround the
newer fix.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2015-01-19 18:35:59 +02:00
Jouni Malinen
422ba11e30 Flush WPS registrar state on wpa_supplicant FLUSH command
This helps hwsim test cases by avoiding undesired state from previously
executed test cases affecting following tests.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-12-21 17:47:42 +02:00
Jouni Malinen
d85e1fc8a5 Check os_snprintf() result more consistently - automatic 1
This converts os_snprintf() result validation cases to use
os_snprintf_error() where the exact rule used in os_snprintf_error() was
used. These changes were done automatically with spatch using the
following semantic patch:

@@
identifier E1;
expression E2,E3,E4,E5,E6;
statement S1;
@@

(
  E1 = os_snprintf(E2, E3, ...);
|
  int E1 = os_snprintf(E2, E3, ...);
|
  if (E5)
	E1 = os_snprintf(E2, E3, ...);
  else
	E1 = os_snprintf(E2, E3, ...);
|
  if (E5)
	E1 = os_snprintf(E2, E3, ...);
  else if (E6)
	E1 = os_snprintf(E2, E3, ...);
  else
	E1 = 0;
|
  if (E5) {
	...
	E1 = os_snprintf(E2, E3, ...);
  } else {
	...
	return -1;
  }
|
  if (E5) {
	...
	E1 = os_snprintf(E2, E3, ...);
  } else if (E6) {
	...
	E1 = os_snprintf(E2, E3, ...);
  } else {
	...
	return -1;
  }
|
  if (E5) {
	...
	E1 = os_snprintf(E2, E3, ...);
  } else {
	...
	E1 = os_snprintf(E2, E3, ...);
  }
)
? os_free(E4);
- if (E1 < 0 || \( E1 >= E3 \| (size_t) E1 >= E3 \| (unsigned int) E1 >= E3 \| E1 >= (int) E3 \))
+ if (os_snprintf_error(E3, E1))
(
  S1
|
{ ... }
)

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-12-08 11:42:07 +02:00
Ilan Peer
f64d3f0e15 WPS: Add missing device types
Add missing device category and device sub category definitions.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
2014-10-26 01:24:08 +03:00
Jouni Malinen
683b408a9e WPS UPnP: Check os_get_random() result more consistently
While the value here would not matter much, make the implementation more
consistent (CID 72692).

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-10-11 19:08:30 +03:00
Jouni Malinen
92b5b371b6 WPS UPnP: Check os_get_random() result
This makes the implementation more consistent (CID 72705).

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-10-11 18:58:17 +03:00
Jouni Malinen
3e9fa3be62 WPS UPnP: Make dl_list_first() use easier for static analyzer
The preceding dl_list_len() check guarantees that dl_list_first()
returns an entry and not NULL. However, that seems to be a bit too
difficult path to follow for static analyzers, so add an explicit check
for the dl_list_first() return value to be non-NULL to avoid warnings.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-10-11 13:45:25 +03:00
Jouni Malinen
8511a0f67b WPS: Extend internal entropy pool help for key/snonce derivation
The internal entropy pool was previously used to prevent 4-way handshake
in AP mode from completing before sufficient entropy was available to
allow secure keys to be generated. This commit extends that workaround
for boards that do not provide secure OS level PRNG (e.g., /dev/urandom
does not get enough entropy) for the most critical WPS operations by
rejecting AP-as-enrollee case (use of AP PIN to learn/modify AP
configuration) and new PSK/passphrase generation. This does not have any
effect on devices that have an appropriately working OS level PRNG
(e.g., /dev/random and /dev/urandom on Linux).

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2014-09-08 12:54:18 +03:00
Jouni Malinen
18389abcca WPS: Clear keys/PINs explicitly
Use an explicit memset call to clear any configuration parameter and
dynamic data that contains private information like keys or identity.
This brings in an additional layer of protection by reducing the length
of time this type of private data is kept in memory.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-07-02 12:52:08 +03:00
Jouni Malinen
ce9c9bcc38 WPS: Use os_memcmp_const() for hash/password comparisons
This makes the implementation less likely to provide useful timing
information to potential attackers from comparisons of information
received from a remote device and private material known only by the
authorized devices.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-07-02 12:38:47 +03:00
Jouni Malinen
305000e160 WPS: Check wps_build_wfa_ext() return value consistently (CID 68104)
While this call cannot really fail, check the return value to be more
consistent with all the other wps_build_wfa_ext() calls.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-06-14 12:32:52 +03:00
Jouni Malinen
c02f35fb59 WPS: Clean up indentation level (CID 68109)
The implementation here was doing what it was supposed to, but the code
was indented in a way that made it quite confusing in the context of a
single line if statement body.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-06-12 19:45:31 +03:00
Jouni Malinen
86388afa56 WPS: Check for theoretical gmtime() failure
In theory, gmtime() could return NULL if the year value would not fit
into an integer. However, that cannot really happen with the current
time() value in practice. Anyway, clean up static analyzer reports by
checking for this corner case.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-06-07 17:39:51 +03:00
Jouni Malinen
d75a5ae018 WPS ER: Fix UDN parser to handle missing field
Must check that UDN was present before trying to parse it. Avoid a NULL
pointer dereference by checking the result before using os_strstr() when
parsing device description from an AP.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-06-07 17:35:22 +03:00
Jouni Malinen
0bceb8d6f4 Make dl_list_first() and dl_list_last() uses easier for static analyzers
The previous check for dl_list_len() or having an entry from the list is
sufficient, but some static analyzers cannot figure out that
dl_list_first() and dl_list_last() will return non-NULL in this type of
cases. Avoid invalid reports by explicitly checking for NULL.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-04-29 12:52:10 +03:00
Jouni Malinen
5f693cb1b6 WPS HTTP: Remove unused assignment
bbp is not used in the code path that skips trailers.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-04-29 12:52:10 +03:00
Jouni Malinen
27b418715f WPS: Print setsockopt() failure in debug log
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2014-04-28 16:54:09 +03:00
Jouni Malinen
dc39004318 WPS: Remove unused WEP related functionality
Now that WPS 2.0 support is enabled unconditionally, WEP and Shared auth
type are not allowed. This made some of the older code unused and that
can now be removed to clean up the implementation. There is still one
place where WEP is allowed for testing purposes: wpa_supplicant as
Registrar trying to configure an AP to use WEP. That is now only allowed
in CONFIG_TESTING_OPTIONS=y builds, though.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-04-06 12:31:45 +03:00
Jouni Malinen
c201f93a9e WPS: Enable WSC 2.0 support unconditionally
There is not much point in building devices with WPS 1.0 only supported
nowadays. As such, there is not sufficient justification for maintaining
extra complexity for the CONFIG_WPS2 build option either. Remove this by
enabling WSC 2.0 support unconditionally.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2014-03-25 18:33:21 +02:00
Jouni Malinen
f19e370e15 WPS: Do not advertise WPA/WPA2-Enterprise Auth Type Flags
While the device itself may support WPA/WPA2-Enterprise, enrollment of
credentials for EAP authentication is not supported through WPS. As
such, there is no need to claim support for these capabilities within
WPS information.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2014-03-20 15:13:48 +02:00
Jouni Malinen
fdb45355d4 WPS: Extend per-station PSK to support ER case as well
When wpa_psk_file is used instead of wpa_psk/wpa_passphrase, each WPS
Enrollee was given a unique PSK. This did not work for the
station-as-Registrar case where ER would learn the current AP settings
instead of enrolling itself (i.e., when using the AP PIN instead of
station PIN). That case can be covered with a similar design, so
generate a per-device PSK when building M7 as an AP in wpa_psk_file
configuration.

Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
2014-03-13 18:22:25 +02:00
Rahul Jain
8714caa1c2 WPS: Parse Registrar Configuration Methods
This new subelement was added into the WFA Vendor Extension.

Signed-off-by: Rahul Jain <rahul.jain@samsung.com>
2014-03-05 23:26:16 +02:00
Jouni Malinen
3b6170b78d WPS: Remove duplicate variable setting
There is no need to use a for loop here since the h variable is
set identically at the beginning of the body anyway.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-03-02 17:15:12 +02:00
Jouni Malinen
bd27b1360f Make code path easier for static analyzers
record->type == NULL case was handled through the record->type_length
comparison. While this was correct, it is a bit difficult for static
analyzers to understand, so add an extra check for NULL to avoid false
reports on this.

Signed-off-by: Jouni Malinen <j@w1.fi>
2014-03-02 17:15:11 +02:00