Skip to content

Commit 3637f40

Browse files
committed
crypto: fix macOS trust settings default for absent kSecTrustSettingsResult
When kSecTrustSettingsResult is absent from a trust settings dictionary, Apple specifies kSecTrustSettingsResultTrustRoot as the default value. Previously, the trust result evaluation (deny check, self-issued check, TrustAsRoot check) was inside the block that only executed when kSecTrustSettingsResult was explicitly present. When the key was absent, the function fell through to return UNSPECIFIED, incorrectly rejecting self-signed certificates that should have been trusted via the default. Move the trust result evaluation outside the conditional block so the default value of kSecTrustSettingsResultTrustRoot flows through the same code path as explicit values. This aligns with Chromium's trust_store_mac.cc implementation.
1 parent 86282b5 commit 3637f40

File tree

1 file changed

+33
-27
lines changed

1 file changed

+33
-27
lines changed

src/crypto/crypto_context.cc

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -386,35 +386,41 @@ TrustStatus IsTrustDictionaryTrustedForPolicy(CFDictionaryRef trust_dict,
386386
&trust_settings_result)) {
387387
return TrustStatus::UNSPECIFIED;
388388
}
389-
390-
if (trust_settings_result == kSecTrustSettingsResultDeny) {
391-
return TrustStatus::DISTRUSTED;
392-
}
393-
394-
// This is a bit of a hack: if the cert is self-issued allow either
395-
// kSecTrustSettingsResultTrustRoot or kSecTrustSettingsResultTrustAsRoot on
396-
// the basis that SecTrustSetTrustSettings should not allow creating an
397-
// invalid trust record in the first place. (The spec is that
398-
// kSecTrustSettingsResultTrustRoot can only be applied to root(self-signed)
399-
// certs and kSecTrustSettingsResultTrustAsRoot is used for other certs.)
400-
// This hack avoids having to check the signature on the cert which is slow
401-
// if using the platform APIs, and may require supporting MD5 signature
402-
// algorithms on some older OSX versions or locally added roots, which is
403-
// undesirable in the built-in signature verifier.
404-
if (is_self_issued) {
405-
return trust_settings_result == kSecTrustSettingsResultTrustRoot ||
406-
trust_settings_result == kSecTrustSettingsResultTrustAsRoot
407-
? TrustStatus::TRUSTED
408-
: TrustStatus::UNSPECIFIED;
409-
}
410-
411-
// kSecTrustSettingsResultTrustAsRoot can only be applied to non-root certs.
412-
return (trust_settings_result == kSecTrustSettingsResultTrustAsRoot)
413-
? TrustStatus::TRUSTED
414-
: TrustStatus::UNSPECIFIED;
415389
}
416390

417-
return TrustStatus::UNSPECIFIED;
391+
// When kSecTrustSettingsResult is absent from the trust dict,
392+
// Apple docs specify kSecTrustSettingsResultTrustRoot as the default.
393+
// Refs https://github.com/apple-oss-distributions/Security/blob/db15acbe6a7f257a859ad9a3bb86097bfe0679d9/trust/headers/SecTrustSettings.h#L119-L122
394+
// This is also enforced at write time for self-signed certs get TrustRoot,
395+
// and non-self-signed certs cannot have an empty settings,
396+
// Refs https://github.com/apple-oss-distributions/Security/blob/db15acbe6a7f257a859ad9a3bb86097bfe0679d9/OSX/sec/Security/SecTrustStore.c#L196-L207
397+
398+
if (trust_settings_result == kSecTrustSettingsResultDeny) {
399+
return TrustStatus::DISTRUSTED;
400+
}
401+
402+
// From https://source.chromium.org/chromium/chromium/src/+/main:net/cert/internal/trust_store_mac.cc;l=144-146
403+
// This is a bit of a hack: if the cert is self-issued allow either
404+
// kSecTrustSettingsResultTrustRoot or kSecTrustSettingsResultTrustAsRoot on
405+
// the basis that SecTrustSetTrustSettings should not allow creating an
406+
// invalid trust record in the first place. (The spec is that
407+
// kSecTrustSettingsResultTrustRoot can only be applied to root(self-signed)
408+
// certs and kSecTrustSettingsResultTrustAsRoot is used for other certs.)
409+
// This hack avoids having to check the signature on the cert which is slow
410+
// if using the platform APIs, and may require supporting MD5 signature
411+
// algorithms on some older OSX versions or locally added roots, which is
412+
// undesirable in the built-in signature verifier.
413+
if (is_self_issued) {
414+
return (trust_settings_result == kSecTrustSettingsResultTrustRoot ||
415+
trust_settings_result == kSecTrustSettingsResultTrustAsRoot)
416+
? TrustStatus::TRUSTED
417+
: TrustStatus::UNSPECIFIED;
418+
}
419+
420+
// kSecTrustSettingsResultTrustAsRoot can only be applied to non-root certs.
421+
return (trust_settings_result == kSecTrustSettingsResultTrustAsRoot)
422+
? TrustStatus::TRUSTED
423+
: TrustStatus::UNSPECIFIED;
418424
}
419425

420426
TrustStatus IsTrustSettingsTrustedForPolicy(CFArrayRef trust_settings,

0 commit comments

Comments
 (0)