Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/pins/tang/clevis-decrypt-tang
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,25 @@ if ! url="$(jose fmt -j- -Og clevis -g tang -g url -Su- <<< "$jhd")"; then
exit 1
fi

# Optional (m)TLS parameters. Absent for JWEs created without mTLS, in which
# case curl behaves exactly as before.
cacert="$(jose fmt -j- -Og clevis -g tang -g cacert -Su- <<< "$jhd")" || true

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During decryption, cacert, cert, key (and url) are all extracted from the JWE protected header and fed to curl (line 115) before the AEAD integrity check at line 128 (jose jwe dec). The url field was already consumed unauthenticated before this patch, so this is a pre-existing design property of clevis-tang. However, cacert qualitatively extends the attack surface: an attacker who can modify the stored JWE can now also override which CA curl trusts, enabling MITM against the legitimate tang server with a self-signed cert planted on disk. Previously, modifying url alone still required a certificate trusted by the system CA store.

The practical exploitability is limited (requires disk write access + network position, and the ECMR exchange value alone does not reveal the decryption key), so this may be acceptable as a known trade-off.

Worth considering: validate that cert/key/cacert paths are under an expected prefix (e.g., /etc/clevis/), or use indirection (store a profile name, resolve to paths at runtime from a fixed directory). At minimum, document the trust model explicitly so users understand the implication.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your review! In my understanding, if attacker could modify JWE header, they could probably also modify the ca trust store in /etc/pki. If this is correct, cacert does not add more security risk here (correct me if my understanding is wrong). I also thought about adding validation for cert/key/cacert paths, but considering this aims to be used generically, it is hard to add any specific cacert path validation so I feel path-prefix validation felt too restrictive here.
I added one section named "mTLS SECURITY MODEL" in clevis-encrypt-tang.1.adoc to describe the potential risk of cacert and recommende that the CA be installed in the system trust store (with cacert omitted) where possible

cert="$(jose fmt -j- -Og clevis -g tang -g cert -Su- <<< "$jhd")" || true
key="$(jose fmt -j- -Og clevis -g tang -g key -Su- <<< "$jhd")" || true

# Fail with a targeted diagnostic if a referenced (m)TLS file is missing.
# Otherwise curl -sfg fails silently below and the generic "Error
# communicating with server" sends the operator chasing network issues,
# when the real cause is a rotated cert or a file left out of the initramfs.
[ -n "$cacert" ] && [ ! -f "$cacert" ] && { echo "mTLS CA cert '$cacert' not found (is it in the initramfs?)" >&2; exit 1; }
[ -n "$cert" ] && [ ! -f "$cert" ] && { echo "mTLS client cert '$cert' not found (is it in the initramfs?)" >&2; exit 1; }
[ -n "$key" ] && [ ! -f "$key" ] && { echo "mTLS client key '$key' not found (is it in the initramfs?)" >&2; exit 1; }

curl_opts=()
[ -n "$cacert" ] && curl_opts+=(--cacert "$cacert")
[ -n "$cert" ] && curl_opts+=(--cert "$cert")
[ -n "$key" ] && curl_opts+=(--key "$key")

if ! crv="$(jose fmt -j- -Og crv -Su- <<< "$clt")"; then
echo "Unable to determine EPK's curve!" >&2
exit 1
Expand All @@ -101,7 +120,7 @@ xfr="$(jose jwk exc -i '{"alg":"ECMR"}' -l- -r- <<< "$clt$eph")"

rec_url="$url/rec/$kid"
ct="Content-Type: application/jwk+json"
if ! rep="$(curl -sfg -X POST -H "$ct" --data-binary @- "$rec_url" <<< "$xfr")"; then
if ! rep="$(curl -sfg "${curl_opts[@]}" -X POST -H "$ct" --data-binary @- "$rec_url" <<< "$xfr")"; then
echo "Error communicating with server $url" >&2
Comment thread
liyue32 marked this conversation as resolved.
exit 1
fi
Expand Down
39 changes: 38 additions & 1 deletion src/pins/tang/clevis-encrypt-tang
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ if [ -t 0 ]; then
echo " adv: <string> A filename containing a trusted advertisement"
echo " adv: <object> A trusted advertisement (raw JSON)"
echo
echo " cacert: <string> Path to a CA bundle used to verify the Tang"
echo " server's TLS certificate (curl --cacert)"
echo
echo " cert: <string> Path to the client TLS certificate for mutual"
echo " TLS authentication (curl --cert)"
echo
echo " key: <string> Path to the private key for the client TLS"
echo " certificate (curl --key). May be omitted if the"
echo " key is bundled in the 'cert' file"
echo
echo "Obtaining the thumbprint of a trusted signing key is easy. If you"
echo "have access to the Tang server's database directory, simply do:"
echo
Expand Down Expand Up @@ -77,6 +87,28 @@ fi

thp="$(jose fmt -j- -Og thp -Su- <<< "$cfg")" || true

# Optional (m)TLS parameters. If none are set, curl behaves exactly as before
# (plain HTTP/TLS, no client certificate and default CA verification).
cacert="$(jose fmt -j- -Og cacert -Su- <<< "$cfg")" || true
cert="$(jose fmt -j- -Og cert -Su- <<< "$cfg")" || true
key="$(jose fmt -j- -Og key -Su- <<< "$cfg")" || true
Comment thread
liyue32 marked this conversation as resolved.

# Validate the (m)TLS file paths up front, matching the 'adv' check below.
# Otherwise a typo'd path either surfaces as a misleading "Unable to fetch
# advertisement" error, or (when 'adv' is inline and curl is never called)
# gets baked into the JWE header and only fails later at decrypt time.
[ -n "$cacert" ] && [ ! -f "$cacert" ] && { echo "CA cert file '$cacert' not found!" >&2; exit 1; }
[ -n "$cert" ] && [ ! -f "$cert" ] && { echo "Client cert file '$cert' not found!" >&2; exit 1; }
[ -n "$key" ] && [ ! -f "$key" ] && { echo "Client key file '$key' not found!" >&2; exit 1; }

# 'key' on its own is a no-op: curl ignores --key unless --cert is also given.
[ -n "$key" ] && [ -z "$cert" ] && { echo "'key' requires 'cert' to also be set!" >&2; exit 1; }

curl_opts=()
[ -n "$cacert" ] && curl_opts+=(--cacert "$cacert")
[ -n "$cert" ] && curl_opts+=(--cert "$cert")
[ -n "$key" ] && curl_opts+=(--key "$key")
Comment thread
liyue32 marked this conversation as resolved.

### Get the advertisement
if jws="$(jose fmt -j- -g adv -Oo- <<< "$cfg")"; then
thp="${thp:-any}"
Expand All @@ -92,7 +124,7 @@ elif jws="$(jose fmt -j- -g adv -Su- <<< "$cfg")"; then
fi

thp="${thp:-any}"
elif ! jws="$(curl -sfg "$url/adv/$thp")"; then
elif ! jws="$(curl -sfg "${curl_opts[@]}" "$url/adv/$thp")"; then
echo "Unable to fetch advertisement: '$url/adv/$thp'!" >&2
exit 1
fi
Expand Down Expand Up @@ -156,5 +188,10 @@ kid="$(jose jwk thp -i- -a "${CLEVIS_DEFAULT_THP_ALG}" <<< "$jwk")"
jwe='{"protected":{"alg":"ECDH-ES","enc":"A256GCM","clevis":{"pin":"tang","tang":{}}}}'
jwe="$(jose fmt -j "$jwe" -g protected -q "$kid" -s kid -UUo-)"
jwe="$(jose fmt -j "$jwe" -g protected -g clevis -g tang -q "$url" -s url -UUUUo-)"
# Persist any (m)TLS parameters so decryption can reuse them. These are file
# paths (not secrets); the JWE header is stored in plaintext.
[ -n "$cacert" ] && jwe="$(jose fmt -j "$jwe" -g protected -g clevis -g tang -q "$cacert" -s cacert -UUUUo-)"
[ -n "$cert" ] && jwe="$(jose fmt -j "$jwe" -g protected -g clevis -g tang -q "$cert" -s cert -UUUUo-)"
[ -n "$key" ] && jwe="$(jose fmt -j "$jwe" -g protected -g clevis -g tang -q "$key" -s key -UUUUo-)"
jwe="$(jose fmt -j "$jwe" -g protected -g clevis -g tang -j- -s adv -UUUUo- <<< "$jwks")"
exec jose jwe enc -i- -k- -I- -c < <(echo -n "$jwe$jwk"; /bin/cat)
44 changes: 44 additions & 0 deletions src/pins/tang/clevis-encrypt-tang.1.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,50 @@ This command uses the following configuration properties:
* *adv* (object) :
A trusted advertisement (raw JSON)

* *cacert* (string) :
Path to a CA bundle used to verify the Tang server's TLS certificate
(passed to curl as --cacert). Only needed when the server certificate is
signed by a CA that is not in the system trust store.

* *cert* (string) :
Path to the client TLS certificate used for mutual TLS (mTLS)
authentication (passed to curl as --cert).

* *key* (string) :
Path to the private key for the client TLS certificate (passed to curl as
--key). May be omitted if the private key is bundled in the *cert* file.

The *cacert*, *cert* and *key* properties are optional. When none are set,
Clevis talks to the Tang server exactly as before (plain HTTP/HTTPS with
default CA verification). These properties are file paths, not secrets, and
are stored in the JWE header so that decryption reuses them automatically.

NOTE: When used for automatic unlocking at boot (for example with LUKS), the
referenced certificate and key files must also be present at the same paths
inside the initramfs, since decryption happens there.

== mTLS EXAMPLE

$ cfg='{"url":"https://tang.srv","cacert":"/etc/clevis/ca.crt",
"cert":"/etc/clevis/client.crt","key":"/etc/clevis/client.key"}'
$ clevis encrypt tang "$cfg" < PT > JWE

== mTLS SECURITY MODEL

The *cacert*, *cert* and *key* properties are stored as plaintext in the JWE
header without authentication, so there is a chance they could be modified.
In particular, if an attacker changes *cacert* to a CA they control, curl will
trust a certificate signed by that CA, which lets a rogue server impersonate
the Tang server. Exploiting this requires the attacker to have write
access to the storage holding the JWE *and* a network position to impersonate
the Tang server.

To keep the trust anchor under administrator control, you may prefer installing
the Tang server's CA into the system trust store (for example with
*update-ca-trust* or *update-ca-certificates*) and omitting *cacert*. Store any
*cacert*, *cert* and *key* files referenced by the JWE in a root-owned location
that is not writable by untrusted users.

== OPTIONS

* *-y* :
Expand Down
1 change: 1 addition & 0 deletions src/pins/tang/tests/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ env.prepend('PATH',
)

test('pin-tang', find_program('pin-tang'), env: env)
test('pin-tang-mtls', find_program('pin-tang-mtls'), env: env)
test('tang-validate-adv', find_program('tang-validate-adv'), env: env)
test('default-thp-alg', find_program('default-thp-alg'), env: env)
81 changes: 81 additions & 0 deletions src/pins/tang/tests/pin-tang-mtls
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
#!/bin/bash -xe
# vim: set tabstop=8 shiftwidth=4 softtabstop=4 expandtab smarttab colorcolumn=80:
#
# Copyright (c) Meta Platforms, Inc. and affiliates.
#
# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
#

. tang-common-test-functions

on_exit() {
exit_status=$?
[ -n "${TMP}" ] && tang_stop "${TMP}"
[ -d "${TMP}" ] && rm -rf "$TMP"
exit "${exit_status}"
}
Comment thread
liyue32 marked this conversation as resolved.

trap 'on_exit' EXIT

# Bail out early (as a skip) if the environment cannot do mTLS.
tang_mtls_sanity_check

TMP="$(mktemp -d)"

# Generate CA, server and client certificates.
tang_generate_certs "${TMP}"

# Start a TLS tang server that *requires* a client certificate (mutual TLS).
tang_run_mtls "${TMP}" sig exc
port=$(tang_get_port "${TMP}")

url="https://localhost:${port}"
cacert="${TMP}/ca.crt"
cert="${TMP}/client.crt"
key="${TMP}/client.key"

# 1. Positive case: with a valid client certificate, encryption (which fetches
# the advertisement via GET /adv) and decryption (POST /rec) must both work
# over mutual TLS. '-y' skips the interactive trust check.
cfg="$(printf '{"url":"%s","cacert":"%s","cert":"%s","key":"%s"}' \
"$url" "$cacert" "$cert" "$key")"
enc="$(echo -n "hi" | clevis encrypt tang "$cfg" -y)"
dec="$(echo -n "$enc" | clevis decrypt)"
test "$dec" == "hi"

# 2. The (m)TLS parameters must have been persisted into the JWE protected
# header so that decryption can reuse them without any extra configuration.
hdr="$(echo -n "${enc}" | cut -d. -f1)"
jhd="$(jose b64 dec -i- <<< "${hdr}")"
test "$(jose fmt -j- -Og clevis -g tang -g cacert -Su- <<< "$jhd")" == "${cacert}"
test "$(jose fmt -j- -Og clevis -g tang -g cert -Su- <<< "$jhd")" == "${cert}"
test "$(jose fmt -j- -Og clevis -g tang -g key -Su- <<< "$jhd")" == "${key}"

# 3. Negative case: mutual TLS must actually be enforced. Without a client
# certificate the TLS handshake fails, so fetching the advertisement (and
# therefore encryption) must fail.
cfg_noclient="$(printf '{"url":"%s","cacert":"%s"}' "$url" "$cacert")"
! echo -n "hi" | clevis encrypt tang "${cfg_noclient}" -y

# 4. Negative case: 'key' without 'cert' is a no-op for curl, so encryption
# must be rejected up front rather than silently dropping the client key.
cfg_keynocert="$(printf '{"url":"%s","cacert":"%s","key":"%s"}' \
"$url" "$cacert" "$key")"
! echo -n "hi" | clevis encrypt tang "${cfg_keynocert}" -y

# 5. Negative case: a referenced (m)TLS file that does not exist must be
# reported at encrypt time instead of being baked into the JWE.
cfg_badcert="$(printf '{"url":"%s","cacert":"%s","cert":"%s","key":"%s"}' \
"$url" "$cacert" "${TMP}/does-not-exist.crt" "$key")"
! echo -n "hi" | clevis encrypt tang "${cfg_badcert}" -y
86 changes: 86 additions & 0 deletions src/pins/tang/tests/tang-common-test-functions.in
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,89 @@ run_test_server() {
process_wait_until_port_ready "${pid}"
process_find_port "${pid}" "tang" > "${portfile}"
}

# Extra sanity check for the mutual-TLS (mTLS) tests: on top of the regular
# tang requirements, we need the 'openssl' command and a socat built with
# OpenSSL support (for the OPENSSL-LISTEN address type).
tang_mtls_sanity_check() {
tang_sanity_check
command -v openssl >/dev/null 2>&1 \
|| skip_test "mTLS test needs the 'openssl' command"
"${SOCAT}" -V 2>/dev/null | grep -q -i 'openssl' \
|| skip_test "mTLS test needs socat built with OpenSSL support"
return 0
}

# Generate a CA plus server and client certificates for the mTLS tests.
# Creates, under ${basedir}:
# ca.crt - CA certificate (curl --cacert / socat cafile=)
# server.pem - server cert+key (socat cert=)
# client.crt - client certificate (curl --cert)
# client.key - client private key (curl --key)
tang_generate_certs() {
local basedir="${1}"
[ -z "${basedir}" ] && error "tang_generate_certs: please specify 'basedir'"

local ca_key="${basedir}/ca.key"
local ca_crt="${basedir}/ca.crt"
local srv_key="${basedir}/server.key"
local srv_crt="${basedir}/server.crt"
local srv_csr="${basedir}/server.csr"
local srv_pem="${basedir}/server.pem"
local cli_key="${basedir}/client.key"
local cli_crt="${basedir}/client.crt"
local cli_csr="${basedir}/client.csr"

# Self-signed CA.
openssl req -x509 -newkey rsa:2048 -nodes -keyout "${ca_key}" \
-out "${ca_crt}" -subj "/CN=clevis-test-ca" -days 1

# Server certificate signed by the CA. The subjectAltName must include
# 'localhost' or curl's hostname verification would reject it.
openssl req -newkey rsa:2048 -nodes -keyout "${srv_key}" \
-out "${srv_csr}" -subj "/CN=localhost"
openssl x509 -req -in "${srv_csr}" -CA "${ca_crt}" -CAkey "${ca_key}" \
-CAcreateserial -out "${srv_crt}" -days 1 \
-extfile <(printf 'subjectAltName=DNS:localhost,IP:127.0.0.1')
cat "${srv_crt}" "${srv_key}" > "${srv_pem}"

# Client certificate signed by the same CA.
openssl req -newkey rsa:2048 -nodes -keyout "${cli_key}" \
-out "${cli_csr}" -subj "/CN=clevis-test-client"
openssl x509 -req -in "${cli_csr}" -CA "${ca_crt}" -CAkey "${ca_key}" \
-CAcreateserial -out "${cli_crt}" -days 1

return 0
}

# Start a test tang server fronted by TLS with mandatory client-certificate
# verification, i.e. mutual TLS. Requires tang_generate_certs to have been
# run against the same basedir first.
tang_run_mtls() {
tang_mtls_sanity_check
local basedir="${1}"
local sig_name="${2:-}"
local exc_name="${3:-}"

[ -z "${basedir}" ] && error "tang_run_mtls: please specify 'basedir'"

if ! tang_new_keys "${basedir}" "" "${sig_name}" "${exc_name}"; then
error "Error creating new keys for tang server"
fi

local KEYS="${basedir}/db"
local srv_pem="${basedir}/server.pem"
local ca_crt="${basedir}/ca.crt"

local pid pidfile portfile
pidfile="${basedir}/tang.pid"
portfile="${basedir}/tang.port"

"${SOCAT}" OPENSSL-LISTEN:0,fork,reuseaddr,cert="${srv_pem}",cafile="${ca_crt}",verify=1 \
exec:"${TANGD} ${KEYS}" &
pid=$!

echo "${pid}" > "${pidfile}"
process_wait_until_port_ready "${pid}"
process_find_port "${pid}" "tang" > "${portfile}"
}
Loading