openid / openyolo-android Goto Github PK
View Code? Open in Web Editor NEWAndroid protocol for credential exchange and update - "You Only Login Once"
Home Page: http://openid.net/wg/ac/
License: Apache License 2.0
Android protocol for credential exchange and update - "You Only Login Once"
Home Page: http://openid.net/wg/ac/
License: Apache License 2.0
With the version 0.1.1, the pom file provided the dependencies, it's not the case anymore on 0.2.
That means a "client" will need to include openyolo-api:0.2.0
, but also bbq:0.2.0
, openyolo-protocol:0.2.0
, support-annotations
โฆ
Any reason why we got rid of this?
OpenYolo API 0.2.0:
<?xml version="1.0" encoding="UTF-8"?>
<project xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<modelVersion>4.0.0</modelVersion>
<groupId>org.openyolo</groupId>
<artifactId>openyolo-api</artifactId>
<version>0.2.0</version>
<packaging>aar</packaging>
<name>OpenYOLO for Android Client API</name>
<url>https://github.com/openid/OpenYOLO-Android</url>
<licenses>
<license>
<name>The Apache Software License, Version 2.0</name>
<url>http://www.apache.org/licenses/LICENSE-2.0.txt</url>
<distribution>repo</distribution>
</license>
</licenses>
<developers>
<developer>
<id>iainmcgin</id>
<name>Iain McGinniss</name>
<email>[email protected]</email>
</developer>
</developers>
<scm>
<connection>https://github.com/openid/OpenYOLO-Android.git</connection>
<developerConnection>https://github.com/openid/OpenYOLO-Android.git</developerConnection>
<url>https://github.com/openid/OpenYOLO-Android</url>
</scm>
</project>
OpenYolo API 0.1.1:
<?xml version="1.0" encoding="UTF-8"?>
<project xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd" xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<modelVersion>4.0.0</modelVersion>
<groupId>org.openyolo</groupId>
<artifactId>openyolo-api</artifactId>
<version>0.1.1</version>
<packaging>aar</packaging>
<name>OpenYOLO for Android Client API</name>
<url>https://github.com/openid/OpenYOLO-Android</url>
<licenses>
<license>
<name>The Apache Software License, Version 2.0</name>
<url>http://www.apache.org/licenses/LICENSE-2.0.txt</url>
<distribution>repo</distribution>
</license>
</licenses>
<developers>
<developer>
<id>iainmcgin</id>
<name>Iain McGinniss</name>
<email>[email protected]</email>
</developer>
</developers>
<scm>
<connection>https://github.com/openid/OpenYOLO-Android.git</connection>
<developerConnection>https://github.com/openid/OpenYOLO-Android.git</developerConnection>
<url>https://github.com/openid/OpenYOLO-Android</url>
</scm>
<dependencies>
<dependency>
<groupId>org.openyolo</groupId>
<artifactId>bbq</artifactId>
<version>0.1.1</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.openyolo</groupId>
<artifactId>openyolo-protocol</artifactId>
<version>0.1.1</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>com.android.support</groupId>
<artifactId>support-annotations</artifactId>
<version>25.3.1</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>com.android.support</groupId>
<artifactId>appcompat-v7</artifactId>
<version>25.3.1</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>com.android.support</groupId>
<artifactId>support-vector-drawable</artifactId>
<version>25.3.1</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<version>1.3</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>org.valid4j</groupId>
<artifactId>valid4j</artifactId>
<version>0.5.0</version>
<scope>compile</scope>
</dependency>
</dependencies>
</project>
Tracking bug to capture when all current providers are ready for us to remove the backwards compatibility with BBQ retrieve intents.
Droidcon London is around the corner and I was planning to cut an up to date release for the event. My plan was to cut the release once PR #94 had landed, but wanted to check with everyone before doing so.
I see an exception thrown by validation that appears to be incorrectly handling results that do not include a Hint value, such as when the user cancels the Hint flow.
src - https://github.com/openid/OpenYOLO-Android/blob/master/api/proguard-rules.txt
isn't this too broad?
-keep class org.hamcrest.** { *; }
Centralize all the resolution and intent creation code into ProviderResolver, where it is easier to test.
Our libraries depend a couple 26.x.y version of the support library (e.g. com.android.support:appcompat-v7:26.0.2
.) AFAICT these versions are no longer served via the Android Studio's SDK Manager and instead can be retrieved by adding the Google maven repository. The getting started guide should be updated to include this.
protobuf.gradle#makeExecutable's Files.setPosixFilePermissions appears to not be supported on Windows.
def makeExecutable = { file ->
Files.setPosixFilePermissions(
file.toPath(),
[
PosixFilePermission.OWNER_EXECUTE,
PosixFilePermission.GROUP_EXECUTE
].toSet())
}
In order to be generic and extensible, the OpenYOLO model types (Credential, etc) have additional property maps that map string keys to byte arrays. We anticipate that the most common case for usage of these additional property maps will be to map strings to strings - as such, we should add utility methods for this case on the builder and model types:
class ModelType {
// ...
/**
* Returns a String value for the specified additional property, if it exists. String value are expected
* to be encoded using UTF-8.
*/
public String getStringAdditionalProperty(
@NonNull String additionalPropertyKey) { /* ... */ }
public static final Builder {
// ...
/**
* Adds a string valued additional property. The value is encoded using UTF-8.
*/
public void setStringAdditionalProperty(
@NonNull String key,
@Nullable String value) { /* ... */ }
}
}
public final class AdditionalPropertiesUtil {
/**
* Encodes a string using UTF-8 to a byte array, for storage in an additional property map.
* If the string is null, then null will be returned.
*/
@Nullable
public byte[] encodeStringValue(@Nullable String value);
}
When I tried to integrate lib build failed since OpenYOLO uses Java 8, to use Java 8 I need to migrate my app to Gradle 3.0 plugin which has its own setup process, maybe bit relaxed requirement of Java is good for easy integration.
Android O introduced the Autofill framework. With it forms may be automatically filled in via an inline UX as seen in modern web browsers and an save dialog may be shown. This can lead to a poor experience in apps that use OpenYOLO if unplanned for. The two interactions that stand out to me are:
We should decide on a standard recommendation for both clients and providers, document these recommendations and implement them if possible.
Some of my thoughts on this are:
Overall I think telling clients "Mark fields as not important for Autofill" on their account creation/sign in is an easy message that should cover both issues. We could even add a method to the client that would do this for clients (e.g. CredentialClient.disableAutofill(Activity activity). In addition we could also add some helpers to enable OpenYOLO provider that are also Autofill providers handle 2.b.
What do you guys think?
As seen in iainmcgin@'s representative demo app (PR #69) when a client is performing an account creation flow they may want to convert the returned hint into a credential. We should offer a convenience transformer for this operation as it appears it this may be a common pattern.
The CredentialRetrieveRequest proto documents that at least one authentication method is required while it's associated RetrieveRequest class documents the absence of any authentication methods should be treated as a wildcard.
I suggest that the field should be required and that the wildcard is made explicit (For example openyolo://any
). What does everyone else think?
Based on our own (Google's) internal security guidelines, we should treat security bugs in client code as an inevitability and plan accordingly. When shipping our code as AARs that is compiled into client apps, it is infeasible to "remote update" the client library, without introducing a whole host of other possible security threats to integrators of the library.
My proposal for dealing with this situation is as follows:
Integrate a "kill switch" into the client library. This operates by retrieving a configuration file from an OpenID Foundation controlled domain, e.g. "www.openyolo.org". This configuration file will contain a blacklist of client library versions for which we have found security bugs. If the client library matches any of these versions, it switches to a "no-op" mode of operation where all requests fail, as though there were no credential providers on the device.
Set up an "openyolo-security-announce" mailing list where we can broadcast the inclusion of versions in the blacklist, and notify users of the library to update their apps with patched versions of the library as appropriate.
The technical details of implementing the kill switch are still a little unclear to me, I believe we should be able to retain the client library version by using a custom BuildConfig field, though I will need to experiment with that.
It is also possible that users of the library won't like having a dependency on an external configuration file that can disable functionality in this way. So, we may need to make this opt-out, but with glaring security warnings about doing so. Those who opt out would have to be extra vigilant about monitoring the announce list and updating their apps.
Thoughts?
Well written demo app with recent best practices however when I wants to see a sample piece to integrate with my app or how it works having these extra bells and whistles is deviating from my main intention.
A large library size may deter some from integrating. It would be great to have automatic check during pull requests that calculate and record the our size as well as gate code changes that regress too much.
Looks like a simple fix:
private Builder setHintFromProto(@Nullable Protobufs.Hint hint) {
mHint = (hint != null) ? Hint.fromProtobuf(hint) : null;
return this;
}
This protobuf runtime implementation returns a default implementation for nested messages. The conditional check above needs to be modified to check against this rather than null.
With the addition of the result messages and result codes it is now possible to provide the result code in each test page's response section. This will provide more specific information and removes the difficulty of reading the result toast before it disappears.
Delete was specified as part of spec draft 3.
PR #94 introduces some Google specific branding into the client module as a quick fix before the Droidcon London release. This should be solved at the protocol level and ideally this provider specific branding/logic should be removed.
... for consistency with the other method names.
We are currently using camel case field names (from spec draft 2). Change the protos to spec draft 3 form.
Currently the OpenYolo specification allows clients to request credentials that are disjoint from the caller's authentication domain.
The set of authentication domains specified in the request may be disjoint from the set of domains known to be related to the requester (via its package name): such requests would allow trusted intermediaries (e.g. keyboard apps, mail clients, etc.) to request credentials that they do not directly own. It is the responsibility of the credential provider to determine whether to permit such requests. Typically, requests should only be permitted for an authentication domain that is provably associated with to the requester, or if the app has been explicitly whitelisted (by the user or provider) to request credentials outside its equivalence class.
Remove the ability for clients to request a set of authentication domains in credential retrieve requests. In its place only the implied authentication domain as determined by the caller's package and signature must be used.
Interested in everyone's thoughts. Is there a motivating use case to support it?
The validating wrapper types we have defined in protocol should all following a general pattern:
A Builder constructor should exist, which takes a protocol buffer. This should throw unchecked exceptions if the protocol buffer is semantically invalid.
A fromProtobuf
method (and utility variants) should throw checked exceptions if the protocol buffer is semantically invalid.
This will allow code paths that are accepting potentially invalid data to use fromProtobuf
and catch the exception for invalid data, and recover appropriately.
After adding a fix for #49 I noticed that we currently do not have any unit tests for CredentialRetrieveResult.
Builder classes use Valid4j's require API which throws Errors when it's validation conditions are not met.
While this is allows would be fine for client implementations which should only be using constant values when constructing builders this puts credential providers in the position where they must catch Errors to prevent malformed requests from crashing their application.
Relax our validation to allow credential providers to recover from malformed requests without catching Errors. Switching to throwing run-time exceptions and catching them in our SPI base classes allows for the best of both worlds.
Currently the hint and save API methods will display a "provider selector" if more than one provider is available on the device, or if the sole installed provider is not on the known provider list. This step is frustratingly repetitive. The spec includes a description of a proposed API to store a pointer to the user's preferred credential provider, which would allow bypassing this step when a preference has been set.
We are not sure exactly what release of Google Play Services we will be able to release this in, and in the interim I would like to define an improved heuristic that will minimize the impact of not having it. The heuristic would behave as follows: if there are two known providers, and one of them is Google, use the other one.
The justification for this is that the Google provider is in the privileged position of being installed on the vast majority of Android devices - for most users, it is not a conscious choice to have Smart Lock for Passwords on their device. However, if a second provider is installed, this is almost certainly a conscious choice: installing 1Password generally implies that you want to use that for password management, and therefore you would also want that to be used for generating and saving credentials.
This heuristic does not change the behavior of retrieve, which still uses BBQ to query credential availability in all installed providers. So, if a credential exists in Smart Lock for Passwords, this would still be presented as an option on retrieve.
We may consider generalizing this proposal to "preinstalled" vs "manually installed" providers. However, that will hopefully not be necessary, as the explicit preference API will eventually land in Play Services and provide a better solution to the problem overall.
@dxslly recommends this as a cleaner pattern, to cut down on the number of params passed to the hidden constructor of our validating types.
Flow tested
Try the above steps with dashlane and lastpass, but after 6th step its not picking the previously saved password, i verified data in each of the app properly still not auto-logging in.
To help 3P client developers get started quickly we should distribute our client library in a way that allows developers to use Gradle's dependency management to import OpenYOLO.
http://openid.net/specs/openyolo-android.html#credential-saving
I've been doing another pass of security review over the code and protocol and have spotted a potential security vulnerability for older Android devices using OpenYOLO. This relies on exploitation of deserialization bugs, such as CVE-2015-3837, where deserialization of Intent extras leads to an injection attack. This particular bug affects Android 5.1.1 and lower, if the fix has not been specifically back-ported by the manufacturer. It seems unlikely that this is the only deserialization bug in Android, so it's worth considering how to mitigate this class of attacks entirely.
During the BBQ phase of the retrieve process, providers respond to requests for credentials by sending a serialized Intent if they have credentials available. At this phase, any app can respond, regardless of status as a "known" credential provider. As such, a malicious app could respond to a BBQ query for a credential and provide an Intent crafted to trigger a deserialization bug, potentially resulting in remote code execution as is the case in CVE-2015-3837.
According to Roee Hay's talk on Android serialization vulnerabilities at RSAConf 2016, most devices are now patched against CVE-2015-3837. However, as other deserialization bugs are possible, it makes sense to pursue mitigation where it does not disrupt the core functionality of OpenYOLO.
The protocol was designed to allow returning an Intent, so that the provider could potentially include some pre-loaded information that would help in handling the subsequent Intent invocation to retrieve the credential. This extra information is of little interest to the client; as such, an alternative approach is possible:
This completely avoids the need to deserialize data in the client, removing this class of exploit.
As far as I'm aware, none of the providers are currently returning extra data in the Intents they return via BBQ, so it should be possible to make the necessary change in the client library right now without breakage. The protocol spec and spi should also be updated to clarify the nature of the data returned via the broadcast stage of credential retrieval.
It seems plausible that CVE-2015-3837 and similar deserialization bugs could affect providers too, where a malicious client sends an Intent with extra data intended to execute code within the context of the provider. It's not clear to me if there is anything specific that a provider could to to protect itself from this vulnerability, which would also apply if any of its other exported activities reads received Intent extras. One approach to protect against loading unexpected data could be to override the class loader for received intents, via
Intent.setExtrasClassLoader,
such that only a whitelisted set of classes may be used during deserialization. I haven't verified that this approach works, but it is worth further exploration.
The purpose of multiple Signatures attached to an Android app is poorly defined, both as to when multiple signatures can be present, and how these should be interpreted:
In practice, the Play Store only allows a single signature. The current Smart Lock for Passwords implementation makes this assumption, and therefore only uses the first signature for the app's identity. We should formalize this within the spec and implementation.
CredentialClient.getSaveIntent() appears to not find credential providers that support saving all credentials. As defined in the spec:
If the data filter is omitted, this indicates the provider supports saving all credentials, regardless of authentication type.
The current implementation will only find credential providers that use data filters.
Flow exits below, i.e no password, but no password filed, even error message password too short is not displayed
if (userPassword.length() < MIN_PASSWORD_LENGTH) { passwordError.set(getResourceString(R.string.error_password_too_short)); return; }
This change makes the demo workable
I am confident that our use of drawables on the textview is responsible. The test devices is pre-L and our support library version is 25.3.1. Based on this blog post from the Android dev team (https://plus.google.com/+AndroidDevelopers/posts/iTDmFiGrVne) support for vector drawables from resources on pre-Lollipop devices has been removed.
05-31 09:32:11.445 768 2422 I ActivityManager: Start proc org.openyolo.testapp for activity org.openyolo.testapp/.OpenYoloTestActivity: pid=20254 uid=10076 gids={50076, 3003}
05-31 09:33:20.165 768 1129 I ActivityManager: START u0 {cmp=org.openyolo.testapp/org.openyolo.api.ui.ProviderPickerActivity (has extras)} from pid 20254
05-31 09:33:20.265 20254 20254 E AndroidRuntime: FATAL EXCEPTION: main
05-31 09:33:20.265 20254 20254 E AndroidRuntime: Process: org.openyolo.testapp, PID: 20254
05-31 09:33:20.265 20254 20254 E AndroidRuntime: android.view.InflateException: Binary XML file line #33: Error inflating class TextView
05-31 09:33:20.265 20254 20254 E AndroidRuntime: at android.view.LayoutInflater.createViewFromTag(LayoutInflater.java:713)
05-31 09:33:20.265 20254 20254 E AndroidRuntime: at android.view.LayoutInflater.rInflate(LayoutInflater.java:755)
05-31 09:33:20.265 20254 20254 E AndroidRuntime: at android.view.LayoutInflater.rInflate(LayoutInflater.java:758)
05-31 09:33:20.265 20254 20254 E AndroidRuntime: at android.view.LayoutInflater.inflate(LayoutInflater.java:492)
05-31 09:33:20.265 20254 20254 E AndroidRuntime: at android.view.LayoutInflater.inflate(LayoutInflater.java:397)
05-31 09:33:20.265 20254 20254 E AndroidRuntime: at org.openyolo.api.ui.ProviderPickerActivity$ProviderAdapter.getView(ProviderPickerActivity.java:/** 187)
......
05-31 09:33:20.265 768 779 W ActivityManager: Force finishing activity org.openyolo.testapp/org.openyolo.api.ui.ProviderPickerActivity
05-31 09:33:20.265 768 779 W ActivityManager: Force finishing activity org.openyolo.testapp/.OpenYoloTestActivity
Credential providers declare there capabilities via marked Android manifest entries. Clients discover credential providers by querying for the canonical entries via the PackageManager. For the Save/Hint flows there are no additional "handshakes" and the client will then assume the credential provider is capable of handling their request.
Of course credential providers are free to decide to not handle a hint/save request once it sent, but this is not ideal as it interferes with credential provider choice logic.
Pros:
Cons:
Pros:
Cons:
OpenYolo for Android sends many of its requests via startActivityForResult. This allows for the for the credential provider to optionally display UI to the user without the need for additional handshakes. The UI shown is not defined by the specification allowing each credential provider the freedom in what is displayed.
The specification does define responses for credentials retrieve, save and hint requests. Each has a success case marked by a result code of RESULT_OK and a failure case marked by a result code of RESULT_CANCELED.
Add support for more informative failure result codes that convey the users intent when a successful result is not returned.
A credential providers can return an unsuccessful response to a hint request, but the client is unsure if the user intends to continue or leave a sign up process.
For example: User Bob is using an shopping app, Bob begins the checkout process which requires the user to be signed in, the application is unable to find a credential via OpenYolo credential request and falls back to assisted sign up via a hint request, the credential provider may display a UI. At this point the user may intend to leave the process by clicking the back button or a button on the credential provider's UI. Clicking the back button may mark that the user wishes to exit the sign up process, while a custom button (For example: A credential picker is show, but Bob does not wish to use any of the available accounts, and clicks a "None of these accounts" button) may show intent to continue the sign up process and an absence of available credentials may not reveal any intent.
This is a pattern we followed with AppAuth for Android and has proven to be very valuable. It's labor intensive, but helps put the code in context.
I don't know how you build the OpenYolo demo apps, but if I want to do it, I will have errors:
error: package com.squareup.wire does not exist
To fix it, I have to import the library wire into my project BBQ:
compile 'com.squareup.wire:wire-runtime:2.2.0'
Am I missing something? If not, why this dependency is not into the build.gradle and how is it working on your side?
I originally used Square's protocol buffer library because it was, generally speaking, much easier to integrate its code generation into the build process, and I preferred their generated code to what protobuf v2 was generating. However, a security engineer within Google made a good point that we have to be very careful about the dependencies we choose for this library, as they will be a critical part of the operational security of the protocol.
It was suggested that I move to a Google controlled library, as it is subject to continuous scrutiny by our security engineers - the google implementations are core to virtually every product we build, and any security issues discovered would be fixed urgently as a result. The same may be true of the square libraries, but we have less confidence in this generally.
The impact of this change would be in the structure of the generated protocol buffer classes, so any code currently written that handles these protocol buffers within the SPI and above would have to change. There should be no visible change to the client API.
If there are no objections to this proposal within the next week, I'll start the migration work.
The motivation is to avoid forming ill first impressions in the eyes of potential clients integrating. Until it decided that OpenYOLO is production ready I believe we should put a loud barrier in their way. For example (if possible) we could modify the client library in such a way that it requires the developer to flip a consent bit as a precondition for compilation.
On the demo app Barbican, I have the class CredentialQueryReceiver
which contains:
if (credentialsFound) {
Intent retrieveIntent = RetrieveCredentialActivity.createIntent(context, request);
CredentialRetrieveBbqResponse response =
CredentialRetrieveBbqResponse.newBuilder()
.setRetrieveIntent(IntentUtil.toByteString(retrieveIntent))
.build();
responseBytes = response.toByteArray();
}
This is working fine with the OpenYolo project demo apps.
But on my side I have the following error:
Any idea why and how to fix it?
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.