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

Adding a '+' to the phone number before hashing should be mandatory, not optional #65

Closed
eric-murray opened this issue Oct 23, 2023 · 11 comments · Fixed by #90
Closed

Comments

@eric-murray
Copy link

Problem description
Phone number optionally prefixed with '+' before hashing? Why is this an option?

    NumberVerificationRequestBody:
        ...
        hashedPhoneNumber:
          description: Hashed phone number. SHA-256 (in hexadecimal representation) of the mobile phone number in **E.164 format (starting with country code)**. Optionally prefixed with '+'.

This means that the supplied hashed phone number must be compared to two hashes of the true phone number to determine if they match. Some have claimed this is a "feature" for those who cannot read a specification or format phone numbers properly, but I think this is a bug that increases implementation complexity and processing time.

Expected behaviour
Adding '+' before hashing should be mandatory

    NumberVerificationRequestBody:
        ...
        hashedPhoneNumber:
          description: Hashed phone number. SHA-256 (in hexadecimal representation) of the mobile phone number in **E.164 format (starting with country code and mandatorily prefixed by '+')**

Alternative solution
Remove option for hashing completely. It adds no data security for such a small plaintext space.

Additional context

@bigludo7
Copy link
Collaborator

+1 on @eric-murray proposal as we get same discussion in Orange

@chrishowell
Copy link

chrishowell commented Oct 30, 2023

I would prefer to remove hashedPhoneNumber - for the small plaintext space, I agree it is not really helpful.

@fernandopradocabrillo
Copy link
Collaborator

I would actually agree with both options. Remove the hashing as it does not add much security and make the '+' required.

And make it required both in request body and in the response (in the operations that return a phoneNumber). Because having it optional will bring other issues, for example the API Client might receive the phone number with '+' in one operator and without it in other, so it needs to have further logic to handle it.

@chrishowell
Copy link

chrishowell commented Nov 1, 2023

I think we have some consensus then to remove the hash parameter. Eric will you produce the PR?
W.r.t. plus or no-plus for the plaintext, I think this should be agreed in commonalities - we want to have that standardised across APIs. Should we split that into a separate issue? Perhaps worth noting that there's already talk of a tel:+447255255255 format in auth

@eric-murray
Copy link
Author

I'm not directly involved in this sub-project, so would look to a maintainer to raise the PR. But maybe they prefer to discuss this during the regular meetings first.

@ECORMAC
Copy link
Collaborator

ECORMAC commented Nov 1, 2023

With regard to requiring + with the E.164, the ITU E.164 spec (International operation – Numbering plan of the
international telephone service) chapter 6.2 (Structure of the international ITU-T E.164-number) does not mandate it. If we "mandate / require" something perhaps we should be able to point at an industry aligned specification.

@fernandopradocabrillo
Copy link
Collaborator

@ECORMAC
I can see a couple of references suggesting that its use is recommended:

In the ITU-T Rec. E.164:
image

In the ITU-T E.123 referenced above:
image

Given that having it optional brings more problems than benefits, I think we could keep it as required and refer to these paragraphs which do not oblige perse, but recommend its use.

WDYT?

@chrishowell
Copy link

Yes, we should include it :D

@hdamker
Copy link
Contributor

hdamker commented Nov 22, 2023

I think we have some consensus then to remove the hash parameter. Eric will you produce the PR?

@chrishowell This point seems to be outside of the scope of this issue (adding '+' to the phone number) - would you mind to open an explicit issue for that?

@HuubAppelboom
Copy link
Collaborator

I think we have some consensus then to remove the hash parameter. Eric will you produce the PR?

@chrishowell This point seems to be outside of the scope of this issue (adding '+' to the phone number) - would you mind to open an explicit issue for that?

I think removing the hashing should be better thought through, since it will impact the privacy of end users. In general (just like with KYC match), hashing is currently applied to prevent that the MNO learns more about the users' data than they should.

For example, users may have more than one phone number in use, and by the this the telco can become aware of other phone numbers that end user has in use, in case the wrong phone number is entered. That is the reason in the current Mobile Connect Number Verify specs the hashing is used as a privacy protecting measure.

@maxl2287
Copy link
Contributor

Short question besides:

Will the phone_number in the access-token be provided as clear-text only or can it also be a hashed-number?

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

Successfully merging a pull request may close this issue.

8 participants