Skip to content
This repository has been archived by the owner on Jan 5, 2024. It is now read-only.

nftlb leaks the key length to the process table #19

Open
ghost opened this issue Jun 24, 2020 · 4 comments
Open

nftlb leaks the key length to the process table #19

ghost opened this issue Jun 24, 2020 · 4 comments

Comments

@ghost
Copy link

ghost commented Jun 24, 2020

Hi. Here is an example of what I mean:

# pgrep -a nftlb
13111 /usr/sbin/nftlb -c /var/lib/nftlb/config.json -l 7 -H 127.0.0.1 -P 5555 -k                      no

Note that there are exactly 22 spaces between -k and the next argument. This reveals that my key contains exactly 20 characters to anyone with unfettered access to the process table. Knowing the key length would confer an advantage to any local attacker that might attempt to brute force the password.

In order to hide the key length, the only realistic option that I have is to take advantage of the hidepid mount that the proc filesystem supports. For me, that is not a problem, but it is not a suitable arrangement for all users. Besides, the whitespace is aesthetically displeasing.

It would be preferable for the argument containing the key not to be padded out in this fashion, if it can be avoided. Moreover, it might be useful to have an option that allows for the key to be read from a file, or even a given file descriptor. To put this into perspective, here is how nftlb is launched by my current initscript:

start-stop-daemon -b -m -p "$pidfile" -2 "$errfile" -- "$command" \
	-c "$NFTLB_CONFIG" \
	-l "$NFTLB_LOGLEVEL" \
	-H "$NFTLB_HOST" \
	-P "$NFTLB_PORT" \
	-k "$NFTLB_KEY" \
	$NFTLB_IPV6

If there were a --keyfile option, say, then I could potentially do this:

--keyfile <(printf %s "$NFTLB_KEY")

However, this would require bash, which is a problem for some environments - not least, Gentoo's OpenRC. For more flexibility, a --key-fd option might be useful. Imagine being able to do the following …

# Read from a key file
nftlb --key-fd 3 ... 3</path/to/keyfile

# Read the key from STDIN
printf %s "$NFTLB_KEY" | nftlb --key-fd 0 ...

What do you think?

@nevola
Copy link
Contributor

nevola commented Jul 2, 2020

Hi @kerframil

In the case that anyone with unfettered access would have a better way to gather the key by intercepting the plain HTTP socket content.

In any case, if you believe that reading the key from a file is more secure, we can include that quite easily without having to manage with bash scripting.

Cheers.

@ghost
Copy link
Author

ghost commented Jul 2, 2020

Hi @nevola

The point is that, on a typical GNU/Linux system, anyone can read the process table. Although the password argument is obfuscated, it continues to act as an oracle. Further, though I have not tried, it might be possible to win a race against nftlb and gather the exact value of the argument from the process table before it is altered. There are peripheral risks, such as accidental leakage of secrets into the (not necessarily secured) history of an interactive shell session.

As you say, HTTP is insecure. However, to sniff locally generated packets would require CAP_NET_RAW and/or CAP_NET_ADMIN. These are privileges that non-root users do not implicitly have. What I do is bind nftlb to the loopback interface and use other methods to remotely interact with nftlb, thus limiting its exposure in general.

I do not mean to suggest that this issue is a matter of urgency. Indeed, it does not affect me because I am fortunate enough to be able to use hidepid=2 without any side effects. Nevertheless, it would be useful to be able to gather the credentials by alternate means in the future, even if it is not a priority right now.

@nevola
Copy link
Contributor

nevola commented Jul 7, 2020

I do not mean to suggest that this issue is a matter of urgency. Indeed, it does not affect me because I am fortunate enough to be able to use hidepid=2 without any side effects. Nevertheless, it would be useful to be able to gather the credentials by alternate means in the future, even if it is not a priority right now.

Yes, sure. We can include it in the near future without concern. Thank you for the proposal.

@elico
Copy link

elico commented Aug 30, 2020

@nevola +1 for a config file. It's not a must but it's better in many real world scenarios.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants