You've already forked pgbackrest
							
							
				mirror of
				https://github.com/pgbackrest/pgbackrest.git
				synced 2025-10-30 23:37:45 +02:00 
			
		
		
		
	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.
This commit is contained in:
		
				
					committed by
					
						 GitHub
						GitHub
					
				
			
			
				
	
			
			
			
						parent
						
							d295156dd3
						
					
				
				
					commit
					dfb620b0b8
				
			| @@ -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."> | ||||
|   | ||||
| @@ -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} | ||||
|   | ||||
| @@ -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) | ||||
|   | ||||
| @@ -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); | ||||
|  | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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----- | ||||
|   | ||||
| @@ -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; | ||||
|   | ||||
| @@ -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, ...)                                                                                         \ | ||||
|   | ||||
| @@ -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( | ||||
|   | ||||
		Reference in New Issue
	
	Block a user