pgbackrest.git / summary / log / commit / refs
commit dfb620b0b8cf884ca486b413f4cd162e258b2fc5
Author: David Christensen <david.christensen@crunchydata.com>
Commit: GitHub <noreply@github.com>
Date: Thu Jul 11 03:54:20 2024 +0000
Support IP-based SANs for TLS certificate validation.
The prior SAN code only recognized DNS-based SANs, which meant that it would not properly validate if using an IP-based SAN.
Add support for IPv4 and IPv6 SANs with exact matching only.
This simplifies testing when certificate generation tools have trouble generating a DNS:1.2.3.4-style SAN, preferring to include the SAN as IP:1.2.3.4.
doc/xml/release/2024/2.53.xml | 14 +++++++
src/common/io/tls/client.c | 57 +++++++++++++++++++++++++
src/common/io/tls/common.c | 19 +++++++++
src/common/io/tls/common.h | 3 ++
test/certificate/pgbackrest-test-server.cnf | 7 +++-
test/certificate/pgbackrest-test-server.crt | 35 ++++++++--------
test/src/common/harnessServer.c | 4 +-
test/src/common/harnessServer.h | 1 +
test/src/module/common/ioTlsTest.c | 65 ++++++++++++++++++++++++++++-
9 files changed, 184 insertions(+), 21 deletions(-)
diff --git a/doc/xml/release/2024/2.53.xml b/doc/xml/release/2024/2.53.xml
index 17f418b57..034926900 100644
--- a/doc/xml/release/2024/2.53.xml
+++ b/doc/xml/release/2024/2.53.xml
@@ -35,6 +35,20 @@
</release-item>
</release-bug-list>
+ <release-feature-list>
+ <release-item>
+ <github-issue id="1977"/>
+ <github-pull-request id="2047"/>
+
+ <release-item-contributor-list>
+ <release-item-contributor id="david.christensen"/>
+ <release-item-reviewer id="david.steele"/>
+ </release-item-contributor-list>
+
+ <p>Support IP-based SANs for <proper>TLS</proper> certificate validation.</p>
+ </release-item>
+ </release-feature-list>
+
<release-improvement-list>
<release-item>
<commit subject="Refactor lock module.">
diff --git a/src/common/io/tls/client.c b/src/common/io/tls/client.c
index be37fa418..7f991bfd7 100644
--- a/src/common/io/tls/client.c
+++ b/src/common/io/tls/client.c
@@ -3,6 +3,12 @@ TLS Client
***********************************************************************************************************************************/
#include "build.auto.h"
+// {uncrustify_off - header order required for FreeBSD}
+#include <sys/socket.h>
+#include <sys/types.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+// {uncrustify_on}
#include <strings.h>
#include "common/crypto/common.h"
@@ -125,6 +131,55 @@ tlsClientHostVerifyName(const String *const host, const String *const name)
FUNCTION_LOG_RETURN(BOOL, result);
}
+/***********************************************************************************************************************************
+Check if an IP address from the server certificate matches the hostname
+
+Adapted from libpq/fe-secure-common.c.
+***********************************************************************************************************************************/
+static bool
+tlsClientHostVerifyIPAddr(const String *const host, const Buffer *const address)
+{
+ FUNCTION_LOG_BEGIN(logLevelTrace);
+ FUNCTION_LOG_PARAM(STRING, host);
+ FUNCTION_LOG_PARAM(BUFFER, address);
+ FUNCTION_LOG_END();
+
+ ASSERT(host != NULL);
+ ASSERT(address != NULL);
+
+ bool result = false;
+
+ // The data from the certificate is in network byte order. Convert our host string to network-ordered bytes as well for
+ // comparison. The host string is not guaranteed to be an IP address so if this conversion fails consider it a mismatch rather
+ // than an error. Use the size of the address parameter to determine ipv4 or ipv6 checking.
+ const size_t addrSize = bufSize(address);
+
+ // IPv4
+ if (addrSize == sizeof(struct in_addr))
+ {
+ struct in_addr addr;
+
+ if (inet_pton(AF_INET, strZ(host), &addr) == 1 && // {vm_covered}
+ memcmp(bufPtrConst(address), &addr.s_addr, addrSize) == 0) // {vm_covered}
+ {
+ result = true; // {vm_covered}
+ }
+ }
+ // Else IPv6
+ else if (addrSize == sizeof(struct in6_addr))
+ {
+ struct in6_addr addr;
+
+ if (inet_pton(AF_INET6, strZ(host), &addr) == 1 && // {vm_covered}
+ memcmp(bufPtrConst(address), &addr.s6_addr, addrSize) == 0) // {vm_covered}
+ {
+ result = true; // {vm_covered}
+ }
+ }
+
+ FUNCTION_LOG_RETURN(BOOL, result);
+}
+
/***********************************************************************************************************************************
Verify that the server certificate matches the hostname we connected to
@@ -162,6 +217,8 @@ tlsClientHostVerify(const String *const host, X509 *const certificate)
if (name->type == GEN_DNS) // {vm_covered}
result = tlsClientHostVerifyName(host, tlsAsn1ToStr(name->d.dNSName)); // {vm_covered}
+ else if (name->type == GEN_IPADD) // {vm_covered}
+ result = tlsClientHostVerifyIPAddr(host, tlsAsn1ToBuf(name->d.dNSName)); // {vm_covered}
if (result != false) // {vm_covered}
break; // {vm_covered}
diff --git a/src/common/io/tls/common.c b/src/common/io/tls/common.c
index 3cb9a3dd6..63b436cee 100644
--- a/src/common/io/tls/common.c
+++ b/src/common/io/tls/common.c
@@ -14,6 +14,25 @@ TLS Common
#include "common/user.h"
#include "storage/posix/storage.h"
+/**********************************************************************************************************************************/
+FN_EXTERN Buffer *
+tlsAsn1ToBuf(const ASN1_STRING *const nameAsn1)
+{
+ FUNCTION_TEST_BEGIN();
+ FUNCTION_TEST_PARAM_P(VOID, nameAsn1);
+ FUNCTION_TEST_END();
+
+ // The name should not be null
+ if (nameAsn1 == NULL) // {vm_covered}
+ THROW(CryptoError, "TLS certificate name entry is missing");
+
+ FUNCTION_TEST_RETURN( // {vm_covered}
+ BUFFER,
+ bufNewC(
+ (const char *)ASN1_STRING_get0_data(nameAsn1),
+ (size_t)ASN1_STRING_length(nameAsn1)));
+}
+
/**********************************************************************************************************************************/
FN_EXTERN String *
tlsAsn1ToStr(const ASN1_STRING *const nameAsn1)
diff --git a/src/common/io/tls/common.h b/src/common/io/tls/common.h
index fa9c6cbec..316dc3f53 100644
--- a/src/common/io/tls/common.h
+++ b/src/common/io/tls/common.h
@@ -11,6 +11,9 @@ TLS Common
/***********************************************************************************************************************************
Functions
***********************************************************************************************************************************/
+// Convert an ASN1 string used in certificates to a Buffer
+FN_EXTERN Buffer *tlsAsn1ToBuf(const ASN1_STRING *nameAsn1);
+
// Convert an ASN1 string used in certificates to a String
FN_EXTERN String *tlsAsn1ToStr(const ASN1_STRING *nameAsn1);
diff --git a/test/certificate/pgbackrest-test-server.cnf b/test/certificate/pgbackrest-test-server.cnf
index 7443b43b5..850ba32a6 100644
--- a/test/certificate/pgbackrest-test-server.cnf
+++ b/test/certificate/pgbackrest-test-server.cnf
@@ -32,9 +32,12 @@ DNS.1 = test.pgbackrest.org
DNS.2 = *.test.pgbackrest.org
DNS.3 = *.test2.pgbackrest.org
-# Used in non-container unit tests
-DNS.4 = 127.0.0.1
+# Test IP matching
IP.1 = 127.0.0.1
+IP.2 = ::1
+
+# Unused alt name type for coverage
+email.0 = email@email.com
# Used in integration tests
DNS.5 = pg1
diff --git a/test/certificate/pgbackrest-test-server.crt b/test/certificate/pgbackrest-test-server.crt
index 01c904265..dd0407753 100644
--- a/test/certificate/pgbackrest-test-server.crt
+++ b/test/certificate/pgbackrest-test-server.crt
@@ -1,8 +1,8 @@
-----BEGIN CERTIFICATE-----
-MIIF8jCCA9qgAwIBAgIUJCya0E5vFzyH2AgiM3HSAHmpZ1QwDQYJKoZIhvcNAQEL
+MIIGCzCCA/OgAwIBAgIUJCya0E5vFzyH2AgiM3HSAHmpZ14wDQYJKoZIhvcNAQEL
BQAwXDELMAkGA1UEBhMCVVMxDDAKBgNVBAgMA0FsbDEMMAoGA1UEBwwDQWxsMRMw
EQYDVQQKDApwZ0JhY2tSZXN0MRwwGgYDVQQDDBN0ZXN0LnBnYmFja3Jlc3Qub3Jn
-MCAXDTI0MDMwNDAxMzgzMFoYDzIyOTcxMjE3MDEzODMwWjB6MQswCQYDVQQGEwJV
+MCAXDTI0MDcwODA0NDEyOVoYDzIyOTgwNDIyMDQ0MTI5WjB6MQswCQYDVQQGEwJV
UzEMMAoGA1UECAwDQWxsMQwwCgYDVQQHDANBbGwxEzARBgNVBAoMCnBnQmFja1Jl
c3QxHDAaBgNVBAsME1VuaXQgVGVzdGluZyBEb21haW4xHDAaBgNVBAMME3Rlc3Qu
cGdiYWNrcmVzdC5vcmcwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDD
@@ -16,19 +16,20 @@ dCdyXuSftGFx0JxvmDhl9qFGarv1BKgwO83j7sy3IREte1K21JaIHNBVWP+NwU0N
4Z4OMqnpnnnGiyi0xnfJVqOXghu5BLWl9MuOntZ0nnzLmFD7w795uNRgjE6jmRmF
FlX+PGqhHsZr0wZsBDsE9xO4i2l8aqJZx1hT5l3LIXC+lei/qo2gJi3nyePuz4UB
t53sTNEdrZndFUaRyq/aJfkR13J0eaoqKn5BRRHhw8tRef6S84e0kQ6ABYNRGHQN
-V+GswPl1fV37114FTBnz2Bi/GSQSs8vWjw49HHKK5wIDAQABo4GLMIGIMAkGA1Ud
-EwQCMAAwCwYDVR0PBAQDAgXgMG4GA1UdEQRnMGWCE3Rlc3QucGdiYWNrcmVzdC5v
-cmeCFSoudGVzdC5wZ2JhY2tyZXN0Lm9yZ4IWKi50ZXN0Mi5wZ2JhY2tyZXN0Lm9y
-Z4IJMTI3LjAuMC4xhwR/AAABggNwZzGCA3BnMoIEcmVwbzANBgkqhkiG9w0BAQsF
-AAOCAgEAX0MEXH6ANllRhdQz6neQ7SVG48Aj1lEAGeOhfpoNKzuyBcRjVw7+NyNN
-IwlPKSSBDpnxaWQ5rCLtBtXod6yPMGKTRlFHwFFzfOps6nlRQjPsA848d6daLBvj
-unpUQx4NFGPZJSs6z5z4BlT/+5mJVHC9qsrZBtkndYpRWo37xbVhRqP0+FSTbzrx
-Gj2td3PoqQBgA/AmSKIpwagGzw7cSor1r4uEjkVxxyOMRbjuuASHMHUM7MtQV3YR
-rz9UspvGfoKBdUkzMoqKRwxZWuh+uAoM+1GWXBjqlN6uAdQxpV2wZ75iRJp3Y4Bk
-/CkXTLZ83lARGLqS/E5EFfg7Z9Bre2f5fHzV8C/h6WGpvCt/GlZqTx8fix/mMPT2
-CFq+FcSmvF5JsIMxUnpvTw1hcTDNRPnOkFKnO1bjf9+jGCwzDUoGReYpb5veFSxh
-IFkQ3oyw3/6v11aPstXSADTvRTFMyRklqu5NIHkMVQCPwQCAE3346KpEsT5HO2T7
-X57sP05alMESjUv1sR260yYC3GUr0dbmf8gthVDhH1SsP3Drn/A3l70xGASnJA46
-LCZwAJZ0KI6G/ok17lTZe9Hjwn2DkmVf1CKD7gXjmroYQI9O+etUtD/g+B2AISTG
-SP60NakteOBWqmcSirV5npKh/SZR8Il5oaLS+3HerhgwvNDJ078=
+V+GswPl1fV37114FTBnz2Bi/GSQSs8vWjw49HHKK5wIDAQABo4GkMIGhMAkGA1Ud
+EwQCMAAwCwYDVR0PBAQDAgXgMIGGBgNVHREEfzB9ghN0ZXN0LnBnYmFja3Jlc3Qu
+b3JnghUqLnRlc3QucGdiYWNrcmVzdC5vcmeCFioudGVzdDIucGdiYWNrcmVzdC5v
+cmeHBH8AAAGHEAAAAAAAAAAAAAAAAAAAAAGBD2VtYWlsQGVtYWlsLmNvbYIDcGcx
+ggNwZzKCBHJlcG8wDQYJKoZIhvcNAQELBQADggIBAIsHA0juhV08BvCTEdwrwW2p
+9EWEftVptdkKSanToAxV1gWWHh5/20sjUJyuRNc9MeiiwSjM07I43oUHRpo07zdk
+2IxBx+vQWmeJbiKERuHiL5PUPOn8Acb4kjrJtrbp5zjS9fs8sqbV5zNH3BNz3j6r
+RjVhyXLc5LShMh8SGeiRItpgwKOHiZwWRB2X7SFGcBb4sr4zumbhpIj3IvscsVgw
+hfK3BiHCgrnqV4tKcdfub/JfBd6l052ui+mNyJGvcFskcPMJPgXeOMCgMpCexPJu
+QiFWVpvju5EtrVwFC9ejHjxs0OXeJMxCM4hUMJqa5gSZa330HRg8AflWlBO0qm94
+FofBcZm6hzbwK/hHREs4BCU2p0XSmGyt/05fi8Mlu6DmDo6+B+MAwRj6ADh5S1wU
+2AGFlza1SykntQDVyGCQDbP6zlVosNHAkVTIySgXJcNk9g5i8eHa6Hi2cIn9iuUX
+dJSjqJ24hWkGWD2f/YJcRTDq4AfmFOkSioSREspM7Xo5zt5UXhEosf29YfLo5JDL
+IWBYD1BDvfBGmF0q5ZlDZ6RT8qHIJK6rbHGPp1F3LpDpRwrVmOJDjAba2Ld2yBn+
+TBasF+zOLIZbdkNFAukcEvnz+jIGHYG+7MJ1sxvNwgZJDDRMv/zRgqG4PBDDnmpS
+RE4TH20A2b9H8rrPKY8W
-----END CERTIFICATE-----
diff --git a/test/src/common/harnessServer.c b/test/src/common/harnessServer.c
index 0e8e9a185..5edc02fef 100644
--- a/test/src/common/harnessServer.c
+++ b/test/src/common/harnessServer.c
@@ -244,8 +244,10 @@ hrnServerRun(IoRead *const read, const HrnServerProtocol protocol, const unsigne
FUNCTION_HARNESS_PARAM(IO_READ, read);
FUNCTION_HARNESS_PARAM(ENUM, protocol);
FUNCTION_HARNESS_PARAM(UINT, port);
+ FUNCTION_HARNESS_PARAM(STRING, param.ca);
FUNCTION_HARNESS_PARAM(STRING, param.certificate);
FUNCTION_HARNESS_PARAM(STRING, param.key);
+ FUNCTION_HARNESS_PARAM(STRING, param.address);
FUNCTION_HARNESS_END();
ASSERT(read != NULL);
@@ -268,7 +270,7 @@ hrnServerRun(IoRead *const read, const HrnServerProtocol protocol, const unsigne
tlsServer = tlsServerNew(STRDEF(HRN_SERVER_HOST), param.ca, param.key, param.certificate, 5000);
}
- IoServer *socketServer = sckServerNew(STRDEF("127.0.0.1"), port, 5000);
+ IoServer *socketServer = sckServerNew(param.address == NULL ? STRDEF("127.0.0.1") : param.address, port, 5000);
// Loop until no more commands
IoSession *serverSession = NULL;
diff --git a/test/src/common/harnessServer.h b/test/src/common/harnessServer.h
index b1f673c24..964e2e750 100644
--- a/test/src/common/harnessServer.h
+++ b/test/src/common/harnessServer.h
@@ -57,6 +57,7 @@ typedef struct HrnServerRunParam
const String *ca; // TLS CA store when protocol = hrnServerProtocolTls
const String *certificate; // TLS certificate when protocol = hrnServerProtocolTls
const String *key; // TLS key when protocol = hrnServerProtocolTls
+ const String *address; // Use address other than 127.0.0.1
} HrnServerRunParam;
#define hrnServerRunP(read, protocol, port, ...) \
diff --git a/test/src/module/common/ioTlsTest.c b/test/src/module/common/ioTlsTest.c
index 332605e18..be26de434 100644
--- a/test/src/module/common/ioTlsTest.c
+++ b/test/src/module/common/ioTlsTest.c
@@ -569,9 +569,10 @@ testRun(void)
// Additional coverage not provided by testing with actual certificates
// *****************************************************************************************************************************
- if (testBegin("tlsAsn1ToStr(), tlsClientHostVerify(), and tlsClientHostVerifyName()"))
+ if (testBegin("tlsAsn1ToStr/Buf(), tlsClientHostVerify(), tlsClientHostVerifyName(), and tlsClientHostVerifyIpAddr()"))
{
TEST_ERROR(tlsAsn1ToStr(NULL), CryptoError, "TLS certificate name entry is missing");
+ TEST_ERROR(tlsAsn1ToBuf(NULL), CryptoError, "TLS certificate name entry is missing");
TEST_ERROR(
tlsClientHostVerifyName(
@@ -582,6 +583,14 @@ testRun(void)
TEST_RESULT_BOOL(tlsClientHostVerifyName(STRDEF("host"), STRDEF("**")), false, "invalid pattern");
TEST_RESULT_BOOL(tlsClientHostVerifyName(STRDEF("host"), STRDEF("*.")), false, "invalid pattern");
TEST_RESULT_BOOL(tlsClientHostVerifyName(STRDEF("a.bogus.host.com"), STRDEF("*.host.com")), false, "invalid host");
+
+ // -------------------------------------------------------------------------------------------------------------------------
+ TEST_TITLE("tlsClientHostVerifyIpAddr()");
+
+ TEST_RESULT_BOOL(tlsClientHostVerifyIPAddr(STRDEF("127.0.0.1"), bufNewC("\x7F\0\0", 3)), false, "invalid len");
+ TEST_RESULT_BOOL(tlsClientHostVerifyIPAddr(STRDEF("127.0.0.1"), bufNewC("\x7F\0\0\x02", 4)), false, "ipv4 no match");
+ TEST_RESULT_BOOL(
+ tlsClientHostVerifyIPAddr(STRDEF("::1"), bufNewC("\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x02", 16)), false, "ipv6 no match");
}
// *****************************************************************************************************************************
@@ -814,6 +823,19 @@ testRun(void)
STRDEF("host.test2.pgbackrest.org"), 0, 0, true, .caFile = STRDEF(HRN_SERVER_CA))),
"open connection");
+ // -----------------------------------------------------------------------------------------------------------------
+ TEST_TITLE("match ipv4");
+
+ hrnServerScriptAccept(tls);
+ hrnServerScriptClose(tls);
+
+ TEST_RESULT_VOID(
+ ioClientOpen(
+ tlsClientNewP(
+ sckClientNew(STRDEF("127.0.0.1"), testPort, 5000, 5000), STRDEF("127.0.0.1"), 0, 0, true,
+ .caFile = STRDEF(HRN_SERVER_CA))),
+ "open connection");
+
// -----------------------------------------------------------------------------------------------------------------
TEST_TITLE("unable to find matching hostname in certificate");
@@ -862,6 +884,47 @@ testRun(void)
}
HRN_FORK_END();
+ // Server on IPv6
+ // -------------------------------------------------------------------------------------------------------------------------
+ HRN_FORK_BEGIN()
+ {
+ const unsigned int testPort = hrnServerPortNext();
+
+ HRN_FORK_CHILD_BEGIN(.prefix = "test server", .timeout = 5000)
+ {
+ // Start server to test various certificate errors
+ TEST_RESULT_VOID(
+ hrnServerRunP(
+ HRN_FORK_CHILD_READ(), hrnServerProtocolTls, testPort, .certificate = STRDEF(HRN_SERVER_CERT),
+ .key = STRDEF(HRN_SERVER_KEY), .address = STRDEF("::1")),
+ "tls alt name ipv6 server");
+ }
+ HRN_FORK_CHILD_END();
+
+ HRN_FORK_PARENT_BEGIN(.prefix = "test client", .timeout = 1000)
+ {
+ IoWrite *const tls = hrnServerScriptBegin(HRN_FORK_PARENT_WRITE(0));
+
+ // -----------------------------------------------------------------------------------------------------------------
+ TEST_TITLE("match ipv6");
+
+ hrnServerScriptAccept(tls);
+ hrnServerScriptClose(tls);
+
+ TEST_RESULT_VOID(
+ ioClientOpen(
+ tlsClientNewP(
+ sckClientNew(STRDEF("::1"), testPort, 5000, 5000), STRDEF("::1"), 0, 0, true,
+ .caFile = STRDEF(HRN_SERVER_CA))),
+ "open connection");
+
+ // -----------------------------------------------------------------------------------------------------------------
+ hrnServerScriptEnd(tls);
+ }
+ HRN_FORK_PARENT_END();
+ }
+ HRN_FORK_END();
+
// -------------------------------------------------------------------------------------------------------------------------
// Put root-owned server key
storagePutP(
[parent: d295156dd38c]