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.

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

Add support for privileged and unprivileged connections #4

Conversation

@alessandrogario
Copy link
Contributor

@alessandrogario alessandrogario commented Jul 2, 2018

This PR adds support for separating the interface into privileged and unprivileged, depending on the permissions of the connecting client

/**
Enables or disables the connection from unprivileged users. (server)
*/
- (void)allowUnprivilegedClients:(BOOL)enable;

This comment has been minimized.

@russellhancox

russellhancox Jul 2, 2018
Contributor

I think this property can be removed and the presence of unprivilegedExportedInterface can be used to infer this. I'll elaborate more below.

*/
@property(retain, nullable) NSXPCInterface *exportedInterface;
@property(retain, nullable) NSXPCInterface *unprivilegedExportedInterface;

This comment has been minimized.

@russellhancox

russellhancox Jul 2, 2018
Contributor

This interface is nice and clean but to avoid breaking upgrading clients, I'd like to leave the exportedInterface property around for a release or two. Could you add it back and mark it as deprecated, like so:

/**
  Old interface property, please update to use privilegedExportedInterface and/or unprivilegedExportedInterface instead.
*/
@property(retain, nullable) NSXPCInterface *exportedInterface __attribute__((
   deprecated(Use privilegedExportedInterface and / or unprivilegedExportedInterface instead.)));
fallback_interface = self.privilegedExportedInterface;
}

NSXPCInterface *userInterface = (preferred_interface ? preferred_interface : fallback_interface);

This comment has been minimized.

@russellhancox

russellhancox Jul 2, 2018
Contributor

I think this could be simpler and more explicit about which interface to export; if we remove the allowUnprivilegedClients property as I suggested above and require that the right interface is used for privileged/unprivileged clients, this can become:

NSXPCInterface *interface;
if ([connection effectiveUserIdentifier] == 0) {
  interface = self.privilegedInterface;
} else {
  interface = self.unprivilegedInterface;
}

// TODO(any): Remove 1-2 releases after exportedInterface was marked deprecated.
if (!interface) {
  interface = self.exportedInterface;
}

if (!interface) return NO;

.....

connection.exportedInterface = interface;

Clients which then want the same interface exported for both privileged and unprivileged clients can use:
conn.privilegedExportedInterface = conn.unprivilegedExportedInterface = myNewInterface;

And if the appropriate interface is unset (and assuming exportedInterface isn't being used) the connection will be rejected.

@alessandrogario
Copy link
Contributor Author

@alessandrogario alessandrogario commented Jul 2, 2018

Hello @russellhancox!

Thanks for reviewing the PR! It's a lot better now with your corrections 👍

I also updated the tests by adding a dummy protocol because we are now failing the connection if there is no interface set.

Copy link
Contributor

@russellhancox russellhancox left a comment

Thanks again!

@russellhancox russellhancox merged commit 4790f8c into google:master Jul 2, 2018
1 check passed
1 check passed
cla/google All necessary CLAs are signed
@russellhancox
Copy link
Contributor

@russellhancox russellhancox commented Jul 2, 2018

This has been published in v1.2 and pushed to CocoaPods. A 'pod update' in dependent projects will pick this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.