Commit Graph

219 Commits

Author SHA1 Message Date
sobomax
034bc5d427 Further refine r336195: do not even attempt to verify/update interface's
MTU if we've set it once and there were no changes on the DHCP server
side since the last refresh. This is consistent I believe with how dhclient
handles other settings like IP address, mask etc.

Approved by:	cem, eugen
Differential Revision:	https://reviews.freebsd.org/D18546
2019-02-23 23:31:13 +00:00
jilles
8f8b9d94a5 dhclient: Return non-zero status when script exits due to a signal
r343896 made it such that a non-zero exit status was passed through, but was
still wrong if the script exits on a signal. POSIX does not say what the
WEXITSTATUS macro returns in this case and in practice 0 is a common value.

Instead, translate the wait status into 8 bits the same way as the shell
calculates $?.

Reviewed by:	kib, Nash Kaminski
MFC after:	1 week
2019-02-08 23:03:28 +00:00
kib
0a987876ef Correctly return exit status from the exited process.
This is also OpenBSD rev. 1.117, as pointed out by
Ryan Moeller <ryan@ixsystems.com>.

Submitted by:	Nash Kaminski <nashkaminski@gmail.com>
MFC after:	1 week
2019-02-08 07:36:08 +00:00
oshogbo
b60d6a5607 capsicum: use a new capsicum helpers in tools
Use caph_{rights,ioctls,fcntls}_limit to simplify the code.
2018-11-04 19:24:49 +00:00
brd
93e7e0f67a Move dhclient.conf to sbin/dhclient/.
This also leverages CONFS for handling config files.

Approved by:	re (gjb), will (mentor)
Differential Revision:	https://reviews.freebsd.org/D17160
2018-09-18 00:11:45 +00:00
brd
e0bc05ab40 Remove dhclient.conf from here in prep for the move of it from etc/.
This is being done a separate step to ease importing into other VCSes.

Approved by:	re (gjb), will (mentor)
Differential Revision:	https://reviews.freebsd.org/D17160
2018-09-18 00:10:29 +00:00
emaste
4e22ee3754 Allow dhclient and ping to build WITHOUT_DYNAMICROOT
dhclient and ping normally use libcasper services.  These are not
available in statically-linked binaries, so when WITHOUT_DYNAMICROOT is
set disable libcasper use, as with rescue builds.  Also emit a warning
as it's undesirable to build this way.

Reported by:	Michael Dexter
Reviewed by:	rgrimes
Tested by:	Michael Dexter
Approved by:	re (kib)
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D17074
2018-09-09 17:26:44 +00:00
markj
3f47b833e6 dhclient: Enter capability mode before dropping privileges.
This is needed to be able to chroot in the fallback case where
Capsicum is not available.

Reported by:	Daniel Braniss <danny@cs.huji.ac.il>
X-MFC with:	r337382
Sponsored by:	The FreeBSD Foundation
2018-08-07 13:50:21 +00:00
markj
f571872c4d dhclient: Don't chroot if we are in capability mode.
The main dhclient process is Capsicumized but also chroots to
restrict filesystem access.  With r322369, pidfile(3) maintains a
directory descriptor for the pidfile, which can cause the chroot
to fail in certain cases.  To minimize the problem, only chroot
if we fail to enter capability mode, and store dhclient pidfiles
in a subdirectory of /var/run, thus restricting access via
pidfile(3)'s directory descriptor.

PR:		223327
Reviewed by:	cem, oshogbo
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D16584
2018-08-06 16:22:01 +00:00
eugen
bc4b5f2cdc Make dhclient(8) verify if new MTU (option 26) differs from current one and skip unneeded MTU change.
This check eliminates infinite loop of MTU change / link flap / lease verification / MTU change / link flap etc.
in case of some NIC drivers like em(4) or igb(4).

N.B.: obsolete u_int16_t is used in consistency with the rest of the file.

PR:		229432
Approved by:	mav (mentor)
MFC after:	1 week
2018-07-11 09:41:50 +00:00
eadler
e20bc09203 dhclient: recorrect __progname to getprogname()
A more correct way to modernize code that uses __progname is to just
replace each occurance of it with a call to getprogname(3)

Reported by:	ian
Reviewed by:	imp
2018-06-25 01:29:54 +00:00
eadler
813adb3812 dhclient: build with WARNS=6
- add static in a number of places
- initialize __progname rather than rely on magical extern values
- use nitems() instead of manually spelling it out
- unshadow 'idi'
- teach 'error' that it is '__dead2'
- add missing 'break'
2018-06-24 13:23:27 +00:00
oshogbo
8acf9b1609 Convert cap_enter() < 0 && errno != ENOSYS to caph_enter() < 0.
No functional change intended.
2018-06-19 23:43:14 +00:00
cem
4ece63474c dhclient(8): allow to supersede interface-mtu option
In some cases broken DHCP servers might send invalid MTU value, so allow to
use 'supersede' in dhclient.conf to override this. When superseded value is
0, MTU value is not updated at all.

PR:		206721
Submitted by:	novel@
Reported by:	<jimp AT pfsense.org>
MFC after:	37 minutes (if you care about 11, please MFC to 11.2)
Relnotes:	yes (potentially surprising behavior change w/ broken dhcpd mtu)
Differential Revision:	https://reviews.freebsd.org/D15484
2018-05-31 19:36:24 +00:00
dab
2ec60e4f58 dhclient violates RFC2131 when sending early DHCPREQUEST message to re-obtain old IP
When dhclient first starts, if an old IP address exists in the
dhclient.leases file, dhclient(8) sends early DHCPREQUEST message(s)
in an attempt to re-obtain the old IP address again. These messages
contain the old IP as a requested-IP-address option in the message
body (correct) but also use the old IP address as the packet's source
IP (incorrect).

RFC2131 sec 4.1 states:

  DHCP messages broadcast by a client prior to that client obtaining
  its IP address must have the source address field in the IP header
  set to 0.

The use of the old IP as the packet's source address is incorrect if
(a) the computer is now on a different network or (b) it is on the
same network, but the old IP has been reallocated to another host.

Fix dhclient to use 0.0.0.0 as the source IP in this circumstance
without removing any existing functionality. Any previously-used old
IP is still requested in the body of an early DHCPREQUEST message.

PR:		199378
Submitted by:	J.R. Oldroyd <fbsd@opal.com>
Reported by:	J.R. Oldroyd <fbsd@opal.com>
Reviewed by:	cem, asomers, vangyzen
MFC after:	1 week
Sponsored by:	Dell EMC
Differential Revision:	https://reviews.freebsd.org/D14527
2018-02-27 21:59:23 +00:00
asomers
09ecb465d2 dhclient: raise WARNS to 4
Mostly const-correctness fixes. There were also some variable-shadowing,
unused variable, and a couple of sockaddr type-correctness changes. I also had
trouble with cast-align warnings. I was able to prove that one of them was a
false positive. But ultimately I had to disable the warning program-wide to
deal with the others.

Reviewed by:	cem
MFC after:	3 weeks
Sponsored by:	Spectra Logic Corp
Differential Revision:	https://reviews.freebsd.org/D14460
2018-02-21 21:13:08 +00:00
oshogbo
ae68afbc70 Use daemonfd(3) in the dhclient(8).
Reviewed by:	brooks@
Differential Revision:	https://reviews.freebsd.org/D13603
2018-02-02 18:11:56 +00:00
asomers
91f667b32e dhclient(8): add missing include
stdbool is technically needed, though we can get away without it due to
header pollution.

MFC after:	3 days
2018-01-12 04:26:40 +00:00
asomers
5c3755a444 dhclient(8): raise WARNS to 3
Mostly had to fix a lot of signed/unsigned comparison warnings

MFC after:	3 weeks
Sponsored by:	Spectra Logic Corp
2017-12-14 15:41:32 +00:00
asomers
05786d4442 dhclient(8): Don't shift through the sign bit of a signed int
PR:		208007
Submitted by:	Michael McConville <mmcco@mykolab.com>
MFC after:	3 weeks
2017-12-13 20:48:20 +00:00
pfg
872b698bd4 General 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:49:47 +00:00
oshogbo
799ff1d3f3 dhclient(8) should not close stdio using daemon(3) function.
dhclient(8) is chrooted so opening /dev/null always will fail.
In capability world this is also annoying because we getting error that
open(2) is not permitted in Capsicum. dhclient(8) is closing stdio by
precaching fd to /dev/null before chroot.
This is done few lines below daemon(3) function so let's not try to do that
in daemon(3) function.

Reviewed by:	cem@
Differential Revision:	https://reviews.freebsd.org/D12826
2017-11-12 08:54:23 +00:00
oshogbo
90ae5f55e7 Use syslog service in dhclient(8).
dhclient(8) is failing during boot to connect to the syslog service, because
syslog daemon is started after dhclient(8). This can be reproduced by stooping
syslog daemon and ktrace the dhclient or use kern.trap_enotcap sysctl and boot
the machine. Using the Casper syslog service fix the problem.

Reviewed by:	bapt@
Differential Revision:	https://reviews.freebsd.org/D12825
2017-11-12 08:42:43 +00:00
bdrewery
a598c4b809 DIRDEPS_BUILD: Update dependencies.
Sponsored by:	Dell EMC Isilon
2017-10-31 00:07:04 +00:00
ngie
d26727d972 Add HAS_TESTS to all Makefiles that are currently using the
`SUBDIR.${MK_TESTS}+= tests` idiom.

This is a follow up to r321912.
2017-08-02 08:50:42 +00:00
ngie
734d081ed1 MFhead@r321912 2017-08-02 08:38:36 +00:00
stevek
f7f5f7e196 parse.c parse_string
When parse_semi fails, free s before returning

parse.c parse_numeric_aggregate
The memory assigned to bufp is complicated, it can either be from the input
parameter buf or allocated locally. Introduce a new variable lbufp to track
when it is assigned locally and to free it when appropriate.

Submitted by:	Thomas Rix <trix@juniper.net>
Reviewed by:	jhb
Approved by:	sjg (mentor)
Obtained from:	Juniper Networks, Inc.
MFC after:	1 week
Differential Revision:	https://reviews.freebsd.org/D9899
2017-05-31 21:31:15 +00:00
n_hibma
c9550acb7c Fix the output of very large rebind, renew and lease time options in
lease file.

Some routers set very large values for rebind time (Netgear) and these
are erroneously reported as negative in the leasefile. This was due to a
wrong printf format specification of %ld for an unsigned long on 32-bit
platforms.
2017-05-07 21:11:28 +00:00
n_hibma
1260ccd59c Fix handling of large DHCP expiry values.
They would overflow a signed 32-bit time_t on 32 bit architectures. This
was taken care of, but a compiler optimisation makes this behave
erratically. This could be resolved by adding a -fwrapv flag, but
instead we can check the value before adding the current timestamp to
it.

In the lease file values are still wrong though:

  option dhcp-rebinding-time -644245096;

PR:		218980
Reported by:	Bob Eager
MFC after:	2 weeks
2017-05-07 19:59:37 +00:00
n_hibma
03ca34f30c Make dhcp-lease-time option supersedable as well.
Note: It is not recommended to set this value to above the value that
the server provided, unless that value is bogus.
2017-03-30 18:20:04 +00:00
n_hibma
f7f038e32a Allow superseding the lease renewal and rebind times.
Also make sure that the renewal is never more than 1/2 * expiry and
rebind never more than 7/4 * renewal (the default values in the spec).
This should allow adjusting high values from the server as well as
making sure the values from the server make sense.

Renewal and rebind times will be adjusted down if the expiry time is set
very high in a server, not the other way around. This change just makes
sure the values keep making sense.
2017-03-30 17:31:12 +00:00
ngie
2fad502048 sbin/dhclient: fix vendor storage leak in parse_option_decl(..)
This ensures the storage isn't leaked when non-NULL and the function
returns early, prior to the `free(vendor)` later on in the function.

MFC after:	1 week
Reported by:	Coverity
CID:		1007111-1007113
Reviewed by:	cem
Sponsored by:	Dell EMC Isilon
Differential Revision:	D9993
2017-03-13 17:45:00 +00:00
ngie
9d2dfc21b0 Fix -Wunused-but-set-warning with ret
While here, resolve Coverity warnings by demonstrating that vfprintf's
return value is being explicitly ignored.

MFC after:	1 week
Reported by:	gcc 6.3.0
Tested with:	clang, gcc 4.2.1, gcc 6.3.0
Sponsored by:	Dell EMC Isilon
2017-03-13 17:15:45 +00:00
ngie
b990ff3c5d sbin/dhclient: fix a memory leak in parse_client_lease_statement(..)
The memory stored by `lease` would have previously been leaked if an
unterminated lease declaration was found in an early-return code path.

MFC after:	1 week
Reported by:	clang static analyzer, Coverity
CID:		1007114
Submitted by:	Tom Rix <trix@juniper.net>
Sponsored by:	Dell EMC Isilon; Juniper, Inc
Differential Revision:	D9992
2017-03-13 17:05:49 +00:00
ngie
3c4b37d990 sbin: normalize paths using SRCTOP-relative paths or :H when possible
This simplifies make logic/output

MFC after:	1 month
Sponsored by:	Dell EMC Isilon
2017-03-04 11:33:01 +00:00
asomers
f1c6fc5f44 Remove dead code in dhclient(8)
The offending code has been dead ever since the import from OpenBSD in
r195805.  OpenBSD later deleted that entire function.

Reported by:	Coverity
CID:		500059
MFC after:	4 weeks
Sponsored by:	Spectra Logic Corp
2017-01-04 18:13:05 +00:00
brooks
ba54a678f6 Allocate a struct ifreq rather than using a (wrong) computed size for
the BIOCSETIF ioctl.

The kernel always copies an entire struct ifreq and IPv4 addresses will
always fit in an ifreq.

On systems with pointers larger than 64-bits, the computed size will be
less than the size of struct ifreq, potentially resulting in the kernel
attempting to copyin memory from outside the allocation.

Reviewed by:	jhb
Obtained from:	CheriBSD
MFC after:	1 week
Sponsored by:	DARPA, AFRL
Differential Revision:	https://reviews.freebsd.org/D8445
2016-11-22 22:45:15 +00:00
cem
df343ad2c2 dhclient(8): Enable numbered user class ID option
By adding it to the option priorities table.

PR:		184117
Submitted by:	Lowell Gilbert <freebsd-bugs-local at be-well.ilk.org>
Reported by:	Tomek CEDRO <cederom at tlen.pl>
Reviewed by:	jhb
Differential Revision:	https://reviews.freebsd.org/D7911
2016-09-20 19:21:41 +00:00
cem
413d779950 dhclient: add support for interface-mtu (26)
Make dhclient set interface MTU if it was provided.

This version implements MTU setting in dhclient itself before it runs
dhclient-script.

PR:		206721
Submitted by:	novel@
Reported by:	Jarrod Petz <jlpetz at gmail.com>
Reviewed by:	cem, allanjude
Differential Revision:	https://reviews.freebsd.org/D5675
2016-09-02 21:14:29 +00:00
pfg
bd6fcb16e5 dhclient(1): correct obvious mismatch in get_char().
Correct switch between current and previous line buffers when
encountering a carriage return in the input.

CID:		1305719
Obtained from:	OpenBSD (CVS rev. 1.30)
MFC after:	3 days
2016-06-03 03:40:39 +00:00
cem
16b93d1013 dhclient: Fix the trivial buffer overruns correctly
A DHCP client identifier is simply the hardware type (one byte) concatenated
with the hardware address (some variable number of bytes, but at most 16).
Limit the size of the temporary buffer to match and the rest of the
calculations shake out correctly.

This is a follow-up to the incorrect r299512, reverted in r300172.

CIDs:		1008682, 1305550
Sponsored by:	EMC / Isilon Storage Division
2016-05-18 23:41:55 +00:00
cem
35a390d5d6 Revert r299512
It broke client identifiers because I misunderstood the intent of the code.
There is still a minor issue detected by Coverity (at least, I can't find where
the code proves it isn't an issue).  I'll follow up with a better fix for the
CIDs.

Reported by:	Ian FREISLICH
Sponsored by:	EMC / Isilon Storage Division
2016-05-18 23:35:37 +00:00
cem
7403ada3a6 dhclient: Fix some trivial buffer overruns
There was some confusion about how to limit a hardware address to at most 16
bytes.  In some cases it would overrun a byte off the end of the array.
Correct the types and rectify the overrun.

Reported by:	Coverity
CIDs:		1008682, 1305550
Sponsored by:	EMC / Isilon Storage Division
2016-05-12 04:28:22 +00:00
ngie
92100036c8 Merge ^/user/ngie/release-pkg-fix-tests to unbreak how test files are installed
after r298107

Summary of changes:

- Replace all instances of FILES/TESTS with ${PACKAGE}FILES. This ensures that
  namespacing is kept with FILES appropriately, and that this shouldn't need
  to be repeated if the namespace changes -- only the definition of PACKAGE
  needs to be changed
- Allow PACKAGE to be overridden by callers instead of forcing it to always be
  `tests`. In the event we get to the point where things can be split up
  enough in the base system, it would make more sense to group the tests
  with the blocks they're a part of, e.g. byacc with byacc-tests, etc
- Remove PACKAGE definitions where possible, i.e. where FILES wasn't used
  previously.
- Remove unnecessary TESTSPACKAGE definitions; this has been elided into
  bsd.tests.mk
- Remove unnecessary BINDIRs used previously with ${PACKAGE}FILES;
  ${PACKAGE}FILESDIR is now automatically defined in bsd.test.mk.
- Fix installation of files under data/ subdirectories in lib/libc/tests/hash
  and lib/libc/tests/net/getaddrinfo
- Remove unnecessary .include <bsd.own.mk>s (some opportunistic cleanup)

Document the proposed changes in share/examples/tests/tests/... via examples
so it's clear that ${PACKAGES}FILES is the suggested way forward in terms of
replacing FILES. share/mk/bsd.README didn't seem like the appropriate method
of communicating that info.

MFC after: never probably
X-MFC with: r298107
PR: 209114
Relnotes: yes
Tested with: buildworld, installworld, checkworld; buildworld, packageworld
Sponsored by: EMC / Isilon Storage Division
2016-05-04 23:20:53 +00:00
sephe
453693ecc0 dhclient: Log a warning instead of bailing upon "illegal" options
In Azure, the DHCP servers add private option (id 0xf5), which contains
binary form of an IPv4 address. Once this option is converted to string
form, it could contain '$', e.g.

IPv4 address: 100.72.36.54
binary form: 0x64 0x48 0x24 0x36
string form: "dH$6"

dhclient bails upon "illegal" options like the above example, thus the
VM bring-up will fail.

Also as a side note, this "illegal" option detection was added in
OpenBSD ~11years ago:
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sbin/dhclient/dhclient.c?rev=1.50&content-type=text/x-cvsweb-markup

And it was removed along with the removal of script support in OpenBSD
~3years ago:
http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sbin/dhclient/dhclient.c?rev=1.159&content-type=text/x-cvsweb-markup

Reported by:	Hongxiong Xian <v-hoxian microsoft com>
Reviewed by:	jhb, Dexuan Cui <decui microsoft com>
Tested by:	Hongxiong Xian <v-hoxian microsoft com>
Analyzed by:	Dong Liu <doliu microsoft com>
MFC after:	1 week
Sponsored by:	Microsoft OSTC
Differential Revision:	https://reviews.freebsd.org/D5853
2016-04-20 23:56:25 +00:00
gjb
1c7e318a9a MFH
Sponsored by:	The FreeBSD Foundation
2016-03-10 21:16:01 +00:00
bdrewery
aab40fdc3d DIRDEPS_BUILD: Connect MK_TESTS.
Sponsored by:	EMC / Isilon Storage Division
2016-03-09 22:46:01 +00:00
gjb
4e38aaedb8 Explicitly add more files to the 'runtime' package.
Sponsored by:	The FreeBSD Foundation
2016-02-09 20:19:31 +00:00
gjb
a6998ad84f First pass to fix the 'tests' packages.
Sponsored by:	The FreeBSD Foundation
2016-02-02 22:26:49 +00:00
bdrewery
bb97a3d43c Fix LDADD/DPADD that should be LIBADD.
Sponsored by:	EMC / Isilon Storage Division
2015-12-04 03:17:47 +00:00