Skip to content

Commit 0b52058

Browse files
committed
More TLS improvements
1 parent 09b7efc commit 0b52058

7 files changed

Lines changed: 121 additions & 117 deletions

File tree

ChangeLog

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22

33
* Source/GSTLS.m: Enable strict host certificate verification by
44
default and use available server name for host name verification
5-
if it is set. This is a big deal as it alters the behavior of
5+
if it is set. Improve printing certificate and other debug.
6+
Fix bug checking remote host name, and change to use system trust
7+
list (if possible) in preference to the one in the gnustep bundle.
8+
This is a big deal as it alters the behavior of
69
NSURL (and the NSURLHandle/NSURLConnection APIs) so that they will
710
now fail to send the HTTPS request if the remote host has a bad
811
certificate (not trusted or secure enough etc). When the APIs were
@@ -16,6 +19,12 @@
1619
is only activated by programattically setting TLS configuration for a
1720
connection, so code which wants certificate verification should be
1821
setting the option to get it anyway.
22+
modified: Source/GSTLS.m
23+
* Tests/base/NSStream/socket.m:
24+
* Tests/base/NSURL/test02.m:
25+
* Tests/base/NSURLConnection/Helpers/NSURLConnectionTest.m:
26+
* Tests/base/NSURLConnection/test02.m:
27+
Update tests to disable hostname validation where we are using bad certs
1928

2029
2026-05-07 Richard Frith-Macdonald <rfm@gnu.org>
2130

Source/GSTLS.m

Lines changed: 86 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,9 @@ static int gcry_mutex_unlock(void **lock)
131131

132132
/* The caFile variable holds the location of the file containing the default
133133
* certificate authorities to be used by our system.
134-
* The hard-coded value is a file in the GSTLS folder of the base library
135-
* resource bundle, but this can be overridden by the GS_TLS_CA_FILE
134+
* The hard-coded value is the GNUTLS system default set, with a fallback to
135+
* a file in the GSTLS folder of the base library resource bundle,
136+
* but this can be overridden by the GS_TLS_CA_FILE
136137
* environment variable, which in turn will be overridden by the GSTLSCAFile
137138
* user default string.
138139
*/
@@ -244,12 +245,6 @@ + (void) _defaultsChanged: (NSNotification*)n
244245
{
245246
str = [env objectForKey: @"SSL_CERT_FILE"];
246247
}
247-
if (nil == str)
248-
{
249-
str = [bundle pathForResource: @"ca-certificates"
250-
ofType: @"crt"
251-
inDirectory: @"GSTLS"];
252-
}
253248
}
254249
str = standardizedPath(str);
255250
ASSIGN(caFile, str);
@@ -608,98 +603,26 @@ @implementation GSTLSCertificateList
608603

609604
+ (void) certInfo: (gnutls_x509_crt_t)cert to: (NSMutableString*)str
610605
{
611-
#if GNUTLS_VERSION_NUMBER >= 0x030507
612-
gnutls_datum_t dn;
613-
#else
614-
char dn[1024];
615-
size_t dn_size = sizeof(dn);
616-
#endif
617-
char serial[40];
618-
size_t serial_size = sizeof(serial);
619-
time_t expiret;
620-
time_t activet;
621-
int algo;
622-
unsigned int bits;
623-
int i;
624-
625-
[str appendFormat: _(@"- Certificate version: #%d\n"),
626-
gnutls_x509_crt_get_version(cert)];
627-
628-
#if GNUTLS_VERSION_NUMBER >= 0x030507
629-
if (GNUTLS_E_SUCCESS == gnutls_x509_crt_get_dn3(cert, &dn, 0))
630-
{
631-
[str appendFormat: @"- Certificate DN: %@\n",
632-
[NSString stringWithUTF8String: (const char*)dn.data]];
633-
gnutls_free(dn.data);
634-
}
635-
if (GNUTLS_E_SUCCESS == gnutls_x509_crt_get_issuer_dn3(cert, &dn, 0))
606+
gnutls_datum_t dn = { 0, 0 };
607+
int ret;
608+
609+
ret = gnutls_x509_crt_print(cert, GNUTLS_CRT_PRINT_FULL, &dn);
610+
if (0 == ret)
611+
{
612+
NSString *tmp;
613+
614+
tmp = [[NSString alloc] initWithBytesNoCopy: dn.data
615+
length: dn.size
616+
encoding: NSUTF8StringEncoding
617+
freeWhenDone: YES];
618+
[str appendString: tmp];
619+
RELEASE(tmp);
620+
}
621+
else
636622
{
637-
[str appendFormat: _(@"- Certificate Issuer's DN: %@\n"),
638-
[NSString stringWithUTF8String: (const char*)dn.data]];
639-
gnutls_free(dn.data);
640-
}
641-
#else
642-
dn_size = sizeof(dn) - 1;
643-
gnutls_x509_crt_get_dn(cert, dn, &dn_size);
644-
dn[dn_size] = '\0';
645-
[str appendFormat: @"- Certificate DN: %@\n",
646-
[NSString stringWithUTF8String: dn]];
647-
648-
dn_size = sizeof(dn) - 1;
649-
gnutls_x509_crt_get_issuer_dn(cert, dn, &dn_size);
650-
dn[dn_size] = '\0';
651-
[str appendFormat: _(@"- Certificate Issuer's DN: %@\n"),
652-
[NSString stringWithUTF8String: dn]];
653-
#endif
654-
655-
activet = gnutls_x509_crt_get_activation_time(cert);
656-
[str appendFormat: _(@"- Certificate is valid since: %s"),
657-
ctime(&activet)];
658-
659-
expiret = gnutls_x509_crt_get_expiration_time(cert);
660-
[str appendFormat: _(@"- Certificate expires: %s"),
661-
ctime (&expiret)];
662-
663-
#if 0
664-
{
665-
char digest[20];
666-
size_t digest_size = sizeof(digest);
667-
if (gnutls_x509_fingerprint(GNUTLS_DIG_MD5,
668-
&cert_list[0], digest, &digest_size) >= 0)
669-
{
670-
[str appendString: _(@"- Certificate fingerprint: ")];
671-
for (i = 0; i < digest_size; i++)
672-
{
673-
[str appendFormat: @"%.2x ", (unsigned char)digest[i]];
674-
}
675-
[str appendString: @"\n"];
676-
}
677-
}
678-
#endif
679-
680-
if (gnutls_x509_crt_get_serial(cert, serial, &serial_size) >= 0)
681-
{
682-
[str appendString: _(@"- Certificate serial number: ")];
683-
for (i = 0; i < serial_size; i++)
684-
{
685-
[str appendFormat: @"%.2x ", (unsigned char)serial[i]];
686-
}
687-
[str appendString: @"\n"];
688-
}
689-
690-
[str appendString: _(@"- Certificate public key: ")];
691-
algo = gnutls_x509_crt_get_pk_algorithm(cert, &bits);
692-
if (GNUTLS_PK_RSA == algo)
693-
{
694-
[str appendFormat: _(@"RSA - Modulus: %d bits\n"), bits];
695-
}
696-
else if (GNUTLS_PK_DSA == algo)
697-
{
698-
[str appendFormat: _(@"DSA - Exponent: %d bits\n"), bits];
699-
}
700-
else
701-
{
702-
[str appendString: _(@"UNKNOWN\n")];
623+
[str appendString: @"Unknown"];
624+
NSLog(@"-certInfo:to: failed with '%s'", gnutls_strerror(ret));
625+
if (dn.data) free(dn.data);
703626
}
704627
}
705628

@@ -1303,13 +1226,57 @@ + (GSTLSCredentials*) credentialsFromCAFile: (NSString*)ca
13031226
{
13041227
c->trust = YES; // Loaded at least one trusted CA
13051228
}
1306-
if (YES == debug)
1229+
if (debug)
13071230
{
13081231
NSLog(@"Default trusted authorities (from %@): %d",
13091232
dca, ret);
13101233
}
13111234
}
13121235
}
1236+
else
1237+
{
1238+
const char *path;
1239+
int ret;
1240+
1241+
/* Try system sertificates, and if those fail, try base library ones.
1242+
*/
1243+
ret = gnutls_certificate_set_x509_system_trust(c->certcred);
1244+
if (ret < 0)
1245+
{
1246+
NSBundle *bundle = [NSBundle bundleForClass: [NSObject class]];
1247+
NSString *str;
1248+
1249+
str = [bundle pathForResource: @"ca-certificates"
1250+
ofType: @"crt"
1251+
inDirectory: @"GSTLS"];
1252+
str = standardizedPath(str);
1253+
path = [str gnutlsFileSystemRepresentation];
1254+
ret = gnutls_certificate_set_x509_trust_file(c->certcred,
1255+
path, GNUTLS_X509_FMT_PEM);
1256+
if (ret >= 0)
1257+
{
1258+
if (debug)
1259+
{
1260+
NSLog(@"Default trusted authorities (from base) %d", ret);
1261+
}
1262+
if (ret > 0)
1263+
{
1264+
c->trust = YES; // Loaded at least one trusted CA
1265+
}
1266+
}
1267+
}
1268+
else
1269+
{
1270+
if (debug)
1271+
{
1272+
NSLog(@"Default trusted authorities (from O/S) %d", ret);
1273+
}
1274+
if (ret > 0)
1275+
{
1276+
c->trust = YES; // Loaded at least one trusted CA
1277+
}
1278+
}
1279+
}
13131280

13141281
/* Load any specified trusted authority certificates.
13151282
*/
@@ -2073,12 +2040,13 @@ - (BOOL) handshake
20732040
ret = [self verify];
20742041
if (ret < 0)
20752042
{
2076-
if (globalDebug > 1 || (requireVerified && globalDebug > 0)
2043+
if (globalDebug > 1
2044+
|| (requireVerified && globalDebug > 0)
20772045
|| YES == [[opts objectForKey: GSTLSDebug] boolValue])
20782046
{
20792047
NSLog(@"%p unable to verify SSL connection - %s",
20802048
handle, gnutls_strerror(ret));
2081-
NSLog(@"%p %@", handle, [self sessionInfo]);
2049+
NSLog(@"%p failed verify:\n%@", handle, [self sessionInfo]);
20822050
}
20832051
if (requireVerified)
20842052
{
@@ -2371,7 +2339,8 @@ - (NSString*) sessionInfo
23712339
gnutls_x509_crt_import(cert,
23722340
&cert_list[cert_num], GNUTLS_X509_FMT_DER);
23732341

2374-
[str appendFormat: _(@"- Certificate %d info:\n"), cert_num];
2342+
[str appendFormat: _(@"- Index %d of %d, "),
2343+
cert_num, cert_list_size];
23752344

23762345
[GSTLSCertificateList certInfo: cert to: str];
23772346

@@ -2405,6 +2374,7 @@ - (int) verify
24052374
{
24062375
NSArray *names;
24072376
NSString *str;
2377+
NSMutableString *ci;
24082378
unsigned int status;
24092379
const gnutls_datum_t *cert_list;
24102380
unsigned int cert_list_size;
@@ -2521,6 +2491,9 @@ - (int) verify
25212491
#endif
25222492
}
25232493

2494+
ci = [NSMutableString stringWithCapacity: 2000];
2495+
[GSTLSCertificateList certInfo: cert to: ci];
2496+
25242497
str = [opts objectForKey: GSTLSRemoteHosts];
25252498
if (nil == str)
25262499
{
@@ -2557,7 +2530,7 @@ - (int) verify
25572530

25582531
while (nil != (name = [enumerator nextObject]))
25592532
{
2560-
if (0 == gnutls_x509_crt_check_hostname(cert, [name UTF8String]))
2533+
if (gnutls_x509_crt_check_hostname(cert, [name UTF8String]))
25612534
{
25622535
found = YES;
25632536
break;
@@ -2566,26 +2539,25 @@ - (int) verify
25662539
if (NO == found)
25672540
{
25682541
str = [NSString stringWithFormat:
2569-
@"TLS verification: certificate's hostname does not match '%@'",
2570-
names];
2542+
@"TLS verification: hostname does not match '%@' in %@",
2543+
names, ci];
25712544
ASSIGN(problem, str);
25722545
gnutls_x509_crt_deinit(cert);
25732546
if (YES == debug) NSLog(@"%p %@", handle, problem);
25742547
return GNUTLS_E_CERTIFICATE_ERROR;
25752548
}
25762549
}
25772550

2578-
gnutls_x509_crt_deinit(cert);
2579-
25802551
names = [opts objectForKey: GSTLSIssuers];
25812552
if ([names isKindOfClass: [NSArray class]])
25822553
{
25832554
if (nil == issuer || NO == [names containsObject: issuer])
25842555
{
25852556
str = [NSString stringWithFormat:
2586-
@"TLS verification: certificate's issuer does not match '%@'",
2587-
names];
2557+
@"TLS verification: issuer does not match '%@' in %@",
2558+
names, ci];
25882559
ASSIGN(problem, str);
2560+
gnutls_x509_crt_deinit(cert);
25892561
if (YES == debug) NSLog(@"%p %@", handle, problem);
25902562
return GNUTLS_E_CERTIFICATE_ERROR;
25912563
}
@@ -2597,14 +2569,16 @@ - (int) verify
25972569
if (nil == owner || NO == [names containsObject: owner])
25982570
{
25992571
str = [NSString stringWithFormat:
2600-
@"TLS verification: certificate's owner does not match '%@'",
2601-
names];
2572+
@"TLS verification: owner does not match '%@' in %@",
2573+
names, ci];
26022574
ASSIGN(problem, str);
2575+
gnutls_x509_crt_deinit(cert);
26032576
if (YES == debug) NSLog(@"%p %@", handle, problem);
26042577
return GNUTLS_E_CERTIFICATE_ERROR;
26052578
}
26062579
}
26072580

2581+
gnutls_x509_crt_deinit(cert);
26082582
return 0; // Verified
26092583
}
26102584

Tests/GNUmakefile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ check::
7272
export ADDITIONAL_LDFLAGS;\
7373
export LD_LIBRARY_PATH;\
7474
fi; \
75-
export GS_TLS_VERIFY_S=NO;\
7675
export GNUSTEP_LOCAL_ADDITIONAL_MAKEFILES;\
7776
export ADDITIONAL_INCLUDE_DIRS;\
7877
export ADDITIONAL_LIB_DIRS;\

Tests/base/NSStream/socket.m

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,15 @@ int main()
109109
{
110110
NSAutoreleasePool *arp = [NSAutoreleasePool new];
111111
NSRunLoop *rl;
112+
NSString *name;
112113
NSHost *host;
113114
Listener *li;
114115
NSDate *d;
115116

116117
rl = [NSRunLoop currentRunLoop];
117-
host = [NSHost hostWithName: @"www.google.com"];
118-
//host = [NSHost hostWithName: @"localhost"];
118+
name = @"www.google.com";
119+
//name = @"localhost";
120+
host = [NSHost hostWithName: name];
119121

120122
#if 1
121123
li = [[Listener new] autorelease];
@@ -161,6 +163,8 @@ int main()
161163
forKey: NSStreamSocketSecurityLevelKey];
162164
[defaultOutput setProperty: NSStreamSocketSecurityLevelNegotiatedSSL
163165
forKey: NSStreamSocketSecurityLevelKey];
166+
167+
[defaultOutput setProperty: name forKey: GSTLSServerName];
164168
[defaultInput open];
165169
[defaultOutput open];
166170

Tests/base/NSURL/test02.m

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,15 @@ int main()
113113
[request setHTTPBody: data];
114114
[request setHTTPMethod: @"POST"];
115115

116+
/* Our test certificate is self-signed and out of date ... don't require
117+
* the server to be verified.
118+
*/
119+
#if defined(GNUSTEP_BASE_LIBRARY)
120+
[NSURLProtocol setProperty: @"NO"
121+
forKey: GSTLSVerify
122+
inRequest: request];
123+
#endif
124+
116125
// sending the request
117126
[NSURLConnection sendSynchronousRequest: request
118127
returningResponse: &response

Tests/base/NSURLConnection/Helpers/NSURLConnectionTest.m

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,15 @@ - (void)startTest:(id)extra
109109
[s addObject: @"NSRunLoop"];
110110
}
111111

112+
/* Our test certificate is self-signed and out of date ... don't require
113+
* the server to be verified.
114+
*/
115+
#if defined(GNUSTEP_BASE_LIBRARY)
116+
[NSURLProtocol setProperty: @"NO"
117+
forKey: GSTLSVerify
118+
inRequest: _request];
119+
#endif
120+
112121
_conn = [[NSURLConnection alloc] initWithRequest: _request
113122
delegate: self];
114123

0 commit comments

Comments
 (0)