Skip to content

Rework gRPC certificate loading one last time #2606

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.

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

Merged
merged 5 commits into from
Mar 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# Unreleased
- [fixed] Fixed the way gRPC certificates are loaded on macOS (#2604).

# 1.1.0
- [feature] Added `FieldValue.increment()`, which can be used in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,15 @@
"idiom" : "ipad",
"size" : "83.5x83.5",
"scale" : "2x"
},
{
"idiom" : "ios-marketing",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: are changes to this file intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. They happen any time you manipulate the macOS project. I can put this in a separate PR if you prefer.

"size" : "1024x1024",
"scale" : "1x"
}
],
"info" : {
"version" : 1,
"author" : "xcode"
}
}
}
1 change: 0 additions & 1 deletion Firestore/Example/App/macOS_example/AppDelegate.m
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ - (void)applicationDidFinishLaunching:(NSNotification *)aNotification {

// do the timestamp fix
FIRFirestoreSettings *settings = db.settings;
settings.timestampsInSnapshotsEnabled = true;
db.settings = settings;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shows up as a deprecation warning while building the macOS_example, which was the primary means by which I verified this change was correct.

I can move this into a separate cleanup grab-bag PR if you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I think that would be overkill (applies to other changes of that kind in this PR). As long as this is done on purpose, I'm fine with it being the part of the same PR.


// create a doc
Expand Down
6 changes: 1 addition & 5 deletions Firestore/Example/Firestore.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@
B9C261C26C5D311E1E3C0CB9 /* query_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = query_test.cc; sourceTree = "<group>"; };
BB92EB03E3F92485023F64ED /* Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Example_iOS_Firestore_SwiftTests_iOS.framework; sourceTree = BUILT_PRODUCTS_DIR; };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: are changes to this file intended as part of this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, these are changes that are happening whenever you touch the project.

C8522DE226C467C54E6788D8 /* mutation_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = mutation_test.cc; sourceTree = "<group>"; };
D0A6E9136804A41CEC9D55D4 /* delayed_constructor_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = delayed_constructor_test.cc; sourceTree = "<group>"; };
D0A6E9136804A41CEC9D55D4 /* delayed_constructor_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = delayed_constructor_test.cc; sourceTree = "<group>"; };
D3CC3DC5338DCAF43A211155 /* README.md */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = net.daringfireball.markdown; name = README.md; path = ../README.md; sourceTree = "<group>"; };
D5B2593BCB52957D62F1C9D3 /* perf_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = perf_spec_test.json; sourceTree = "<group>"; };
D5B25E7E7D6873CBA4571841 /* FIRNumericTransformTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FIRNumericTransformTests.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2936,8 +2936,6 @@
"\"leveldb\"",
"-framework",
"\"nanopb\"",
"-framework",
"\"openssl\"",
"-ObjC",
);
PRODUCT_BUNDLE_IDENTIFIER = "com.google.Firestore-macOS";
Expand Down Expand Up @@ -3012,8 +3010,6 @@
"\"leveldb\"",
"-framework",
"\"nanopb\"",
"-framework",
"\"openssl\"",
"-ObjC",
);
PRODUCT_BUNDLE_IDENTIFIER = "com.google.Firestore-macOS";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@

#include "Firestore/core/src/firebase/firestore/remote/grpc_root_certificate_finder.h"

#import <objc/runtime.h>

#include <string>

#include "Firestore/core/src/firebase/firestore/util/filesystem.h"
#include "Firestore/core/src/firebase/firestore/util/hard_assert.h"
#include "Firestore/core/src/firebase/firestore/util/log.h"
#include "Firestore/core/src/firebase/firestore/util/statusor.h"

#import "Firestore/Source/Core/FSTFirestoreClient.h"
#include "absl/strings/str_cat.h"

namespace firebase {
namespace firestore {
Expand All @@ -34,45 +35,106 @@
using util::StatusOr;
using util::StringFormat;

namespace {

/**
* Finds the roots.pem certificate file in the given resource bundle and logs
* the outcome.
*
* @param bundle The bundle to check. Can be a nested bundle in Resources or
* an app or framework bundle to look in directly.
* @param parent The parent bundle of the bundle to search. Used for logging.
*/
NSString* _Nullable FindCertFileInResourceBundle(NSBundle* _Nullable bundle,
NSBundle* _Nullable parent) {
if (!bundle) return nil;

NSString* path = [bundle pathForResource:@"roots" ofType:@"pem"];
if (util::LogIsDebugEnabled()) {
std::string message =
absl::StrCat("roots.pem ", path ? "found " : "not found ", "in bundle ",
util::MakeString([bundle bundleIdentifier]));
if (parent) {
absl::StrAppend(&message, " (in parent ",
util::MakeString([parent bundleIdentifier]), ")");
}
LOG_DEBUG("%s", message);
}

return path;
}

/**
* Finds gRPCCertificates.bundle inside the given parent, if it exists.
*
* This function exists mostly to handle differences in platforms.
* On iOS, resources are nested directly within the top-level of the parent
* bundle, but on macOS this will actually be in Contents/Resources.
*
* @param parent A framework or app bundle to check.
* @return The nested gRPCCertificates.bundle if found, otherwise nil.
*/
NSBundle* _Nullable FindCertBundleInParent(NSBundle* _Nullable parent) {
if (!parent) return nil;

NSString* path = [parent pathForResource:@"gRPCCertificates"
ofType:@"bundle"];
if (!path) return nil;

return [[NSBundle alloc] initWithPath:path];
}

NSBundle* _Nullable FindFirestoreFrameworkBundle() {
// Load FIRFirestore reflectively to avoid a circular reference at build time.
Class firestore_class = objc_getClass("FIRFirestore");
if (!firestore_class) return nil;

return [NSBundle bundleForClass:firestore_class];
}

/**
* Finds the path to the roots.pem certificates file, wherever it may be.
*
* Carthage users will find roots.pem inside gRPCCertificates.bundle in
* the main bundle.
*
* There have been enough variations and workarounds posted on this that
* this also accepts the roots.pem file outside gRPCCertificates.bundle.
*/
NSString* FindPathToCertificatesFile() {
// Certificates file might be present in either the gRPC-C++ bundle or (for
// Certificates file might be present in either the gRPC-C++ framework or (for
// some projects) in the main bundle.
NSBundle* bundles[] = {
// Try to load certificates bundled by gRPC-C++.
// CocoaPods: try to load from the gRPC-C++ Framework.
[NSBundle bundleWithIdentifier:@"org.cocoapods.grpcpp"],
// Users manually adding resources to the project may add the
// certificate to the main application bundle. Note that `mainBundle` is
// nil for unit tests of library projects, so it cannot fully substitute
// for checking the framework bundle.

// Carthage: try to load from the FirebaseFirestore.framework
FindFirestoreFrameworkBundle(),

// Carthage and manual projects: users manually adding resources to the
// project may add the certificate to the main application bundle. Note
// that `mainBundle` is nil for unit tests of library projects.
[NSBundle mainBundle],
};

// search for the roots.pem file in each of these resource locations
NSString* possibleResources[] = {
@"gRPCCertificates.bundle/roots",
@"roots",
};
NSString* path = nil;

for (NSBundle* bundle : bundles) {
if (!bundle) {
continue;
}
for (NSBundle* parent : bundles) {
if (!parent) continue;

for (NSString* resource : possibleResources) {
NSString* path = [bundle pathForResource:resource ofType:@"pem"];
if (path) {
LOG_DEBUG("%s.pem found in bundle %s", resource,
[bundle bundleIdentifier]);
return path;
} else {
LOG_DEBUG("%s.pem not found in bundle %s", resource,
[bundle bundleIdentifier]);
}
}
NSBundle* certs_bundle = FindCertBundleInParent(parent);
path = FindCertFileInResourceBundle(certs_bundle, parent);
if (path) break;

path = FindCertFileInResourceBundle(parent, nil);
if (path) break;
}
return nil;

return path;
}

} // namespace

std::string LoadGrpcRootCertificate() {
NSString* path = FindPathToCertificatesFile();
HARD_ASSERT(
Expand Down