On Tue, Aug 6, 2024 at 2:41 AM Heikki Linnakangas <hlinn...@iki.fi> wrote:
> Seems that on linux or freebsd, you'd plow ahead even if the binary is
> not found, and fail later, while on macOS you'd skip the tests. I think
> we should always error out if the dependencies are not found. If you
> make an effort to add PG_TEST_EXTRA=radius, presumably you really do
> want to run the tests, and if dependencies are missing you'd like to
> know about it.
Fixed.
> > + secret = "shared-secret"
>
> Let's use a random value for this, and for the Cleartext-Password. This
> only runs locally, and only if you explicitly add it to PG_TEST_EXTRA,
> but it still seems nice to protect from other users on the system when
> we can do so easily.
OK, done.
> > +security {
> > + require_message_authenticator = "yes"
> > +}
>
> > +# Note that require_message_authenticator defaulted to "no" before 3.2.5,
> > and
> > +# then switched to "auto" (a new mode that fills the logs up with warning
> > +# messages about clients that don't send MA), and presumably a later
> > version
> > +# will default to "yes".
>
> That's not quite accurate: the option didn't exist before version 3.2.5.
> What happens if you set it on an older server version? /me tests: seems
> that FreeRadius 3.2.1 silently accepts the setting, or any other setting
> it doesn't recognize, and will do nothing with it. A little surprising,
> but ok. I didn't find any mention in the docs on that.
Huh. Thanks, I was confused by that. Fixed.
> (Also, that will make the test fail, until the
> v1-0003-Mitigation-for-BlastRADIUS.patch is also applied. You could
> leave that out of the test in this first patch, and add it
> v1-0003-Mitigation-for-BlastRADIUS.patch)
Yeah, done.
> Review on v1-0003-Mitigation-for-BlastRADIUS.patch:
>
> > + <varlistentry>
> > + <term><literal>radiusrequirema</literal></term>
> > + <listitem>
> > + <para>
> > + Whether to require a valid
> > <literal>Message-Authenticator</literal>
> > + attribute in messages received from RADIUS servers, and ignore
> > messages
> > + that don't contain it. The default value
> > + is <literal>0</literal>, but it can be set to <literal>1</literal>
> > + to enable that requirement.
> > + This setting does not affect requests sent by
> > + <productname>PostgreSQL</productname> to the RADIUS server, which
> > + always include a <literal>Message-Authenticator</literal>
> > attribute
> > + (but didn't in earlier releases).
> > + </para>
> > + </listitem>
> > + </varlistentry>
>
> I think this should include some advise on why and when you should set
> it. Something like:
>
> Enabling this mitigates the RADIUS protocol vulnerability described at
> blastradius.fail. It is recommended to always set this to 1, unless you
> are running an older RADIUS server version that does include the
> mitigation to include Message-Authenticator in all replies. The default
> will be changed to 1 in a future PostgreSQL version.
Done, with small tweaks.
> > attr = (radius_attribute *) (packet + RADIUS_HEADER_LENGTH);
> >
> > while ((uint8 *) attr < end)
> > {
> > /* Would this attribute overflow the buffer? */
> > if (&attr->length >= end || (uint8 *) attr + attr->length >
> > end)
> > return false;
>
> Is this kind of pointer arithmetic safe? In theory, if a packet is
> malformed so that attr->length points beyond the end of the packet,
> "(uint8 *) attr + attr-length" might overflow. That would require the
> packet buffer to be allocated just before the end of the address space,
> so probably cannot happen in practice. I don't remember if there are
> some guarantees on that in C99 or other standards. Still, perhaps it
> would be better to write this differently, e.g. using a separate "size_t
> position" variable to track the current position in the buffer.
Done.
> (This also relies on the fact that "struct radius_attribute" doesn't
> require alignment, which is valid, and radius_add_attribute() made that
> assumption already. Maybe worth a comment though while we're at it; it
> certainly raised my eyebrow while reading this)
Comment added.
> What if the message contains multiple attribute of the same type? If
> there's a duplicate Message-Authenticator, we should surely reject the
> packet. I don't know if duplicate attributes are legal in general.
Duplicate attributes are legal in general per RFC 2865, which has a
table of attributes and their possible quantity; unfortunately this
one is an extension from RFC 2869, and I didn't find where it pins
that down. I suppose we could try to detect an unexpected duplicate,
which might have the side benefit of checking the rest of the
attributes for well-formedness (though in practice there aren't any).
Is it worth bothering with that?
I suppose if we wanted to be extra fastidious, we could also test with
a gallery of malformed packets crafted by a Perl script, but that
feels like overkill. On the other hand it would be bad if you could
crash a server by lobbing UDP packets at it because of some dumb
thinko.
> > + /*
> > + * Add Message-Authenticator attribute first, which for now holds
> > zeroes.
> > + * We remember where it is in the message so that we can fill it in
> > later.
> > + */
>
> Let's mention Blast-RADIUS here as the reason to put this first. Reading
> the paper though, I think it's only important in the server->client
> messages, but I'm not sure, and shouldn't hurt anyway.
Done.
> > + else if (message_authenticator_location == NULL)
> > + {
> > + ereport(LOG,
> > + (errmsg("RADIUS response from %s has
> > no Message-Authenticator",
> > + server)));
> > +
> > + /* We'll ignore this message, unless pg_hba.conf told
> > us not to. */
> > + if (requirema)
> > + continue;
> > + }
>
> This is going to be very noisy if you are running an older server.
Silenced.
> > + uint8 message_authenticator_key[RADIUS_VECTOR_LENGTH];
> > + uint8 message_authenticator[RADIUS_VECTOR_LENGTH];
>
> Perhaps use MD5_DIGEST_LENGTH for these. The Message-Authenticator is an
> HMAC-MD5, which indeed has the same length as the MD5 hash used on the
> password, so it's just pro forma, but it seems a little coincidental.
> There's no fundamental reason they would have to be the same length, if
> the RFC author's had chosen to use a different hash algorithm for
> Message-Authenticator, for example.
The first one is now gone (see next).
Now I have message_authenticator[MD5_DIGEST_LENGTH], and then the
other places that need that number use
lengthof(message_authenticator).
> If the secret is longer than 16 bytes, this truncates it. Is that
> correct? According to https://en.wikipedia.org/wiki/HMAC, you're
> supposed derive the suitably-sized key by calling the hash function on
> the longer key in that case.
Oh, actually I don't think we need that step at all: the HMAC init
function takes a variable length key and does the required
padding/hashing itself.
> # v1-0004-Teach-007_radius-test-about-Message-Authenticator.patch
>
> Looks good to me, although I'm not sure it's worthwhile to do this.
> We're not reaching the codepath where we'd reject a message because of a
> missing Message-Authenticator anyway. If the radiusrequirema option was
> broken and had no effect, for example, the test would still pass.
Dropped.
> # v1-0005-XXX-BlastRADIUS-back-patch-kludge-for-12-and-13.patch
>
> Perhaps throw an error if you set "radiusrequirema=1", but the server is
> compiled without OpenSSL.
Done.
I don't think I would turn this on in the build farm, because of the 3
second timeout which might cause noise. Elsewhere I had a patch to
make the timeout configurable, so it could be set long for positive
tests and short for negative tests, so we could maybe do that in
master and think about turning the test on somewhere.
From 2435882d82cc91f0b4ac73bf6149fbacf08a547c Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Sat, 31 Dec 2022 14:41:57 +1300
Subject: [PATCH v2 1/4] Add simple test for RADIUS authentication.
This is similar to the existing tests for other authentication methods.
It requires FreeRADIUS to be installed, and opens ports that may be
considered insecure, so users have to opt in with PG_EXTRA_TESTS=radius.
Reviewed-by: Heikki Linnakangas <hlinn...@iki.fi>
Reviewed-by: Michael Paquier <mich...@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGLRSPTOC_ygx4_sJjWeKOkOpWGCBCJiRq8cPNuMisuzgw%40mail.gmail.com
---
doc/src/sgml/regress.sgml | 11 ++
src/test/authentication/README | 12 ++
src/test/authentication/meson.build | 1 +
src/test/authentication/t/007_radius.pl | 187 ++++++++++++++++++++++++
4 files changed, 211 insertions(+)
create mode 100644 src/test/authentication/t/007_radius.pl
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index d1042e02228..b9dc28b8dea 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -284,6 +284,17 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption'
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>radius</literal></term>
+ <listitem>
+ <para>
+ Runs the test <filename>src/test/authentication/t/007_radius.pl</filename>.
+ This requires a <productname>FreeRADIUS</productname> installation and
+ opens UDP listen sockets.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>ssl</literal></term>
<listitem>
diff --git a/src/test/authentication/README b/src/test/authentication/README
index 602280a0713..13d7a7f4d84 100644
--- a/src/test/authentication/README
+++ b/src/test/authentication/README
@@ -26,3 +26,15 @@ Either way, this test initializes, starts, and stops a test Postgres
cluster.
See src/test/perl/README for more info about running these tests.
+
+Requirements
+============
+
+The RADIUS test is skipped unless "radius" is listed in PG_TEXT_EXTRA, because
+it requires a FreeRADIUS installation and opens extra ports. FreeRADIUS can be
+installed with:
+
+Debian: apt-get install freeradius
+Homebrew: brew install freeradius-server
+FreeBSD: pkg install freeradius3
+MacPorts: port install freeradius
diff --git a/src/test/authentication/meson.build b/src/test/authentication/meson.build
index 8f5688dcc13..59da3e64169 100644
--- a/src/test/authentication/meson.build
+++ b/src/test/authentication/meson.build
@@ -12,6 +12,7 @@ tests += {
't/004_file_inclusion.pl',
't/005_sspi.pl',
't/006_login_trigger.pl',
+ 't/007_radius.pl',
],
},
}
diff --git a/src/test/authentication/t/007_radius.pl b/src/test/authentication/t/007_radius.pl
new file mode 100644
index 00000000000..c683d1d5579
--- /dev/null
+++ b/src/test/authentication/t/007_radius.pl
@@ -0,0 +1,187 @@
+
+# Copyright (c) 2021-2024, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Utils;
+use PostgreSQL::Test::Cluster;
+use Test::More;
+
+my $radiusd_dir = "${PostgreSQL::Test::Utils::tmp_check}/radiusd_data";
+my $radiusd_conf = "radiusd.conf";
+my $radiusd_users = "users.txt";
+my $radiusd_prefix;
+my $radiusd;
+
+if ($ENV{PG_TEST_EXTRA} !~ /\bradius\b/)
+{
+ plan skip_all => 'radius not enabled in PG_TEST_EXTRA';
+}
+elsif ($^O eq 'freebsd')
+{
+ $radiusd = '/usr/local/sbin/radiusd';
+}
+elsif ($^O eq 'linux' && -f '/usr/sbin/freeradius')
+{
+ $radiusd = '/usr/sbin/freeradius';
+}
+elsif ($^O eq 'linux')
+{
+ $radiusd = '/usr/sbin/radiusd';
+}
+elsif ($^O eq 'darwin' && -d '/opt/local')
+{
+ # typical path for MacPorts
+ $radiusd = '/opt/local/sbin/radiusd';
+ $radiusd_prefix = '/opt/local';
+}
+elsif ($^O eq 'darwin' && -d '/opt/homebrew')
+{
+ # typical path for Homebrew on ARM
+ $radiusd = '/opt/homebrew/bin/radiusd';
+ $radiusd_prefix = '/opt/homebrew';
+}
+elsif ($^O eq 'darwin' && -d '/usr/local')
+{
+ # typical path for Homebrew on Intel
+ $radiusd = '/usr/local/bin/radiusd';
+ $radiusd_prefix = '/usr/local';
+}
+
+if (!defined($radiusd) || ! -f $radiusd)
+{
+ plan skip_all =>
+ "radius tests not supported on $^O or dependencies not installed";
+}
+
+sub make_random_string
+{
+ my $length = 8 + int(rand(16));
+ my $s = "";
+ $s .= chr(ord("A") + rand(26)) for 1..$length;
+ return $s;
+}
+
+my $shared_secret = make_random_string();
+my $password = make_random_string();
+
+note "setting up radiusd";
+
+my $radius_port = PostgreSQL::Test::Cluster::get_free_port();
+
+mkdir $radiusd_dir or die "cannot create $radiusd_dir";
+
+append_to_file("$radiusd_dir/$radiusd_users",
+ qq{test2 Cleartext-Password := "$password"});
+
+my $conf = qq{
+client default {
+ ipaddr = "127.0.0.1"
+ secret = "$shared_secret"
+}
+
+modules {
+ files {
+ filename = "$radiusd_dir/users.txt"
+ }
+ pap {
+ }
+}
+
+server default {
+ listen {
+ type = "auth"
+ ipv4addr = "127.0.0.1"
+ port = "$radius_port"
+ }
+ authenticate {
+ Auth-Type PAP {
+ pap
+ }
+ }
+ authorize {
+ files
+ pap
+ }
+}
+
+log {
+ destination = "files"
+ localstatedir = "$radiusd_dir"
+ logdir = "$radiusd_dir"
+ file = "$radiusd_dir/radius.log"
+}
+
+pidfile = "$radiusd_dir/radiusd.pid"
+};
+
+if ($radiusd_prefix)
+{
+ $conf .= "prefix=\"$radiusd_prefix\"\n";
+}
+
+append_to_file("$radiusd_dir/$radiusd_conf", $conf);
+
+system_or_bail $radiusd, '-xx', '-d', $radiusd_dir;
+
+END
+{
+ kill 'INT', `cat $radiusd_dir/radiusd.pid`
+ if -f "$radiusd_dir/radiusd.pid";
+}
+
+note "setting up PostgreSQL instance";
+
+my $node = PostgreSQL::Test::Cluster->new('node');
+$node->init;
+$node->append_conf('postgresql.conf', "log_connections = on\n");
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE USER test1;');
+$node->safe_psql('postgres', 'CREATE USER test2;');
+
+unlink($node->data_dir . '/pg_hba.conf');
+$node->append_conf(
+ 'pg_hba.conf',
+ qq{
+local all test2 radius radiusservers="127.0.0.1" radiussecrets="$shared_secret" radiusports="$radius_port"
+}
+);
+$node->restart;
+
+note "running tests";
+
+sub test_access
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($node, $role, $expected_res, $test_name, %params) = @_;
+ my $connstr = "user=$role";
+
+ if ($expected_res eq 0)
+ {
+ $node->connect_ok($connstr, $test_name, %params);
+ }
+ else
+ {
+ # No checks of the error message, only the status code.
+ $node->connect_fails($connstr, $test_name, %params);
+ }
+}
+
+$ENV{"PGPASSWORD"} = 'wrong';
+test_access(
+ $node, 'test1', 2,
+ 'authentication fails if user not found in RADIUS',
+ log_unlike => [qr/connection authenticated:/]);
+test_access(
+ $node, 'test2', 2,
+ 'authentication fails with wrong password',
+ log_unlike => [qr/connection authenticated:/]);
+
+$ENV{"PGPASSWORD"} = $password;
+test_access($node, 'test2', 0, 'authentication succeeds with right password',
+ log_like =>
+ [qr/connection authenticated: identity="test2" method=radius/],);
+
+done_testing();
--
2.46.0
From 62d315c47563dcd771dc9a3034a0df7094587656 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Sat, 2 Sep 2023 10:09:47 +1200
Subject: [PATCH v2 2/4] ci: Enable RADIUS tests.
Unix targets only, because FreeRADIUS doesn't exist on Windows.
XXX Not sure it's worth running this stuff on CI. This is just for
demonstration.
---
.cirrus.tasks.yml | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 1ce6c443a8c..b9c37440777 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -20,7 +20,7 @@ env:
MTEST_ARGS: --print-errorlogs --no-rebuild -C build
PGCTLTIMEOUT: 120 # avoids spurious failures during parallel tests
TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
- PG_TEST_EXTRA: kerberos ldap ssl libpq_encryption load_balance
+ PG_TEST_EXTRA: kerberos ldap ssl libpq_encryption load_balance radius
# What files to preserve in case tests fail
@@ -164,7 +164,7 @@ task:
chown root:postgres /tmp/cores
sysctl kern.corefile='/tmp/cores/%N.%P.core'
setup_additional_packages_script: |
- #pkg install -y ...
+ pkg install -y freeradius3
# NB: Intentionally build without -Dllvm. The freebsd image size is already
# large enough to make VM startup slow, and even without llvm freebsd
@@ -313,8 +313,8 @@ task:
EOF
setup_additional_packages_script: |
- #apt-get update
- #DEBIAN_FRONTEND=noninteractive apt-get -y install ...
+ apt-get update
+ DEBIAN_FRONTEND=noninteractive apt-get -y install freeradius
matrix:
- name: Linux - Debian Bookworm - Autoconf
@@ -473,6 +473,7 @@ task:
setup_additional_packages_script: |
sh src/tools/ci/ci_macports_packages.sh \
ccache \
+ freeradius \
icu \
kerberos5 \
lz4 \
--
2.46.0
From fc225f1421e7838b480c0293a6b5dcac35375387 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Sun, 4 Aug 2024 15:58:33 +1200
Subject: [PATCH v2 3/4] Mitigation for BlastRADIUS.
Add an RFC 2869 Message-Authenticator attribute (an HMAC-MD5 signature)
to outgoing Access-Request messages, and optionally require and verify
them on incoming Access-Accept and Access-Reject messages, as
recommended to mitigate CVE-2024-3596 and VU#456537 by:
https://www.blastradius.fail/
No RADIUS server should be upset by the addition of our new
Message-Authenticator attribute in requests, so we always add that.
On the other hand, we can't require the attribute to be present in
responses yet because we can't assume that all sites are already doing
that. For example, FreeRADIUS started sending them in 3.2.5, which
isn't yet in all distributions at the time of writing. Therefore, users
have to opt in to this requirement with radiusrequirema=1 in pg_hba.conf
for now; the default could change in future.
Reviewed-by: Heikki Linnakangas <hlinn...@iki.fi>
Discussion: https://postgr.es/m/CA%2BhUKGLRSPTOC_ygx4_sJjWeKOkOpWGCBCJiRq8cPNuMisuzgw%40mail.gmail.com
---
doc/src/sgml/client-auth.sgml | 27 ++++
src/backend/libpq/auth.c | 205 +++++++++++++++++++++++-
src/backend/libpq/hba.c | 8 +
src/include/libpq/hba.h | 1 +
src/test/authentication/t/007_radius.pl | 5 +
5 files changed, 241 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 51343de7cad..37ee2833d37 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -2161,6 +2161,33 @@ host ... ldap ldapbasedn="dc=example,dc=net"
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>radiusrequirema</literal></term>
+ <listitem>
+ <para>
+ Whether to require a valid
+ <ulink url="https://datatracker.ietf.org/doc/html/rfc2869#section-5.14";>RFC 2869</ulink>
+ <literal>Message-Authenticator</literal>
+ attribute in messages received from RADIUS servers, and ignore messages
+ that don't contain it. The default value
+ is <literal>0</literal>, but it can be set to <literal>1</literal>.
+ This setting does not affect requests sent by
+ <productname>PostgreSQL</productname> to the RADIUS server, which
+ always include a <literal>Message-Authenticator</literal> attribute
+ (but didn't in earlier releases).
+ </para>
+ <para>
+ Two-way <literal>Message-Authenticator</literal> attributes are a
+ mitigation for the security vulnerability known as
+ <ulink url="https://blastradius.fail";>Blast-RADIUS</ulink>. It is
+ recommended to always set this to 1, unless you are running an older RADIUS
+ server version that has not been upgraded to include
+ <literal>Message-Authenticator</literal> in all replies. The default will be
+ changed to 1 in a future PostgreSQL version.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 2b607c52704..07dc1e93791 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -24,6 +24,7 @@
#include <unistd.h>
#include "commands/user.h"
+#include "common/hmac.h"
#include "common/ip.h"
#include "common/md5.h"
#include "libpq/auth.h"
@@ -198,7 +199,7 @@ static int pg_SSPI_make_upn(char *accountname,
*----------------------------------------------------------------
*/
static int CheckRADIUSAuth(Port *port);
-static int PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd);
+static int PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd, bool requirema);
/*
@@ -2802,6 +2803,7 @@ typedef struct
#define RADIUS_PASSWORD 2
#define RADIUS_SERVICE_TYPE 6
#define RADIUS_NAS_IDENTIFIER 32
+#define RADIUS_MESSAGE_AUTHENTICATOR 80
/* RADIUS service types */
#define RADIUS_AUTHENTICATE_ONLY 8
@@ -2809,7 +2811,7 @@ typedef struct
/* Seconds to wait - XXX: should be in a config variable! */
#define RADIUS_TIMEOUT 3
-static void
+static uint8 *
radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *data, int len)
{
radius_attribute *attr;
@@ -2825,7 +2827,7 @@ radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *dat
elog(WARNING,
"adding attribute code %d with length %d to radius packet would create oversize packet, ignoring",
type, len);
- return;
+ return NULL;
}
attr = (radius_attribute *) ((unsigned char *) packet + packet->length);
@@ -2833,6 +2835,65 @@ radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *dat
attr->length = len + 2; /* total size includes type and length */
memcpy(attr->data, data, len);
packet->length += attr->length;
+
+ /*
+ * Return pointer into message, used to fill in Message-Authenticator
+ * contents later.
+ */
+ return attr->data;
+}
+
+/*
+ * Search for an attribute in a received message. If the attribute is found,
+ * sets *value and *length (not including the header), and returns true. If
+ * the attribute is not found but the message appears to be well-formed, sets
+ * *value to NULL and returns true. If the message is found to be mal-formed,
+ * returns false.
+ */
+static bool
+radius_find_attribute(uint8 *packet,
+ size_t packet_size,
+ uint8 type,
+ uint8 **value,
+ uint8 *length)
+{
+ size_t index = RADIUS_HEADER_LENGTH;
+
+ while (index < packet_size)
+ {
+ radius_attribute *attr;
+
+ /* Are there enough bytes left for the attribute header? */
+ if (index + offsetof(radius_attribute, data) >= packet_size)
+ return false;
+
+ /* No alignment requirement, members are all uint8. */
+ attr = (radius_attribute *) &packet[index];
+
+ /* Would this attribute overflow the packet? */
+ if (index + attr->length > packet_size)
+ return false;
+
+ /* Is this attribute's length shorter than its own header? */
+ if (attr->length < offsetof(radius_attribute, data))
+ return false;
+
+ /* Is this it? */
+ if (attr->attribute == type)
+ {
+ *value = attr->data;
+ *length = attr->length - offsetof(radius_attribute, data);
+ return true;
+ }
+
+ /* Nope, skip to the next one. */
+ index += attr->length;
+ }
+
+ /* Well-formed, but not found. */
+ *value = NULL;
+ *length = 0;
+ return true;
}
static int
@@ -2890,7 +2951,8 @@ CheckRADIUSAuth(Port *port)
radiusports ? lfirst(radiusports) : NULL,
identifiers ? lfirst(identifiers) : NULL,
port->user_name,
- passwd);
+ passwd,
+ port->hba->radiusrequirema);
/*------
* STATUS_OK = Login OK
@@ -2931,8 +2993,12 @@ CheckRADIUSAuth(Port *port)
}
static int
-PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd)
+PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd, bool requirema)
{
+ pg_hmac_ctx *hmac_context;
+ uint8 message_authenticator[MD5_DIGEST_LENGTH];
+ uint8 *message_authenticator_location;
+ uint8 message_authenticator_size;
radius_packet radius_send_pack;
radius_packet radius_recv_pack;
radius_packet *packet = &radius_send_pack;
@@ -2993,6 +3059,19 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
return STATUS_ERROR;
}
packet->id = packet->vector[0];
+
+ /*
+ * Add Message-Authenticator attribute first, per recommendations for
+ * Blast-RADIUS mitigation. Initially it holds zeroes, but we remember
+ * where it is in the message so that we can fill it in later.
+ */
+ memset(message_authenticator, 0, lengthof(message_authenticator));
+ message_authenticator_location =
+ radius_add_attribute(packet,
+ RADIUS_MESSAGE_AUTHENTICATOR,
+ message_authenticator,
+ lengthof(message_authenticator));
+
radius_add_attribute(packet, RADIUS_SERVICE_TYPE, (const unsigned char *) &service, sizeof(service));
radius_add_attribute(packet, RADIUS_USER_NAME, (const unsigned char *) user_name, strlen(user_name));
radius_add_attribute(packet, RADIUS_NAS_IDENTIFIER, (const unsigned char *) identifier, strlen(identifier));
@@ -3048,6 +3127,32 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
packetlength = packet->length;
packet->length = pg_hton16(packet->length);
+ /*
+ * Compute the Message-Authenticator for the whole message. The
+ * Message-Authenticator itself is one of the attributes, but it holds
+ * zeroes at this point.
+ */
+ hmac_context = pg_hmac_create(PG_MD5);
+ if (hmac_context == NULL ||
+ pg_hmac_init(hmac_context, (uint8 *) secret, strlen(secret)) < 0 ||
+ pg_hmac_update(hmac_context,
+ (uint8 *) radius_buffer, packetlength) < 0 ||
+ pg_hmac_final(hmac_context,
+ message_authenticator,
+ lengthof(message_authenticator)) < 0)
+ {
+ ereport(LOG, (errmsg("could not compute RADIUS Message-Authenticator: %s",
+ pg_hmac_error(hmac_context))));
+ pg_hmac_free(hmac_context);
+ pg_freeaddrinfo_all(hint.ai_family, serveraddrs);
+ return STATUS_ERROR;
+ }
+ pg_hmac_free(hmac_context);
+
+ /* Overwrite the attribute with the computed signature. */
+ memcpy(message_authenticator_location, message_authenticator,
+ lengthof(message_authenticator));
+
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
if (sock == PGINVALID_SOCKET)
{
@@ -3232,6 +3337,96 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
continue;
}
+ /* Search for the Message-Authenticator attribute. */
+ if (!radius_find_attribute((uint8 *) receive_buffer,
+ packetlength,
+ RADIUS_MESSAGE_AUTHENTICATOR,
+ &message_authenticator_location,
+ &message_authenticator_size))
+ {
+ ereport(LOG,
+ (errmsg("RADIUS response from %s has malformed attributes",
+ server)));
+ continue;
+ }
+ else if (message_authenticator_location == NULL)
+ {
+ /*
+ * If configured to require a Message-Authenticator, ignore this
+ * message.
+ */
+ if (requirema)
+ {
+ ereport(LOG,
+ (errmsg("RADIUS response from %s has no Message-Authenticator",
+ server)));
+ continue;
+ }
+ }
+ else if (message_authenticator_size != lengthof(message_authenticator))
+ {
+ ereport(LOG,
+ (errmsg("RADIUS response from %s has unexpected Message-Authenticator size",
+ server)));
+ continue;
+ }
+ else
+ {
+ uint8 message_authenticator_copy[lengthof(message_authenticator)];
+
+ /*
+ * Save a copy of the received HMAC, and zero out the one in the
+ * message so that we have input required to recompute it.
+ */
+ memcpy(message_authenticator_copy,
+ message_authenticator_location,
+ lengthof(message_authenticator));
+ memset(message_authenticator_location,
+ 0,
+ lengthof(message_authenticator));
+
+ /*
+ * Compute the expected value. Note that the HMAC for
+ * Access-Accept and Access-Reject message uses the authenticator
+ * from the original Access-Request message, so we have to do a
+ * bit of splicing.
+ */
+ hmac_context = pg_hmac_create(PG_MD5);
+ if (hmac_context == NULL ||
+ pg_hmac_init(hmac_context, (uint8 *) secret,
+ strlen(secret)) < 0 ||
+ pg_hmac_update(hmac_context,
+ (uint8 *) receive_buffer,
+ offsetof(radius_packet, vector)) < 0 ||
+ pg_hmac_update(hmac_context,
+ packet->vector,
+ RADIUS_VECTOR_LENGTH) < 0 ||
+ pg_hmac_update(hmac_context,
+ (uint8 *) receive_buffer + RADIUS_HEADER_LENGTH,
+ packetlength - RADIUS_HEADER_LENGTH) < 0 ||
+ pg_hmac_final(hmac_context,
+ message_authenticator,
+ lengthof(message_authenticator)) < 0)
+ {
+ ereport(LOG, (errmsg("could not compute RADIUS Message-Authenticator: %s",
+ pg_hmac_error(hmac_context))));
+ pg_hmac_free(hmac_context);
+ closesocket(sock);
+ return STATUS_ERROR;
+ }
+ pg_hmac_free(hmac_context);
+
+ /* Verify. */
+ if (memcmp(message_authenticator_copy,
+ message_authenticator,
+ lengthof(message_authenticator)) != 0)
+ {
+ ereport(LOG, (errmsg("RADIUS response from %s has invalid Message-Authenticator",
+ server)));
+ continue;
+ }
+ }
+
if (receivepacket->code == RADIUS_ACCESS_ACCEPT)
{
closesocket(sock);
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 75d588e36a1..7ff8e1ca6e7 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2446,6 +2446,14 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
hbaline->radiusidentifiers = parsed_identifiers;
hbaline->radiusidentifiers_s = pstrdup(val);
}
+ else if (strcmp(name, "radiusrequirema") == 0)
+ {
+ REQUIRE_AUTH_OPTION(uaRADIUS, "radiusrequirema", "radius");
+ if (strcmp(val, "1") == 0)
+ hbaline->radiusrequirema = true;
+ else
+ hbaline->radiusrequirema = false;
+ }
else
{
ereport(elevel,
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 8ea837ae82a..139e4bd410a 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -135,6 +135,7 @@ typedef struct HbaLine
char *radiusidentifiers_s;
List *radiusports;
char *radiusports_s;
+ bool radiusrequirema;
} HbaLine;
typedef struct IdentLine
diff --git a/src/test/authentication/t/007_radius.pl b/src/test/authentication/t/007_radius.pl
index c683d1d5579..4f66f039310 100644
--- a/src/test/authentication/t/007_radius.pl
+++ b/src/test/authentication/t/007_radius.pl
@@ -112,6 +112,11 @@ log {
file = "$radiusd_dir/radius.log"
}
+# FreeRADIUS < 3.2.5 doesn't understand this, but ignores it.
+security {
+ require_message_authenticator = "yes"
+}
+
pidfile = "$radiusd_dir/radiusd.pid"
};
--
2.46.0
From 9dd68610b8931ebd6d0969040daeb58d85edb7be Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Mon, 5 Aug 2024 17:09:57 +1200
Subject: [PATCH v2 4/4] XXX BlastRADIUS back-patch kludge for 12 and 13
---
doc/src/sgml/client-auth.sgml | 9 ++++++++
src/backend/libpq/auth.c | 42 ++++++++++++++++++++++++++++++++++-
src/backend/libpq/hba.c | 10 +++++++++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 37ee2833d37..bc88cad132c 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -2185,6 +2185,15 @@ host ... ldap ldapbasedn="dc=example,dc=net"
<literal>Message-Authenticator</literal> in all replies. The default will be
changed to 1 in a future PostgreSQL version.
</para>
+ <note>
+ <para>
+ <literal>Message-Authenticator</literal> attributes can only be
+ computed if <productname>PostgreSQL</productname> was built support for
+ <productname>OpenSSL</productname>. Otherwise, they
+ will be silently omitted from outgoing messages, this setting
+ cannot be turned on, and incoming messages will not be verified.
+ </para>
+ </note>
</listitem>
</varlistentry>
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 07dc1e93791..72a9c0a4e52 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -24,7 +24,6 @@
#include <unistd.h>
#include "commands/user.h"
-#include "common/hmac.h"
#include "common/ip.h"
#include "common/md5.h"
#include "libpq/auth.h"
@@ -40,6 +39,10 @@
#include "storage/ipc.h"
#include "utils/memutils.h"
+#ifdef USE_OPENSSL
+#include "openssl/hmac.h"
+#endif
+
/*----------------------------------------------------------------
* Global authentication functions
*----------------------------------------------------------------
@@ -2811,6 +2814,31 @@ typedef struct
/* Seconds to wait - XXX: should be in a config variable! */
#define RADIUS_TIMEOUT 3
+#ifdef USE_OPENSSL
+/*
+ * Locally redirect PostgreSQL 14's pg_hmac_XXX API directly to OpenSSL
+ * functions, to simplify back-patching of BlastRADIUS mitigation to
+ * PostgreSQL 12 and 13. If built without OpenSSL, skip HMAC support and
+ * disable Message-Authenticator attributes.
+ */
+#define RADIUS_USE_HMAC
+#define pg_hmac_ctx HMAC_CTX
+#define pg_hmac_create(...) HMAC_CTX_new()
+#define pg_hmac_init(ctx, key, key_len, ...) HMAC_Init_ex(ctx, key, key_len, EVP_md5(), NULL)
+#define pg_hmac_update HMAC_Update
+static inline int
+pg_hmac_final(pg_hmac_ctx *ctx, void *md, size_t len)
+{
+ unsigned int l = len;
+ int result = HMAC_Final(ctx, md, &l);
+
+ Assert(l == len);
+ return result;
+}
+#define pg_hmac_free(x) if (x) HMAC_CTX_free(x)
+#define pg_hmac_error(x) "HMAC error"
+#endif
+
static uint8 *
radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *data, int len)
{
@@ -2843,6 +2871,7 @@ radius_add_attribute(radius_packet *packet, uint8 type, const unsigned char *dat
return attr->data;
}
+#ifdef RADIUS_USE_HMAC
/*
* Search for an attribute in a received message. If the attribute is found,
* sets *value and *length (not including the header), and returns true. If
@@ -2895,6 +2924,7 @@ radius_find_attribute(uint8 *packet,
*length = 0;
return true;
}
+#endif
static int
CheckRADIUSAuth(Port *port)
@@ -2995,10 +3025,12 @@ CheckRADIUSAuth(Port *port)
static int
PerformRadiusTransaction(const char *server, const char *secret, const char *portstr, const char *identifier, const char *user_name, const char *passwd, bool requirema)
{
+#ifdef RADIUS_USE_HMAC
pg_hmac_ctx *hmac_context;
uint8 message_authenticator[MD5_DIGEST_LENGTH];
uint8 *message_authenticator_location;
uint8 message_authenticator_size;
+#endif
radius_packet radius_send_pack;
radius_packet radius_recv_pack;
radius_packet *packet = &radius_send_pack;
@@ -3060,6 +3092,8 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
}
packet->id = packet->vector[0];
+#ifdef RADIUS_USE_HMAC
+
/*
* Add Message-Authenticator attribute first, per recommendations for
* Blast-RADIUS mitigation. Initially it holds zeroes, but we remember
@@ -3071,6 +3105,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
RADIUS_MESSAGE_AUTHENTICATOR,
message_authenticator,
lengthof(message_authenticator));
+#endif
radius_add_attribute(packet, RADIUS_SERVICE_TYPE, (const unsigned char *) &service, sizeof(service));
radius_add_attribute(packet, RADIUS_USER_NAME, (const unsigned char *) user_name, strlen(user_name));
@@ -3127,6 +3162,8 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
packetlength = packet->length;
packet->length = pg_hton16(packet->length);
+#ifdef RADIUS_USE_HMAC
+
/*
* Compute the Message-Authenticator for the whole message. The
* Message-Authenticator itself is one of the attributes, but it holds
@@ -3152,6 +3189,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
/* Overwrite the attribute with the computed signature. */
memcpy(message_authenticator_location, message_authenticator,
lengthof(message_authenticator));
+#endif
sock = socket(serveraddrs[0].ai_family, SOCK_DGRAM, 0);
if (sock == PGINVALID_SOCKET)
@@ -3337,6 +3375,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
continue;
}
+#ifdef RADIUS_USE_HMAC
/* Search for the Message-Authenticator attribute. */
if (!radius_find_attribute((uint8 *) receive_buffer,
packetlength,
@@ -3426,6 +3465,7 @@ PerformRadiusTransaction(const char *server, const char *secret, const char *por
continue;
}
}
+#endif
if (receivepacket->code == RADIUS_ACCESS_ACCEPT)
{
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 7ff8e1ca6e7..abe0c194d88 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2450,7 +2450,17 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
{
REQUIRE_AUTH_OPTION(uaRADIUS, "radiusrequirema", "radius");
if (strcmp(val, "1") == 0)
+ {
+#ifdef USE_OPENSSL
hbaline->radiusrequirema = true;
+#else
+ ereport(elevel,
+ (errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("this build does not support radiusrequirema=1"),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, file_name)));
+#endif
+ }
else
hbaline->radiusrequirema = false;
}
--
2.46.0
