View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0010250 | Taler | libeufin-bank | public | 2025-08-12 08:49 | 2025-10-08 16:48 |
| Reporter | Christian Grothoff | Assigned To | sebasjm | ||
| Priority | normal | Severity | major | Reproducibility | have not tried |
| Status | assigned | Resolution | open | ||
| Platform | i7 | OS | Debian GNU/Linux | OS Version | squeeze |
| Product Version | 1.0 | ||||
| Target Version | 1.1 | ||||
| Summary | 0010250: body of tan_challenges stores passwords in the clear? | ||||
| Description | From my understanding (but didn't try it out), the 'body' field of tan_challenges might store the new password in cleartext when a user is given a 2-FA challenge when changing the account password. That would be very bad. I think it would suffice for us to store the hash of the body, which would be more compact and avoid the vulnerability. | ||||
| Tags | security | ||||
| related to | 0010447 | resolved | Antoine A | cors problem on challenge confirm [dev/antoinea/2fa-v2] |
| related to | 0010448 | resolved | Antoine A | UUID string too large [dev/antoinea/2fa-v2] |
| related to | 0010449 | resolved | Antoine A | ask both channel on the same step [dev/antoinea/2fa-v2] |
| parent of | 0010457 | resolved | Antoine A | mfa should be the last step [dev/anoinea/2fa] |
|
|
We wanted to avoid having to hash the body, as this can cause problems especially with canonicalization. The solution was for the server to store the body and use the stored body once the challenge was solved. This means that the body is only transmitted *once*, but it is stored in plain text, which is less than ideal for passwords. If we hashed the parsed body (the Kotlin class) we would have something that does not require canonicalization and but it can break when we update the server. This is not a big problem as the user can just retry. This would be a breaking change as this mean the body is passed *twice* but this is something we can do for the SPA |
|
|
I can see the canonicalization issues, but storing the user's new password in the clear in the database is IMO a *bigger* issue. We should discuss this entire design, but I'm busy tomorrow, so probably Thursday might work. Need to make sure merchant + libeufin do it the same way, and both do it "right". |
|
|
Since v10, the corebank API will use a MFA API similar to the merchant one. This is SPA breaking change. The online doc have been updated for v10 and the code is in the libeufin 'dev/antoinea/2fa-v2' branch. Once the SPA code is ready we merge in sync |
|
|
Lets not create branches please and less breaking changes, we can add a request parameter here so the endpoint only follows the new code path when the request paremter is present (or a request header is better). Then we also define a deprecation date/release when the old code path is deleted and all client should have been updated. This need to be default strategy of every component, not just libeufin |
|
|
Eh, I think a branch is great here, as long as the SPA doesn't support the new feature. It allows you to finish + test the SPA adaptations against the future code, and once it's done we merge both branches into master. That's how it's supposed to be done if we do breaking changes that only affect a SPA and the respective backend! Maintaining compatibility is always painful, and may not always be reasonable, and if it really only affects a SPA and a backend the test-in-branch and merge-simultaneously strategy is totally approved! |
|
|
Less breaking change mean that change should be in the code but do not break (like enable by config or by the client ACK by some runtime param) here we are only delaying the break until merge. My local setup is more complex if we do feature branch. Because against which server should i develop? And I won't have a shared environment to test after merging/breaking (like head.taler.net or test) or are we going to have a deployment with this branch? > Maintaining compatibility is always painful, and may not always be reasonable I still think we have options to keep compat and we are not using it > if it really only affects a SPA and a backend the test-in-branch and merge-simultaneously Not sure about this, I think (I haven't checked) but this also affect taler-harness. How do we find that out? |
|
|
fixed in c405f5e36..3840861e3 two more (minor) things that doesn't work with this approach in the long run (beside merge hell and environment's configuration craziness): * our product version field here on mantis only have "git master" so branch needs to be reported manually on the title (like the associated issues here) * the script that associate git commit with mantis issues doesn't work with branches |
|
|
This was fixed on master branch of taler-typescript-core.git. I guess we now have to merge libeufin-bank to master. I'm moving the issue to Antoinea. If this branch still needs to be hold on libeufin for some other reason I guess we need to also create the same branch in typescript-core since the SPA now doesn't work with libeufin master. |
|
|
Had to move the branch to dev/antoine/2fa-v2 so I can force push |
|
|
The API have been updated to be more like the one of the merchant |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2025-08-12 08:49 | Christian Grothoff | New Issue | |
| 2025-08-12 08:49 | Christian Grothoff | Status | new => assigned |
| 2025-08-12 08:49 | Christian Grothoff | Assigned To | => Antoine A |
| 2025-08-12 08:50 | Christian Grothoff | Tag Attached: security | |
| 2025-09-02 16:26 | Antoine A | Note Added: 0025773 | |
| 2025-09-02 16:27 | Antoine A | Status | assigned => feedback |
| 2025-09-02 16:46 | Christian Grothoff | Note Added: 0025774 | |
| 2025-09-02 16:46 | Christian Grothoff | Status | feedback => assigned |
| 2025-09-18 19:13 | Antoine A | Assigned To | Antoine A => sebasjm |
| 2025-09-18 19:15 | Antoine A | Note Added: 0025943 | |
| 2025-09-18 20:06 | sebasjm | Note Added: 0025944 | |
| 2025-09-18 20:20 | Christian Grothoff | Note Added: 0025945 | |
| 2025-09-18 20:36 | sebasjm | Note Added: 0025946 | |
| 2025-09-23 13:11 | sebasjm | Relationship added | related to 0010447 |
| 2025-09-23 13:11 | sebasjm | Relationship added | related to 0010448 |
| 2025-09-23 13:11 | sebasjm | Relationship added | related to 0010449 |
| 2025-09-23 17:38 | sebasjm | Note Added: 0025996 | |
| 2025-09-23 18:13 | sebasjm | Note Added: 0025999 | |
| 2025-09-23 18:23 | sebasjm | Assigned To | sebasjm => Antoine A |
| 2025-09-27 21:25 | Christian Grothoff | Relationship added | parent of 0010457 |
| 2025-10-07 18:11 | Antoine A | Note Added: 0026104 | |
| 2025-10-08 16:47 | Antoine A | Assigned To | Antoine A => sebasjm |
| 2025-10-08 16:48 | Antoine A | Note Added: 0026114 |
