a fix to AppID/FaceID handling on client app side issue #5

[emersonmello]

Hi Neb,

A suggestion to check the Trusted FacetIds

[npesic]

I believe that argument "appId" is not passed as app Id. The actual value that is coming here is the facet Id. It is passed from here:

UAF/fidouafclient/app/src/main/java/org/ebayopensource/fidouafclient/MainActivity.java

Line 132 in a9a14fe

String regRequest = reg.getUafMsgRegRequest(username, facetID);

The "MainActivity" would also need to change.

Did I miss something?

[emersonmello]

Hi,
I had a look to the code (client and server), FIDO specifications and I think you are right about misuse of these terms (appID and facetID), but I'm not sure if MainActivity needs to be changed. Probably the candidates are RegistrationRequestProcessor and AuthenticationRequestProcessor.

Let me try to explain.

According to FIDO Glossary:

• Application Facet is how an application is implemented on various platforms. For example, the application MyBank may have an Android app, an iOS app, and a Web app. These are all facets of the MyBank application.
• Application Facet ID (facetID) is a platform-specific identifier (URI) for an application facet. For Web applications, the facet id is the RFC6454 origin. For Android applications, the facet id is the URI android:apk-key-hash:
• AppID is an identifier for a set of different Facets of a relying party's application. The AppID is a URL pointing to the TrustedFacets, i.e. list of FacetIDs related to this AppID.

According to FIDO UAF Protocol

In section 3.4.1 "Registration Request Message" we have:

[{"header": {"upv": {"major": 1,"minor": 0},
 "op": "Reg",
 "appID": "https://uaf-test-1.noknoktest.com:8443/SampleApp/uaf/facets",
 "serverData":.............

And after execution of getRegRequest method in Reg class we have:

[{"header": {"upv": {"major": 1,"minor": 0},
 "op": "Reg",
 "appID": "http:\/\/www.head2toes.org\/fidouaf\/v1\/public\/uaf\/facets",
 "serverData":.............

So far so good. In section 3.4.4 Registration Response Message we have:

[{"assertions": [{..............
"header": {
"appID": "https://uaf-test-1.noknoktest.com:8443/SampleApp/uaf/facets",
"op": "Reg",

However after execution of clientSendRegResponse method in Reg class we have:

[{"assertions": [{..............
"header": {
"appID": "android:apk-key-hash:Lir5oIjf552K/XN4bTul0VS3GfM",
"op": "Reg",

So, you are right. There is something wrong here!

In section 3.4.6.2 Registration Request Processing Rules for FIDO UAF Clients

The FIDO UAF Client MUST perform the following steps:

• Obtain FacetID of the requesting Application. If the AppID is missing or empty, set the AppID to the FacetID.
  • Verify that the FacetID is authorized for the AppID according to the algorithms in FIDOAppIDAndFacets.
  • If the FacetID of the requesting Application is not authorized, reject the operation

So, current getUafMsgRegRequest is not doing that.

In my pull request I was trying (a really naive way) to "Verify that the FacetID is authorized for the AppID according to the algorithms in FIDOAppIDAndFacets.", assuming that appId param as acting as facetID.

Deepening a little more, In section 3.4.6.4 Registration Response Generation Rules for FIDO UAF Client

The FIDO UAF Client MUST follow the steps:

• Create a RegistrationResponse message
• Copy RegistrationRequest.header into RegistrationResponse.header

The FIDO Server MUST follow the steps:

• Verify each field in fcp (FinalChallengeParams) and make sure it is valid:
  • Make sure fcp.appID corresponds to the one stored by the FIDO Server
  • Make sure fcp.facetID is in the list of trusted FacetIDs

However in processRequest method in RegistrationRequestProcessor class we are sending these values to FIDO Server:

• appID = android:apk-key-hash:Lir5oIjf552K/XN4bTul0VS3GfM; and
• facetId = null;

So, I think this line is not correct in fidouafclinet.op.Reg class because it is overwritten "appId" param in the request message's header, considering that FIDO Client MUST Copy RegistrationRequest.header into RegistrationResponse.header

Finally, I think that we need some work in getFacetId and setAppId methods in RegistrationRequestProcessor class and in AuthenticationRequestProcessor, considering that FIDO Server MUST make sure fcp.facetID is in the list of trusted FacetIDs

Well, probably I missed something :-). Please let me know what you think.

[npesic]

Thank you, Emerson.

This was a very good review of what is happening. I believe if app id overwrite is removed from the Reg class, then we will be fine.

If you want you could try to change this, and include in the PR.

Thanks,
Neb.

[emersonmello]

Hi Neb,
I wrote a new fix to this issue according to the algorithms in FIDOAppIDAndFacets. Please, have a look on this new pull request.

Now I'm quite sure that is necessary to change getAppId method in FidoUafResource class - and facets method in FidoUafResource class - FIDO Server because they are defining statically trustedFacets and appId as https://www.head2toes.org/fidouaf/v1/public/uaf/facets. People who is using your code should use their own fido server instance and put statically their android application hash in facets method.

[npesic]

Hi Emerson,

I'll take a look and try to respond as early as of tomorrow.

Regards,
Neb.