Skip to content

Commit

Permalink
Use RAII to manage lifetimes of CF object refs
Browse files Browse the repository at this point in the history
  • Loading branch information
@Nevon @hrantzsch
Nevon authored and hrantzsch committed Aug 16, 2023
1 parent d0fa42a commit fffc9c5
Showing 1 changed file with 74 additions and 34 deletions.
108 changes: 74 additions & 34 deletions src/keychain_mac.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include <vector>

#include <memory>

#include <Security/Security.h>

#include "keychain.h"
Expand Down Expand Up @@ -101,54 +103,101 @@ void updateError(keychain::Error &err, OSStatus status) {
}
}

void handleCFCreateFailure(keychain::Error &err, const std::string &errorMessage) {
void handleCFCreateFailure(keychain::Error &err,

Check warning on line 106 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L106 

Added line #L106 was not covered by tests
const std::string &errorMessage) {
err.message = errorMessage;
err.type = keychain::ErrorType::GenericError;
err.code = -1;
}

Check warning on line 111 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L108-L111 

Added lines #L108 - L111 were not covered by tests

CFStringRef createCFStringWithCString(const std::string &str, keychain::Error &err) {
CFStringRef result = CFStringCreateWithCString(kCFAllocatorDefault, str.c_str(), kCFStringEncodingUTF8);
CFStringRef createCFStringWithCString(const std::string &str,
keychain::Error &err) {
CFStringRef result = CFStringCreateWithCString(
kCFAllocatorDefault, str.c_str(), kCFStringEncodingUTF8);
if (result == NULL) {
handleCFCreateFailure(err, "Failed to create CFString");
}

Check warning on line 119 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L119 

Added line #L119 was not covered by tests
return result;
}

Check warning on line 121 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L121 

Added line #L121 was not covered by tests

CFMutableDictionaryRef createCFMutableDictionary(keychain::Error &err) {
CFMutableDictionaryRef result = CFDictionaryCreateMutable(kCFAllocatorDefault, 0, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
CFMutableDictionaryRef result =
CFDictionaryCreateMutable(kCFAllocatorDefault,
0,
&kCFTypeDictionaryKeyCallBacks,
&kCFTypeDictionaryValueCallBacks);
if (result == NULL) {
handleCFCreateFailure(err, "Failed to create CFMutableDictionary");
}
return result;
}

Check warning on line 133 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L133 

Added line #L133 was not covered by tests

CFDataRef createCFData(const std::string &data, keychain::Error &err) {
CFDataRef result = CFDataCreate(kCFAllocatorDefault, reinterpret_cast<const UInt8 *>(data.c_str()), data.length());
CFDataRef result =
CFDataCreate(kCFAllocatorDefault,
reinterpret_cast<const UInt8 *>(data.c_str()),
data.length());
if (result == NULL) {
handleCFCreateFailure(err, "Failed to create CFData");
}
return result;
}

Check warning on line 144 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L144 

Added line #L144 was not covered by tests

CFMutableDictionaryRef createQuery(const std::string &serviceName, const std::string &user, keychain::Error &err) {
CFStringRef cfServiceName = createCFStringWithCString(serviceName, err);
CFStringRef cfUser = createCFStringWithCString(user, err);
CFMutableDictionaryRef query = createCFMutableDictionary(err);
template <typename T> void CFReleaseWrapper(T ref) {
if (ref)
CFRelease(ref);
}

class AutoCFStringRef {
public:
AutoCFStringRef(const std::string &str, keychain::Error &err)
: ref(createCFStringWithCString(str, err),
CFReleaseWrapper<CFStringRef>) {}

operator CFStringRef() const { return ref.get(); }

private:
std::shared_ptr<const __CFString> ref;
};

class AutoCFMutableDictionaryRef {
public:
AutoCFMutableDictionaryRef(keychain::Error &err)
: ref(createCFMutableDictionary(err),
CFReleaseWrapper<CFMutableDictionaryRef>) {}

operator CFMutableDictionaryRef() const { return ref.get(); }

private:
std::shared_ptr<std::remove_pointer<CFMutableDictionaryRef>::type> ref;
};

class AutoCFDataRef {
public:
AutoCFDataRef(const std::string &str, keychain::Error &err)
: ref(createCFData(str, err), CFReleaseWrapper<CFDataRef>) {}

operator CFDataRef() const { return ref.get(); }

private:
std::shared_ptr<const __CFData> ref;
};

AutoCFMutableDictionaryRef createQuery(const std::string &serviceName,
const std::string &user,
keychain::Error &err) {
AutoCFStringRef cfServiceName(serviceName, err);
AutoCFStringRef cfUser(user, err);
AutoCFMutableDictionaryRef query(err);

if (err.type != keychain::ErrorType::NoError) {
if (cfServiceName) CFRelease(cfServiceName);
if (cfUser) CFRelease(cfUser);
return NULL;
return query;

Check warning on line 194 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L194 

Added line #L194 was not covered by tests
}

CFDictionaryAddValue(query, kSecClass, kSecClassGenericPassword);
CFDictionaryAddValue(query, kSecAttrAccount, cfUser);
CFDictionaryAddValue(query, kSecAttrService, cfServiceName);

CFRelease(cfServiceName);
CFRelease(cfUser);

return query;
}

Expand All @@ -161,8 +210,8 @@ void setPassword(const std::string &package, const std::string &service,
Error &err) {
err = Error{};
const auto serviceName = makeServiceName(package, service);
CFDataRef cfPassword = createCFData(password, err);
CFMutableDictionaryRef query = createQuery(serviceName, user, err);
AutoCFDataRef cfPassword(password, err);
AutoCFMutableDictionaryRef query = createQuery(serviceName, user, err);

if (err.type != keychain::ErrorType::NoError) {
return;

Check warning on line 217 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L217 

Added line #L217 was not covered by tests
Expand All @@ -174,33 +223,26 @@ void setPassword(const std::string &package, const std::string &service,

if (status == errSecDuplicateItem) {
// password exists -- override
CFMutableDictionaryRef attributesToUpdate = createCFMutableDictionary(err);
AutoCFMutableDictionaryRef attributesToUpdate(err);
if (err.type != keychain::ErrorType::NoError) {
CFRelease(cfPassword);
CFRelease(query);
return;

Check warning on line 228 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L228 

Added line #L228 was not covered by tests
}

CFDictionaryAddValue(attributesToUpdate, kSecValueData, cfPassword);
status = SecItemUpdate(query, attributesToUpdate);

CFRelease(attributesToUpdate);
}

if (status != errSecSuccess) {
updateError(err, status);
}

CFRelease(cfPassword);
CFRelease(query);
}

std::string getPassword(const std::string &package, const std::string &service,
const std::string &user, Error &err) {
err = Error{};
std::string password;
const auto serviceName = makeServiceName(package, service);
CFMutableDictionaryRef query = createQuery(serviceName, user, err);
AutoCFMutableDictionaryRef query = createQuery(serviceName, user, err);

if (err.type != keychain::ErrorType::NoError) {
return password;

Check warning on line 248 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L248 

Added line #L248 was not covered by tests
Expand All @@ -214,21 +256,21 @@ std::string getPassword(const std::string &package, const std::string &service,
if (status != errSecSuccess) {
updateError(err, status);
} else if (result != NULL) {
CFDataRef cfPassword = (CFDataRef)result;
password = std::string(reinterpret_cast<const char *>(CFDataGetBytePtr(cfPassword)), CFDataGetLength(cfPassword));
CFRelease(result);
std::unique_ptr<const __CFData, decltype(&CFRelease)> cfPassword(
(CFDataRef)result, CFRelease);
password = std::string(
reinterpret_cast<const char *>(CFDataGetBytePtr(cfPassword.get())),
CFDataGetLength(cfPassword.get()));
}

CFRelease(query);

return password;
}

void deletePassword(const std::string &package, const std::string &service,
const std::string &user, Error &err) {
err = Error{};
const auto serviceName = makeServiceName(package, service);
CFMutableDictionaryRef query = createQuery(serviceName, user, err);
AutoCFMutableDictionaryRef query = createQuery(serviceName, user, err);

if (err.type != keychain::ErrorType::NoError) {
return;

Check warning on line 276 in src/keychain_mac.cpp

View check run for this annotation

Codecov / codecov/patch

src/keychain_mac.cpp#L276 

Added line #L276 was not covered by tests
Expand All @@ -239,8 +281,6 @@ void deletePassword(const std::string &package, const std::string &service,
if (status != errSecSuccess) {
updateError(err, status);
}

CFRelease(query);
}

} // namespace keychain

0 comments on commit fffc9c5

Please sign in to comment.