Commit Graph

418 Commits

Author SHA1 Message Date
Gleb Smirnoff
b8a6e03fac Widen NET_EPOCH coverage.
When epoch(9) was introduced to network stack, it was basically
dropped in place of existing locking, which was mutexes and
rwlocks. For the sake of performance mutex covered areas were
as small as possible, so became epoch covered areas.

However, epoch doesn't introduce any contention, it just delays
memory reclaim. So, there is no point to minimise epoch covered
areas in sense of performance. Meanwhile entering/exiting epoch
also has non-zero CPU usage, so doing this less often is a win.

Not the least is also code maintainability. In the new paradigm
we can assume that at any stage of processing a packet, we are
inside network epoch. This makes coding both input and output
path way easier.

On output path we already enter epoch quite early - in the
ip_output(), in the ip6_output().

This patch does the same for the input path. All ISR processing,
network related callouts, other ways of packet injection to the
network stack shall be performed in net_epoch. Any leaf function
that walks network configuration now asserts epoch.

Tricky part is configuration code paths - ioctls, sysctls. They
also call into leaf functions, so some need to be changed.

This patch would introduce more epoch recursions (see EPOCH_TRACE)
than we had before. They will be cleaned up separately, as several
of them aren't trivial. Note, that unlike a lock recursion the
epoch recursion is safe and just wastes a bit of resources.

Reviewed by:	gallatin, hselasky, cy, adrian, kristof
Differential Revision:	https://reviews.freebsd.org/D19111
2019-10-07 22:40:05 +00:00
Fabien Thomas
d5f39c34a6 Fix broken window replay check that will allow old packet to be accepted.
This was introduced in r309144.

Submitted by:	Jean-Francois HREN <jean-francois.hren@stormshield.eu>
Approved by:	ae@
MFC after:	3 days
2019-09-06 14:30:23 +00:00
Andrey V. Elsukov
bf1a213c07 Add missing new line in several log messages.
PR:		239694
MFC after:	1 week
2019-08-09 08:58:09 +00:00
Ryan Libby
0e2464ea18 netipsec key_register: check for M_NOWAIT alloc failure
Reviewed by:	ae, cem
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D20742
2019-06-25 15:43:52 +00:00
John Baldwin
0f70218343 Make the warning intervals for deprecated crypto algorithms tunable.
New sysctl/tunables can now set the interval (in seconds) between
rate-limited crypto warnings.  The new sysctls are:
- kern.cryptodev_warn_interval for /dev/crypto
- net.inet.ipsec.crypto_warn_interval for IPsec
- kern.kgssapi_warn_interval for KGSSAPI

Reviewed by:	cem
MFC after:	1 month
Relnotes:	yes
Sponsored by:	Chelsio Communications
Differential Revision:	https://reviews.freebsd.org/D20555
2019-06-11 23:00:55 +00:00
John Baldwin
c2fd516f3a Add deprecation warnings for IPsec algorithms deprecated in RFC 8221.
All of these algorithms are either explicitly marked MUST NOT, or they
are implicitly MUST NOTs by virtue of not being included in IETF's
list of protocols at all despite having assignments from IANA.

Specifically, this adds warnings for the following ciphers:
- des-cbc
- blowfish-cbc
- cast128-cbc
- des-deriv
- des-32iv
- camellia-cbc

Warnings for the following authentication algorithms are also added:
- hmac-md5
- keyed-md5
- keyed-sha1
- hmac-ripemd160

Reviewed by:	cem, gnn
MFC after:	3 days
Sponsored by:	Chelsio Communications
Differential Revision:	https://reviews.freebsd.org/D20340
2019-05-23 22:06:57 +00:00
Conrad Meyer
a8a16c7128 Replace read_random(9) with more appropriate arc4rand(9) KPIs
Reviewed by:	ae, delphij
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D19760
2019-04-04 01:02:50 +00:00
Mateusz Guzik
cc426dd319 Remove unused argument to priv_check_cred.
Patch mostly generated with cocinnelle:

@@
expression E1,E2;
@@

- priv_check_cred(E1,E2,0)
+ priv_check_cred(E1,E2)

Sponsored by:	The FreeBSD Foundation
2018-12-11 19:32:16 +00:00
Andrey V. Elsukov
adc7bb2237 Add sadb_x_sa2 extension to SADB_ACQUIRE requests.
SADB_ACQUIRE requests are send by kernel, when security policy doesn't
have corresponding security association for outbound packet. IKE daemon
usually registers its handler for such messages and when the kernel asks
for SA it can handle this request. Now such requests will contain
additional fields that can help IKE daemon to create SA. And IKE now
can create SAs using only information from SADB_ACQUIRE request, this
is useful when many if_ipsec(4) interfaces are in use and IKE doesn track
security policies that was installed by kernel.

Obtained from:	Yandex LLC
MFC after:	3 weeks
Sponsored by:	Yandex LLC
2018-10-21 14:19:16 +00:00
Andrey V. Elsukov
0ddfd867ed Fix witness warning in xform_init().
Do not call crypto_newsession() while holding xforms_lock mutex.
Release mutex before invoking crypto_newsession(), and use
ipsec_kmod_enter()/ipsec_kmod_exit() functions to protect from doing
access to unloaded kernel module memory.

Move xform-releated functions into subr_ipsec.c to be able use
ipsec_kmod_* functions. Also unconditionally build ipsec_kmod_*
functions, since now they are always used by IPSec code.

Add xf_cntr field to struct xformsw, it is used by ipsec_kmod_*
functions. Also constify xf_name field, since it is not expected to be
modified.

Approved by:	re (kib)
Differential Revision:	https://reviews.freebsd.org/D17302
2018-09-26 14:47:51 +00:00
Andrew Turner
5f901c92a8 Use the new VNET_DEFINE_STATIC macro when we are defining static VNET
variables.

Reviewed by:	bz
Sponsored by:	DARPA, AFRL
Differential Revision:	https://reviews.freebsd.org/D16147
2018-07-24 16:35:52 +00:00
Conrad Meyer
1b0909d51a OpenCrypto: Convert sessions to opaque handles instead of integers
Track session objects in the framework, and pass handles between the
framework (OCF), consumers, and drivers.  Avoid redundancy and complexity in
individual drivers by allocating session memory in the framework and
providing it to drivers in ::newsession().

Session handles are no longer integers with information encoded in various
high bits.  Use of the CRYPTO_SESID2FOO() macros should be replaced with the
appropriate crypto_ses2foo() function on the opaque session handle.

Convert OCF drivers (in particular, cryptosoft, as well as myriad others) to
the opaque handle interface.  Discard existing session tracking as much as
possible (quick pass).  There may be additional code ripe for deletion.

Convert OCF consumers (ipsec, geom_eli, krb5, cryptodev) to handle-style
interface.  The conversion is largely mechnical.

The change is documented in crypto.9.

Inspired by
https://lists.freebsd.org/pipermail/freebsd-arch/2018-January/018835.html .

No objection from:	ae (ipsec portion)
Reported by:	jhb
2018-07-18 00:56:25 +00:00
Conrad Meyer
2e08e39ff5 OCF: Add a typedef for session identifiers
No functional change.

This should ease the transition from an integer session identifier model to
an opaque pointer model.
2018-07-13 23:46:07 +00:00
Sean Bruno
968ac175e4 fix locking within tcp_ipsec_pcbctl() to match ipsec4_pcbctl(), ipsec4_pcbctl()
IPSEC_PCBCTL() functions, which include tcp_ipsec_pcbctl(),
ipsec4_pcbctl(), and ipsec6_pcbctl(), should all have matching locking
semantics.

ipsec4_pcbctl() and ipsec6_pcbctl() expect the inp to be unlocked on
entry and exit and appear to be correctly implemented as such. But
tcp_ipsec_pcbctl() had other semantics. This patch fixes the semantics
for tcp_ipsec_pcbctl().

Submitted by:	Jason Eggleston <jason@eggnet.com>
MFH:		2 weeks
Sponsored by:	Limelight Networks
Differential Revision:	https://reviews.freebsd.org/D14623
2018-07-04 17:10:07 +00:00
Ed Maste
5b2b45a421 r335795 build fix: make static functions static
-Werror,-Wmissing-prototypes makes this an error otherwise.

MFC with:	335795
Sponsored by:	The FreeBSD Foundation
2018-06-29 14:51:36 +00:00
Andrey V. Elsukov
52b3619f45 Make debug output produced by setkey -x command a more human readable.
Add text names of SADB message types and extension headers to the output.

Obtained from:	Yandex LLC
MFC after:	2 weeks
Sponsored by:	Yandex LLC
Differential Revision:	https://reviews.freebsd.org/D16036
2018-06-29 13:59:33 +00:00
Mateusz Guzik
4e180881ae uma: implement provisional api for per-cpu zones
Per-cpu zone allocations are very rarely done compared to regular zones.
The intent is to avoid pessimizing the latter case with per-cpu specific
code.

In particular contrary to the claim in r334824, M_ZERO is sometimes being
used for such zones. But the zeroing method is completely different and
braching on it in the fast path for regular zones is a waste of time.
2018-06-08 21:40:03 +00:00
Andrey V. Elsukov
6d8fdfa9d5 Rework IP encapsulation handling code.
Currently it has several disadvantages:
- it uses single mutex to protect internal structures. It is used by
  data- and control- path, thus there are no parallelism at all.
- it uses single list to keep encap handlers for both INET and INET6
  families.
- struct encaptab keeps unneeded information (src, dst, masks, protosw),
  that isn't used by code in the source tree.
- matches are prioritized and when many tunneling interfaces are
  registered, encapcheck handler of each interface is invoked for each
  packet. The search takes O(n) for n interfaces. All this work is done
  with exclusive lock held.

What this patch includes:
- the datapath is converted to be lockless using epoch(9) KPI.
- struct encaptab now linked using CK_LIST.
- all unused fields removed from struct encaptab. Several new fields
  addedr: min_length is the minimum packet length, that encapsulation
  handler expects to see; exact_match is maximum number of bits, that
  can return an encapsulation handler, when it wants to consume a packet.
- IPv6 and IPv4 handlers are stored in separate lists;
- added new "encap_lookup_t" method, that will be used later. It is
  targeted to speedup lookup of needed interface, when gif(4)/gre(4) have
  many interfaces.
- the need to use protosw structure is eliminated. The only pr_input
  method was used from this structure, so I don't see the need to keep
  using it.
- encap_input_t method changed to avoid using mbuf tags to store softc
  pointer. Now it is passed directly trough encap_input_t method.
  encap_getarg() funtions is removed.
- all sockaddr structures and code that uses them removed. We don't have
  any code in the tree that uses them. All consumers use encap_attach_func()
  method, that relies on invoking of encapcheck() to determine the needed
  handler.
- introduced struct encap_config, it contains parameters of encap handler
  that is going to be registered by encap_attach() function.
- encap handlers are stored in lists ordered by exact_match value, thus
  handlers that need more bits to match will be checked first, and if
  encapcheck method returns exact_match value, the search will be stopped.
- all current consumers changed to use new KPI.

Reviewed by:	mmacy
Sponsored by:	Yandex LLC
Differential Revision:	https://reviews.freebsd.org/D15617
2018-06-05 20:51:01 +00:00
Conrad Meyer
ede2f7731d Correctly handle the padding for IPv6-AH, as specified by RFC4302
The RFC specifies that under IPv6 the complete AH header must be 64 bit
aligned, and under IPv4, 32 bit aligned. Prior to this change, we (along
with other BSDs and MacOS) had violated this requirement.

This makes it possible to set up IPv6-AH between Linux and BSD, and also
probably between Windows and BSD.

PR:		222684
Reported and tested by:	Jason Mader <jasonmader AT gmail.com>
Obtained from:	NetBSD xform_ah.c 1.105
		(b939fe2483972eb43d71bf990cfb7f26dece7839 NetBSD/src on GH)
		by Maxime Villard
MFC after:	35.2731 hours
Relnotes:	probably (breaks ipv6 compat with older FreeBSD/NetBSD/MacOS)
Sponsored by:	Dell EMC Isilon
2018-06-04 18:51:06 +00:00
Andrey V. Elsukov
33c1b2bd88 Temporary disable SPDCACHE statistic accounting until proper fix will be
committed. This fixes the kernel build without option IPSEC.
2018-05-28 09:23:28 +00:00
Matt Macy
c82dfce3ec netipsec/!VIMAGE: don't declare/define spdcache_destroy on non-VIMAGE builds
this breaks MIPS compiles in universe
2018-05-24 23:47:27 +00:00
Fabien Thomas
f8e73c47d8 Add a SPD cache to speed up lookups.
When large SPDs are used, we face two problems:

- too many CPU cycles are spent during the linear searches in the SPD
  for each packet
- too much contention on multi socket systems, since we use a single
  shared lock.

Main changes:

- added the sysctl tree 'net.key.spdcache' to control the SPD cache
  (disabled by default).
- cache the sp indexes that are used to perform SP lookups.
- use a range of dedicated mutexes to protect the cache lines.

Submitted by: Emeric Poupon <emeric.poupon@stormshield.eu>
Reviewed by: ae
Sponsored by:	Stormshield
Differential Revision: https://reviews.freebsd.org/D15050
2018-05-22 15:54:25 +00:00
Andrey V. Elsukov
bf18dfa28e Merge r1.22-1.23 from NetBSD:
Don't assume M_PKTHDR is set only on the first mbuf of the chain.
  The check is replaced by (m1 != m), which is equivalent to the previous
  code: we want to modify m->m_pkthdr.len only when 'm' was not passed in
  m_adj().

  Fix a pretty bad mistake, that has always been there:
   m_adj(m1, -(m1->m_len - roff));
   if (m1 != m)
	m->m_pkthdr.len -= (m1->m_len - roff);

  This is wrong: m_adj() will modify m1->m_len, so we're using a wrong
  value when manually adjusting m->m_pkthdr.len.

Reported by:	Maxime Villard <max at m00nbsd dot net>
Obtained from:	NetBSD
MFC after:	1 week
2018-04-26 12:23:31 +00:00
John Baldwin
fd40ecf3d4 Set the proper vnet in IPsec callback functions.
When using hardware crypto engines, the callback functions used to handle
an IPsec packet after it has been encrypted or decrypted can be invoked
asynchronously from a worker thread that is not associated with a vnet.
Extend 'struct xform_data' to include a vnet pointer and save the current
vnet in this new member when queueing crypto requests in IPsec.  In the
IPsec callback routines, use the new member to set the current vnet while
processing the modified packet.

This fixes a panic when using hardware offload such as ccr(4) with IPsec
after VIMAGE was enabled in GENERIC.

Reported by:	Sony Arpita Das and Harsh Jain @ Chelsio
Reviewed by:	bz
MFC after:	1 week
Sponsored by:	Chelsio Communications
Differential Revision:	https://reviews.freebsd.org/D14763
2018-03-20 17:05:23 +00:00
Andrey V. Elsukov
017a5e581a Rework key_sendup_mbuf() a bit:
o count in_nomem counter when we have failed to allocate mbuf for
  promisc socket;
o count in_msgtarget counter when we have secussfully sent data to socket;
o Since we are sending messages in a loop, returning error on first fail
  interrupts the loop, and all remaining sockets will not receive this
  message. So, do not return error when we have failed to send data to ALL
  or REGISTERED target. Return error only for KEY_SENDUP_ONE case. Now,
  when some socket has overfilled its receive buffer, this will not break
  other sockets.

MFC after:	2 weeks
2018-03-11 19:14:01 +00:00
Andrey V. Elsukov
d158b221e5 Add KASSERT to check that proper targed was used.
MFC after:	2 weeks
2018-03-11 18:46:40 +00:00
Andrey V. Elsukov
e3004d2429 Replace panic() with KASSERTs.
MFC after:	2 weeks
2018-03-11 18:37:55 +00:00
Andrey V. Elsukov
dccd41cb39 Check that we have PF_KEY sockets before iterating over all RAW sockets.
MFC after:	2 weeks
2018-03-11 18:10:59 +00:00
Andrey V. Elsukov
26cb1e1dba Remove obsoleted and unused key_sendup() function.
Also remove declaration for nonexistend key_usrreq() function.

MFC after:	2 weeks
2018-03-11 18:03:55 +00:00
Andrey V. Elsukov
15bf717a93 Remove unused variables and sysctl declaration.
MFC after:	1 week
2018-02-19 12:20:51 +00:00
Andrey V. Elsukov
6ca39da354 Check packet length to do not make out of bounds access. Also save ah_nxt
value to use it later, since ah pointer can become invalid.

Reported by:	Maxime Villard <max at m00nbsd dot net>
MFC after:	5 days
2018-02-19 11:14:38 +00:00
Andrey V. Elsukov
055679e67e Adopt revision 1.76 and 1.77 from NetBSD:
Fix a vulnerability in IPsec-IPv6-AH, that allows an attacker to remotely
  crash the kernel with a single packet.

  In this loop we need to increment 'ad' by two, because the length field
  of the option header does not count the size of the option header itself.

  If the length is zero, then 'count' is incremented by zero, and there's
  an infinite loop. Beyond that, this code was written with the assumption
  that since the IPv6 packet already went through the generic IPv6 option
  parser, several fields are guaranteed to be valid; but this assumption
  does not hold because of the missing '+2', and there's as a result a
  triggerable buffer overflow (write zeros after the end of the mbuf,
  potentially to the next mbuf in memory since it's a pool).

  Add the missing '+2', this place will be reinforced in separate commits.

Reported by:	Maxime Villard <maxv at NetBSD.org>
MFC after:	1 week
2018-01-24 19:48:25 +00:00
Andrey V. Elsukov
b12a7532e3 Merge revision 1.35 from NetBSD:
fix pointer/offset mistakes in handling of IPv4 options

Reported by:	Maxime Villard <maxv at NetBSD.org>
MFC after:	1 week
2018-01-24 19:06:44 +00:00
Alexander Kabaev
151ba7933a Do pass removing some write-only variables from the kernel.
This reduces noise when kernel is compiled by newer GCC versions,
such as one used by external toolchain ports.

Reviewed by: kib, andrew(sys/arm and sys/arm64), emaste(partial), erj(partial)
Reviewed by: jhb (sys/dev/pci/* sys/kern/vfs_aio.c and sys/kern/kern_synch.c)
Differential Revision: https://reviews.freebsd.org/D10385
2017-12-25 04:48:39 +00:00
Andrey V. Elsukov
d8ba1ddc0f Do better cleaning in key_destroy() for VIMAGE case.
SPDB was cleaned using TAILQ_CONCAT() instead of calling key_unlink()
for each SP, thus we need to properly clean lists in each bucket of
V_sphashtbl to avoid panic in hashdestroy() when INVARIANTS is enabled.

Do the same for V_acqaddrhashtbl and V_acqseqhashtbl.

When we are called in DEFAULT_VNET, destroy also all global locks and
drain key_timer callout.

Reported by:	kp
Tested by:	kp
MFC after:	1 week
2017-12-01 09:59:42 +00:00
Pedro F. Giffuni
fe267a5590 sys: general adoption of SPDX licensing ID tags.
Mainly focus on files that use BSD 2-Clause license, however the tool I
was using misidentified many licenses so this was mostly a manual - error
prone - task.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.

No functional change intended.
2017-11-27 15:23:17 +00:00
Pedro F. Giffuni
51369649b0 sys: further adoption of SPDX licensing ID tags.
Mainly focus on files that use BSD 3-Clause license.

The Software Package Data Exchange (SPDX) group provides a specification
to make it easier for automated tools to detect and summarize well known
opensource licenses. We are gradually adopting the specification, noting
that the tags are considered only advisory and do not, in any way,
superceed or replace the license texts.

Special thanks to Wind River for providing access to "The Duke of
Highlander" tool: an older (2014) run over FreeBSD tree was useful as a
starting point.
2017-11-20 19:43:44 +00:00
Conrad Meyer
f95f6841c8 ipsec: Use the same keysize values for HMAC as prior to r324017
The HMAC construction natively permits any key size between 0 and the input
block length. Before r324017, the auth_hash 'keysize' member was the hash
output length, which was used by ipsec for key sizes. (Non-ipsec consumers
need the ability to use other keysizes, hence, r324017.)

The ipsec SADB code blindly uses the auth_hash 'keysize' member for both
minimum and maximum key size, which is wrong (from an HMAC perspective).
For now, just switch it to 'hashsize', which matches the existing
expectations.

Instead it should probably use the range [0, keysize]. But there may be
other broken code in ipsec that rejects hashes with too small a minimum
key size.

Reported by:	olivier@
Reviewed by:	olivier, no objection from ae
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D12770
2017-11-15 22:42:20 +00:00
Andrey V. Elsukov
cd48d883bd Use correct pointer in key_updateaddresses() when updating NAT-T config.
key_updateaddresses() is used to update SA addresses and NAT-T
configuration in SADB_UPDATE message. This is done using cloning SA
content from old SA into new one. But addresses and NAT-T configuration
are taking from SADB_UPDATE message. Use newsa pointer to set NAT-T
properties into cloned SA.

PR:		223382
MFC after:	1 week
2017-11-03 11:33:13 +00:00
Fabien Thomas
39bbca6ffd crypto(9) is called from ipsec in CRYPTO_F_CBIFSYNC mode. This is working
fine when a lot of different flows to be ciphered/deciphered are involved.

However, when a software crypto driver is used, there are
situations where we could benefit from making crypto(9) multi threaded:
- a single flow is to be ciphered: only one thread is used to cipher it,
- a single ESP flow is to be deciphered: only one thread is used to
decipher it.

The idea here is to call crypto(9) using a new mode (CRYPTO_F_ASYNC) to
dispatch the crypto jobs on multiple threads, if the underlying crypto
driver is working in synchronous mode.

Another flag is added (CRYPTO_F_ASYNC_KEEPORDER) to make crypto(9)
dispatch the crypto jobs in the order they are received (an additional
queue/thread is used), so that the packets are reinjected in the network
using the same order they were posted.

A new sysctl net.inet.ipsec.async_crypto can be used to activate
this new behavior (disabled by default).

Submitted by:	Emeric Poupon <emeric.poupon@stormshield.eu>
Reviewed by:	ae, jmg, jhb
Differential Revision:    https://reviews.freebsd.org/D10680
Sponsored by:	Stormshield
2017-11-03 10:27:22 +00:00
Conrad Meyer
3693b18840 opencrypto: Loosen restriction on HMAC key sizes
Theoretically, HMACs do not actually have any limit on key sizes.
Transforms should compact input keys larger than the HMAC block size by
using the transform (hash) on the input key.

(Short input keys are padded out with zeros to the HMAC block size.)

Still, not all FreeBSD crypto drivers that provide HMAC functionality
handle longer-than-blocksize keys appropriately, so enforce a "maximum" key
length in the crypto API for auth_hashes that previously expressed a
requirement.  (The "maximum" is the size of a single HMAC block for the
given transform.)  Unconstrained auth_hashes are left as-is.

I believe the previous hardcoded sizes were committed in the original
import of opencrypto from OpenBSD and are due to specific protocol
details of IPSec.  Note that none of the previous sizes actually matched
the appropriate HMAC block size.

The previous hardcoded sizes made the SHA tests in cryptotest.py
useless for testing FreeBSD crypto drivers; none of the NIST-KAT example
inputs had keys sized to the previous expectations.

The following drivers were audited to check that they handled keys up to
the block size of the HMAC safely:

  Software HMAC:
    * padlock(4)
    * cesa
    * glxsb
    * safe(4)
    * ubsec(4)

  Hardware accelerated HMAC:
    * ccr(4)
    * hifn(4)
    * sec(4) (Only supports up to 64 byte keys despite claiming to
      support SHA2 HMACs, but validates input key sizes)
    * cryptocteon (MIPS)
    * nlmsec (MIPS)
    * rmisec (MIPS) (Amusingly, does not appear to use key material at
      all -- presumed broken)

Reviewed by:	jhb (previous version), rlibby (previous version)
Sponsored by:	Dell EMC Isilon
Differential Revision:	https://reviews.freebsd.org/D12437
2017-09-26 16:18:10 +00:00
Andrey V. Elsukov
6f9e437bdc Fix possible double releasing for SA reference.
This is missing part of r318734. When crypto subsystem returns error
the xform code handles an error independently.

PR:		221849
MFC after:	5 days
2017-09-01 11:51:07 +00:00
Andrey V. Elsukov
c32396978e Remove stale comments.
MFC after:	1 week
2017-08-21 13:54:29 +00:00
Andrey V. Elsukov
22bbefb2c9 Fix the regression introduced in r275710.
When a security policy should match TCP connection with specific ports,
the SYN+ACK segment send by syncache_respond() is considered as forwarded
packet, because at this moment TCP connection does not have PCB structure,
and ip_output() is called without inpcb pointer. In this case SPIDX filled
for SP lookup will not contain TCP ports and security policy will not
be found. This can lead to unencrypted SYN+ACK on the wire.

This patch restores the old behavior, when ports will not be filled only
for forwarded packets.

Reported by:	Dewayne Geraghty <dewayne.geraghty at heuristicsystems.com.au>
MFC after:	1 week
2017-08-21 13:52:21 +00:00
Andrey V. Elsukov
e54647920b Make user supplied data checks a bit stricter.
key_msg2sp() is used for parsing data from setsockopt(IP[V6]_IPSEC_POLICY)
call. This socket option is usually used to configure IPsec bypass for
socket. Only privileged user can set this socket option.
The message syntax is described here
	http://www.kame.net/newsletter/20021210/

and our libipsec is usually used to create the correct request.
Add additional checks:
* that sadb_x_ipsecrequest_len is not out of bounds of user supplied buffer
* that src/dst's sa_len is the same
* that 2*sa_len is not out of bounds of user supplied buffer
* that 2*sa_len fits into bounds of sadb_x_ipsecrequest

Reported by:	Ilja van Sprundel
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D11796
2017-08-09 19:58:38 +00:00
Andrey V. Elsukov
1a01e0e7ac Add inpcb pointer to struct ipsec_ctx_data and pass it to the pfil hook
from enc_hhook().

This should solve the problem when pf is used with if_enc(4) interface,
and outbound packet with existing PCB checked by pf, and this leads to
deadlock due to pf does its own PCB lookup and tries to take rlock when
wlock is already held.

Now we pass PCB pointer if it is known to the pfil hook, this helps to
avoid extra PCB lookup and thus rlock acquiring is not needed.
For inbound packets it is safe to pass NULL, because we do not held any
PCB locks yet.

PR:		220217
MFC after:	3 weeks
Sponsored by:	Yandex LLC
2017-07-31 11:04:35 +00:00
Andrey V. Elsukov
8e5060a03e Build kdebug_secreplay() function only when IPSEC_DEBUG is defined.
This should fix the build on sparc.

Reported by:	emaste
X-MFC with:	r319118
2017-06-01 10:04:12 +00:00
Andrey V. Elsukov
7f1f65918b Disable IPsec debugging code by default when IPSEC_DEBUG kernel option
is not specified.

Due to the long call chain IPsec code can produce the kernel stack
exhaustion on the i386 architecture. The debugging code usually is not
used, but it requires a lot of stack space to keep buffers for strings
formatting. This patch conditionally defines macros to disable building
of IPsec debugging code.

IPsec currently has two sysctl variables to configure debug output:
 * net.key.debug variable is used to enable debug output for PF_KEY
   protocol. Such debug messages are produced by KEYDBG() macro and
   usually they can be interesting for developers.
 * net.inet.ipsec.debug variable is used to enable debug output for
   DPRINTF() macro and ipseclog() function. DPRINTF() macro usually
   is used for development debugging. ipseclog() function is used for
   debugging by administrator.

The patch disables KEYDBG() and DPRINTF() macros, and formatting buffers
declarations when IPSEC_DEBUG is not present in kernel config. This reduces
stack requirement for up to several hundreds of bytes.
The net.inet.ipsec.debug variable still can be used to enable ipseclog()
messages by administrator.

PR:		219476
Reported by:	eugen
No objection from:	#network
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D10869
2017-05-29 09:30:38 +00:00
Andrey V. Elsukov
3aee70991d Fix possible double releasing for SA and SP references.
There are two possible ways how crypto callback are called: directly from
caller and deffered from crypto thread.

For outbound packets the direct call chain is the following:
 IPSEC_OUTPUT() method -> ipsec[46]_common_output() ->
 -> ipsec[46]_perform_request() -> xform_output() ->
 -> crypto_dispatch() -> crypto_invoke() -> crypto_done() ->
 -> xform_output_cb() -> ipsec_process_done() -> ip[6]_output().

The SA and SP references are held while crypto processing is not finished.
The error handling code wrongly expected that crypto callback always called
from the crypto thread context, and it did references releasing in
xform_output_cb(). But when the crypto callback called directly, in case of
error the error handling code in ipsec[46]_perform_request() also did
references releasing.

To fix this, remove error handling from ipsec[46]_perform_request() and do it
in xform_output() before crypto_dispatch().

MFC after:	10 days
2017-05-23 09:32:26 +00:00
Andrey V. Elsukov
5f7c516f21 Fix possible double releasing for SA reference.
There are two possible ways how crypto callback are called: directly from
caller and deffered from crypto thread.

For inbound packets the direct call chain is the following:
 IPSEC_INPUT() method -> ipsec_common_input() -> xform_input() ->
 -> crypto_dispatch() -> crypto_invoke() -> crypto_done() ->
 -> xform_input_cb() -> ipsec[46]_common_input_cb() -> netisr_queue().

The SA reference is held while crypto processing is not finished.
The error handling code wrongly expected that crypto callback always called
from the crypto thread context, and it did SA reference releasing in
xform_input_cb(). But when the crypto callback called directly, in case of
error (e.g. data authentification failed) the error handling in
ipsec_common_input() also did SA reference releasing.

To fix this, remove error handling from ipsec_common_input() and do it
in xform_input() before crypto_dispatch().

PR:		219356
MFC after:	10 days
2017-05-23 09:01:48 +00:00