openid / OpenID4VCI

68 stars 19 forks source link

[OpenID4VCI] UserPIN description and length #10

Closed OIDF-automation closed 11 months ago

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket: https://bitbucket.org/openid/connect/issues/2020

Original Reporter: pwlb

In Pre-authorized code flow the OpenID4VCI protocol offers the option to use a userPIN to further protect the code from theft/replay. The spec (rightfully) does not make any further assumptions about how and where this userPIN is communicated between Issuer and Holder. But this also means that situations might occur, where the Holder is prompted for the UserPIN in his wallet and he might be clueless of what the PIN is or where it might be communicated. The issuer might give this information on the website, where the QR-Code is displayed but

a) in a same-device scenario the website is not visible anymore for the user

b) in a cross-device scenario the user’s attention is now on the wallet (we have seen problems in the past with DIDComm cross-device flows)

UserPINs might be send as SMS Code or an email might not be aware to the user in this situation.

I therefore propose to extend the properties in the Credential Offer with either

  1. add `user_pin_description` that is a short text describing how and where to get the userPIN OR
  2. add `user_pin_description` in the metadata of the issuer

Additionally wallet implementers told us, they would favor a user_pin_length that says how many digits are expected for the userPIN such that this could be displayed in the app and might give further indications for the user what the correct PIN might be and improve the UI/UX in general.

`user_pin_description` might need i18n similar to what is already existing in the metadata, otherwise the issuer might know the language from the browser metadata.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: nklomp

I am not really sold on the descriptions, as that would also mean you would need i18n etc like you mentioned. If you want to do something like that it makes sense to have predefined values only, so a wallet could determine how to react/display something. But IMO this is a concern of the issuer application to begin with. The issuer itself can be fully decoupled from the application, and could even have multiple applications using the same issuer.

Regarding the userpin length. That for sure would be something which improves UI/UX, as the wallet now knows how render the pin screen. I like individual digits more than having an input field up till a certain amount of digits.

Another improvement would be to have a hash that combines the nonce and the pin, so the wallet would be able to know that the user is entering the correct pin, instead of having to rely on failure later down the road.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

+1 to user_pin_length.

on user_pin_description, I think it should be up to the issuer to display to the user where to find the pin. could sending over the wire where to find pin, help attackers compromise that pin?

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: pwlb

I can see that user_pin_description is more difficult, but in my opinion average users can be very dumb. In a scenario, where users will be “trained” to just scan QR-Codes and buttons, these users will easily overlook the message by the issuer that was next to the Depp-Link/QR-Code. In these cases the additional hint in the user_pin_descrption could come in handy. But yes, the user_pin_descrption could help an attacker.

Concerning the i18n, I think we will be running into similar issues with the display properties from the Issuers metadata, so the solution might be similar. Put the user_pin_descption into the issuer metadata? If metadata is requested with query like lang=en or this is part of a path, this would drastically reduce the size for issuer metadata and could be the same spot where to put a user_pin_descrption. Only downside is that you have the same description for all users, but that’s an acceptable tradeoff.

tlodderstedt commented 1 year ago

I support both, the pin length and the description. I think we should do what we can to make the user experience as easy as possible.

paulbastian commented 1 year ago

After some internal review, we concluded that there are 4 possibilities:

Option A - Only in Credential Offer

The additional information on userPIN length and description reside only in the credentialOffer Example:

{

    credential_issuer: https://issuer-openid4vc.ssi.tir.budru.de,
    credentials: [
        {
            format: "vc+sd-jwt",
            type: "VerifiedEmail"
        }
    ],
    grants: {
        urn:ietf:params:oauth:grant-type:pre-authorized_code: {
            pre-authorized_code: "oaKazRN8I0IbtZ0C7JuMn5",
            user_pin_required: true,
            user_pin_length: 4
            user_pin_description: "Please provide the PIN, which was send via eMail"
        }
    }
}

Option B - Only in metadata

Example:

{
  "credential_issuer": "https://issuer-openid4vc.ssi.tir.budru.de,",
 ...
  "credentials_supported": [
    {
      "id": "VerifiedEMail",
      "format": "vc+sd-jwt",
      "type": "VerifiedEMail",
      ...
           "display": [
            {
              "name": "Verified eMail Address",
              "logo": {
                "url": "https://web.demo.idunion.bundesdruckerei.de/img/Bundesdruckerei_Favicon_RGB_300dpi.png",
                "alternative_text": "Bundesdruckerei Logo"
              },
              "text_color": "#FFFFFF",
              "user_pin": {
                "length": 4,
                "description": "Please provide the PIN, which was send via eMail."
              },
              "background_color": "#12107c",
              "locale": "en-US"
            },
            {
              "name": "Verifizierte eMail Adresse",
              "logo": {
                "url": "https://web.demo.idunion.bundesdruckerei.de/img/Bundesdruckerei_Favicon_RGB_300dpi.png",
                "alternative_text": "Bundesdruckerei Logo"
              },
              "text_color": "#FFFFFF",
              "user_pin": {
                "length": 4,
                "description": "Bitte geben sie die PIN ein, die an ihre eMail gesendet wurde."
              },
              "background_color": "#12107c",
              "locale": "de-DE"
            }
          ]
        }
      }
    },

Option C - Only in Credential_offer_uri

This works similar to Option A, but instead of using Credential_offer we will use credential-offer-uri. Within the HTTP GET request the Wallet will send Accept-Language HTTP Header and the language in the credential offer will be set accordingly.

Option D - UerPIN in Metadata referenced from Credential Offer

This approach is similar to Option B, but instead of having one static user PIN description in the metadata, the issuer uses a reference like an identifer within the credential offer that references one of multiple descriptions form the metadata

Right now, we are favoring Option B and D.

Sakurann commented 1 year ago

I am not sure why option B and D are separated.. I like option B, but I think there are cases, when the issuer has multiple ways of sending the PIN (email, SMS, etc) and would like to be able to dynamically change that - in which case, issuer could have multiple credentials_supported objects each with description for email and SMS respectively. and identifier in the metadata can point one of that object..? but this would increase the size of metadata even more...

for Option A, when issuer is sending the offer, issuer usually has had some kind of interaction with the user, in which case, the issuer probably knows which language it should use, so when it generates the offer, it can already put description in japanese, or german or whatever other language? even if the QR code with the offer is displayer on a shopfront, the issuer probably has an idea which languages its audience knows, so it can put both languages in one description string, like "description": "<description in English> <description in Spanish>.

I think option A is simpler and more flexible.

Sakurann commented 1 year ago

also not sure why A and C are separate..? the structure in A is the same whether credential offer is sent by value or by reference

paulbastian commented 1 year ago

Thanks for the feedback! You are right that D is a special option of B and C is a special option of A.

To clarify option D, I did not mean to use multiple credential offer but instead use an array of user_pin in the metadata with id and use that id within the credential offer:

{
  "credential_issuer": "https://issuer-openid4vc.ssi.tir.budru.de,",
 ...
  "credentials_supported": [
    {
      "id": "VerifiedEMail",
      "format": "vc+sd-jwt",
      "type": "VerifiedEMail",
      ...
           "display": [
            {
              "name": "Verified eMail Address",
              "logo": {
                "url": "https://web.demo.idunion.bundesdruckerei.de/img/Bundesdruckerei_Favicon_RGB_300dpi.png",
                "alternative_text": "Bundesdruckerei Logo"
              },
              "text_color": "#FFFFFF",
              "user_pin": [
                {
                  "id": 1
                  "length": 4,
                  "description": "Please provide the PIN, which was send via **eMail**."
                },
               {
                  "id" : 2
                  "length": 6,
                  "description": "Please provide the PIN, which was send via **SMS**."
                }
              ],
              "background_color": "#12107c",
              "locale": "en-US"
            }
          ]
        }
      }
    },

and the credential offer would look like:


{
    credential_issuer: https://issuer-openid4vc.ssi.tir.budru.de,
    credentials: [
        {
            format: "vc+sd-jwt",
            type: "VerifiedEmail"
        }
    ],
    grants: {
        urn:ietf:params:oauth:grant-type:pre-authorized_code: {
            pre-authorized_code: "oaKazRN8I0IbtZ0C7JuMn5",
            user_pin_required: true,
            user_pin_id: 1
        }
    }
}

So this does not really require much additional space in the metadata. In general this looks way better to me compared to Option A, as this puts the data needed to be displayed by the wallet at the same spot where all the other related display data already is. Dragging this apart looks intuitively to me.

Your suggestion to Option A however works, but also requires to send display-related data between Issuer components that may be separated (web frontend and issuing backend).
Sakurann commented 1 year ago

agreed on option A and C without locale information https://docs.google.com/document/d/1q_vcI7PGJLtM3j0jvpLfmiBsj69jCilTzDQCjKRT2xY/edit

paulbastian commented 1 year ago

At OpenID DCP workshop: