Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle certificates that don't have common name set #106

Merged
merged 2 commits into from
Mar 28, 2024
Merged

Handle certificates that don't have common name set #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e5fecc
Handle certificates that don't have common name set
johankool Mar 21, 2024
e6b57f7
Update fallback discovery
johankool Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 29 additions & 24 deletions geteduroam/GeteduroamPackage/Sources/EAPConfigurator/EAPConfigurator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,22 @@ class EAPConfigurator {
return certificates
}


private func label(for certificateRef: SecCertificate) throws -> String {
var commonNameRef: CFString?
let status: OSStatus = SecCertificateCopyCommonName(certificateRef, &commonNameRef)
if status == errSecSuccess {
return commonNameRef! as String
}

guard let rawSubject = SecCertificateCopyNormalizedSubjectSequence(certificateRef) as? Data else {
Logger.eap.error("addCertificate: unable to get common name or subject sequence from certificate")
throw EAPConfiguratorError.failedToCopyCommonNameOrSubjectSequence
}

return rawSubject.base64EncodedString(options: [])
}

/**
@function addCertificate
@abstract Import Base64 encoded DER to keychain.
Expand All @@ -505,26 +521,20 @@ class EAPConfigurator {
throw EAPConfiguratorError.failedToCreateCertificateFromData
}

var commonNameRef: CFString?
var status: OSStatus = SecCertificateCopyCommonName(certificateRef, &commonNameRef)
guard status == errSecSuccess else {
Logger.eap.error("addCertificate: unable to get common name")
throw EAPConfiguratorError.failedToCopyCommonName
}
let commonName: String = commonNameRef! as String
Logger.eap.info("addCertificate: adding common name \(commonName)")

let label = try label(for: certificateRef)
Logger.eap.info("addCertificate: adding \(label)")

let addquery: [String: Any] = [
kSecClass as String: kSecClassCertificate,
kSecValueRef as String: certificateRef,
kSecAttrLabel as String: commonName,
kSecAttrLabel as String: label,
kSecReturnRef as String: kCFBooleanTrue!,
kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock,
//kSecReturnPersistentRef as String: kCFBooleanTrue!, // Persistent refs cause an error (invalid EAP config) when installing the profile
kSecAttrAccessGroup as String: "\(teamID).com.apple.networkextensionsharing"
]
var item: CFTypeRef?
status = SecItemAdd(addquery as CFDictionary, &item)
var status = SecItemAdd(addquery as CFDictionary, &item)

// TODO: remove this, and always use the "failsafe"?
if status == errSecSuccess && item != nil {
Expand All @@ -544,7 +554,7 @@ class EAPConfigurator {

let getquery: [String: Any] = [
kSecClass as String: kSecClassCertificate,
kSecAttrLabel as String: commonName,
kSecAttrLabel as String: label,
kSecReturnRef as String: kCFBooleanTrue!,
//kSecReturnPersistentRef as String: kCFBooleanTrue!, // Persistent refs cause an error (invalid EAP config) when installing the profile
kSecAttrAccessGroup as String: "\(teamID).com.apple.networkextensionsharing"
Expand Down Expand Up @@ -608,30 +618,25 @@ class EAPConfigurator {
// If we don't do this, we get "failed to find the trust chain for the client certificate" when connecting
for certificate in chain {
let certificateRef: SecCertificate = certificate as SecCertificate
var commonNameRef: CFString?
var status: OSStatus = SecCertificateCopyCommonName(certificateRef, &commonNameRef)
guard status == errSecSuccess else {
Logger.eap.error("addClientCertificate: unable to get common name")
continue
}
let commonName: String = commonNameRef! as String
Logger.eap.info("addClientCertificate: adding common name \(commonName)")

let label = try label(for: certificateRef)
Logger.eap.info("addClientCertificate: adding \(label)")

let addquery: [String: Any] = [
kSecClass as String: kSecClassCertificate,
kSecValueRef as String: certificate,
kSecAttrLabel as String: commonName,
kSecAttrLabel as String: label,
kSecAttrAccessible as String: kSecAttrAccessibleAfterFirstUnlock,
//kSecReturnRef as String: kCFBooleanTrue!, // We're not retrieving the reference at this point, 2nd argument to SecItemAdd is nil
//kSecReturnPersistentRef as String: kCFBooleanTrue!, // Persistent refs cause an error (invalid EAP config) when installing the profile
kSecAttrAccessGroup as String: "\(teamID).com.apple.networkextensionsharing"
]

status = SecItemAdd(addquery as CFDictionary, nil)
let status = SecItemAdd(addquery as CFDictionary, nil)

guard status == errSecSuccess || status == errSecDuplicateItem else {
Logger.eap.error("addClientCertificate: SecItemAdd: \(commonName, privacy: .public) \(String(status), privacy: .public)")
throw EAPConfiguratorError.failedSecItemAdd(status, commonName: commonName)
Logger.eap.error("addClientCertificate: SecItemAdd: \(label, privacy: .public) \(String(status), privacy: .public)")
throw EAPConfiguratorError.failedSecItemAdd(status, label: label)
}
}

Expand Down
14 changes: 7 additions & 7 deletions geteduroam/GeteduroamPackage/Sources/EAPConfigurator/EAPConfiguratorError.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public enum EAPConfiguratorError: Error {
case failedSecPKCS12Import(OSStatus)

/// Unable to add certificate to keychain
case failedSecItemAdd(OSStatus, commonName: String? = nil)
case failedSecItemAdd(OSStatus, label: String? = nil)

/// Unable to copy from keychain
case failedSecItemCopyMatching(OSStatus)
Expand All @@ -35,8 +35,8 @@ public enum EAPConfiguratorError: Error {
/// Unable to create certificate from data
case failedToCreateCertificateFromData

/// Unable to get common name from certificate
case failedToCopyCommonName
/// Unable to get common name or subject sequence f from certificate
case failedToCopyCommonNameOrSubjectSequence

/// No valid configuration found
case noConfigurations
Expand Down Expand Up @@ -78,8 +78,8 @@ extension EAPConfiguratorError: LocalizedError {
case let .failedSecPKCS12Import(status):
return String(format: NSLocalizedString("Unable to import certificate into keychain (%d)", bundle: .module, comment: "addClientCertificate"), status)

case let .failedSecItemAdd(status, commonName):
return String(format: NSLocalizedString("Unable to add certificate %@ to keychain (%d)", bundle: .module, comment: "addClientCertificate: SecItemAdd"), commonName ?? NSLocalizedString("with unknown name", bundle: .module, comment: "with unknown name"), status)
case let .failedSecItemAdd(status, label):
return String(format: NSLocalizedString("Unable to add certificate %@ to keychain (%d)", bundle: .module, comment: "addClientCertificate: SecItemAdd"), label ?? NSLocalizedString("with unknown name", bundle: .module, comment: "with unknown name"), status)

case let .failedSecItemCopyMatching(status):
return String(format: NSLocalizedString("Unable to copy from keychain (%d)", bundle: .module, comment: "SecItemCopyMatching: retrieving identity returned"), status)
Expand All @@ -90,8 +90,8 @@ extension EAPConfiguratorError: LocalizedError {
case .failedToCreateCertificateFromData:
return NSLocalizedString("Unable to create certificate from data", bundle: .module, comment: "SecCertificateCreateWithData: false")

case .failedToCopyCommonName:
return NSLocalizedString("Unable to get common name from certificate", bundle: .module, comment: "Certificate: unable to get common name")
case .failedToCopyCommonNameOrSubjectSequence:
return NSLocalizedString("Unable to get common name from certificate", bundle: .module, comment: "Certificate: unable to get common name or subject sequence ")

case .noConfigurations:
return NSLocalizedString("No valid configuration found", bundle: .module, comment: "noConfigurations")
Expand Down
8 changes: 7 additions & 1 deletion geteduroam/GeteduroamPackage/Sources/EAPConfigurator/Resources/Localizable.xcstrings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@
"No password in configuration" : {
"comment" : "empty pass",
"localizations" : {
"en-GB" : {
"stringUnit" : {
"state" : "translated",
"value" : "No password in configuration"
}
},
"nl" : {
"stringUnit" : {
"state" : "translated",
Expand Down Expand Up @@ -779,7 +785,7 @@
}
},
"Unable to get common name from certificate" : {
"comment" : "Certificate: unable to get common name",
"comment" : "Certificate: unable to get common name or subject sequence ",
"localizations" : {
"ar" : {
"stringUnit" : {
Expand Down
2 changes: 1 addition & 1 deletion geteduroam/discovery.json
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion getgovroam/discovery.json
View file
Open in desktop

Large diffs are not rendered by default.