1433: Create user hashing for Koblenz #1499

ztefanie · 2024-06-19T08:39:19Z

Short description

Add argon2 hashing for card info

Proposed changes

Added argon2id hashing for user data
Added test for argon2id hashing
Added Koblenz region and refactored region db setup a little
Added tests for koblenz canonical json creation
Added docs for Koblenz how to create valid hashes
Added pepper to passbold
Added pepper to env var in Saltstack pr: https://git.tuerantuer.org/DF/salt/pulls/174

Resolved issues

Fixes: #1433

ztefanie · 2024-06-19T08:39:50Z

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt

+                throw Exception("Invalid Json input for hashing")
+            }
+
+            val pepper = Environment.getVariable(KOBLENZ_PEPPER_SYS_ENV) //TODO handle if Null


What error handling do we want, if the env var cannot be accesed / is not set?

just added a basic exception. Do we need a separate exception for that?

docs/CreateKoblenzHash.md

f1sh1918

I'm a little bit confused here about the scope of this task and probably there was a misunderstanding. Seems like i didn't explain this clear enough in the issue :/

I expected in this task: (#1433)

create a UserInfoModel (name, birthday, referenceNr)
hash this model with argon2id
create tests for the hashing
provide sample input and output data to koblenz service provider.

For other tasks like CardCreation we have different tasks, f.e.
#1421

specs/card.proto

...main/kotlin/app/ehrenamtskarte/backend/verification/webservice/schema/CardMutationService.kt

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/CanonicalJson.kt

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt

f1sh1918

Nice implementation!
Added some minor comments
Unfortunately i can not start the backend receiving this error

Exception in thread "main" java.lang.Error: Required project 'koblenz.pass.app' not found!
	at app.ehrenamtskarte.backend.regions.database.SetupKt.insertOrUpdateRegions$createOrUpdateRegion(Setup.kt:21)
	at app.ehrenamtskarte.backend.regions.database.SetupKt.access$insertOrUpdateRegions$createOrUpdateRegion(Setup.kt:1)
	at app.ehrenamtskarte.backend.regions.database.SetupKt$insertOrUpdateRegions$1.invoke(Setup.kt:61)
	at app.ehrenamtskarte.backend.regions.database.SetupKt$insertOrUpdateRegions$1.invoke(Setup.kt:38)
	```

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt

f1sh1918 · 2024-07-03T13:25:02Z

Nice implementation! Added some minor comments Unfortunately i can not start the backend receiving this error

Exception in thread "main" java.lang.Error: Required project 'koblenz.pass.app' not found!
	at app.ehrenamtskarte.backend.regions.database.SetupKt.insertOrUpdateRegions$createOrUpdateRegion(Setup.kt:21)
	at app.ehrenamtskarte.backend.regions.database.SetupKt.access$insertOrUpdateRegions$createOrUpdateRegion(Setup.kt:1)
	at app.ehrenamtskarte.backend.regions.database.SetupKt$insertOrUpdateRegions$1.invoke(Setup.kt:61)
	at app.ehrenamtskarte.backend.regions.database.SetupKt$insertOrUpdateRegions$1.invoke(Setup.kt:38)

@ztefanie
You probably have to create an entry for koblenz in the backend configuration. (config.yml)
Additionally we need for deployment on staging and production configs in salt or the backend service will not start
@svenseeberg can do that

You probably should talk to daniel and sven that we need some domain for koblenz. Just to prepare that

ztefanie · 2024-07-09T08:04:29Z

Nice implementation! Added some minor comments Unfortunately i can not start the backend receiving this error

Exception in thread "main" java.lang.Error: Required project 'koblenz.pass.app' not found!
	at app.ehrenamtskarte.backend.regions.database.SetupKt.insertOrUpdateRegions$createOrUpdateRegion(Setup.kt:21)
	at app.ehrenamtskarte.backend.regions.database.SetupKt.access$insertOrUpdateRegions$createOrUpdateRegion(Setup.kt:1)
	at app.ehrenamtskarte.backend.regions.database.SetupKt$insertOrUpdateRegions$1.invoke(Setup.kt:61)
	at app.ehrenamtskarte.backend.regions.database.SetupKt$insertOrUpdateRegions$1.invoke(Setup.kt:38)

I can start the backend with no problems, maybe some problem with your local setup. We can talk about this, when your vacation is over.

@ztefanie You probably have to create an entry for koblenz in the backend configuration. (config.yml) Additionally we need for deployment on staging and production configs in salt or the backend service will not start @svenseeberg can do that

Not part of this issue.

You probably should talk to daniel and sven that we need some domain for koblenz. Just to prepare that

Getting the domains for Koblenz is not part of this issue and not even related

backend/build.gradle.kts

backend/src/main/kotlin/app/ehrenamtskarte/backend/common/webservice/Constants.kt

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt

docs/CreateKoblenzHash.md

michael-markl · 2024-07-11T08:45:40Z

docs/CreateKoblenzHash.md

+- The birthday is defined in our protobuf [card.proto](../frontend/card.proto) file: It counts the days since the birthday (calculated from 1970-01-01).
+  All values of this field are valid, including the 0, which indicates that the birthday is on 1970-01-01. Birthdays before 1970-01-01 have negative values.


Potentially, it would be easier (for Koblenz and for debuggability) to use a ISO 8601 date string instead, but I'm fine with both :)

I see your point, would keep it like this, so at least our code is consistent?

ISO 8601 might be problematic because there are maybe multiple encodings of the same date. Not sure though. I'm just thinking about the "+Z" and "+00:00"

i'm gonna keep the calculated birthday here if thats okay for everyone

docs/CreateKoblenzHash.md

Co-authored-by: Michael Markl <[email protected]>

backend/src/main/kotlin/app/ehrenamtskarte/backend/regions/database/Setup.kt

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt

docs/CreateKoblenzHash.md

maxammann · 2024-07-23T10:17:32Z

docs/CreateKoblenzHash.md

+- The birthday is defined in our protobuf [card.proto](../frontend/card.proto) file: It counts the days since the birthday (calculated from 1970-01-01).
+  All values of this field are valid, including the 0, which indicates that the birthday is on 1970-01-01. Birthdays before 1970-01-01 have negative values.


ISO 8601 might be problematic because there are maybe multiple encodings of the same date. Not sure though. I'm just thinking about the "+Z" and "+00:00"

docs/CreateKoblenzHash.md

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt

maxammann · 2024-07-23T10:29:13Z

Another approach would have been to hash on the client and just the data to the server for the lookup, right?

maxammann · 2024-07-23T10:33:06Z

I believe this endpoint should have some rate limiting as it might be a target for DoS (argon2id is costly). The alternative approach would be to generate the hash on the client.

michael-markl · 2024-07-23T10:34:20Z

Another approach would have been to hash on the client and just the data to the server for the lookup, right?

Not really possible, as we don't want that the client can decide what to write to the card.

maxammann · 2024-07-23T10:37:17Z

Another approach would have been to hash on the client and just the data to the server for the lookup, right?

Not really possible, as we don't want that the client can decide what to write to the card.

I believe we could still send the user data to the server and do the additional check only if we find the matching hash. Then the expensive argon2id/pbkdf verification only runs if the hash can be found. The user would still send the name, birthdate etc to the server.

ztefanie · 2024-07-23T11:04:43Z

I believe this endpoint should have some rate limiting as it might be a target for DoS (argon2id is costly). The alternative approach would be to generate the hash on the client.

Will be done here: #1421
and is already mentioned in the issue, that it needs rate limiting.

# Conflicts: # backend/src/main/kotlin/app/ehrenamtskarte/backend/cards/Argon2IdHasher.kt # backend/src/main/kotlin/app/ehrenamtskarte/backend/exception/service/NotCorrectProjectExceptions.kt # backend/src/test/kotlin/app/ehrenamtskarte/backend/cards/Argon2IdHasherTest.kt

backend/src/main/kotlin/app/ehrenamtskarte/backend/common/utils/Environment.kt

backend/src/main/kotlin/app/ehrenamtskarte/backend/regions/database/Setup.kt

docs/CreateKoblenzHash.md

f1sh1918 · 2024-07-29T13:51:16Z

Just updated the pr by adding a project config, adjusting hash and argon config in the docs.
Is there anything else that is missing?

…docs

…a import

backend/src/main/kotlin/app/ehrenamtskarte/backend/user/KoblenzUser.kt

docs/CreateKoblenzHash.md

michael-markl · 2024-07-30T10:13:28Z

I believe we could still send the user data to the server and do the additional check only if we find the matching hash. Then the expensive argon2id/pbkdf verification only runs if the hash can be found.

I just noticed that this is also not possible, as the user would know the "secret salt/pepper" to generate the hash. So I think we can only do rate limiting.

ztefanie requested review from steffenkleinle, f1sh1918 and seluianova as code owners June 19, 2024 08:39

ztefanie commented Jun 19, 2024

View reviewed changes

ztefanie force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch 2 times, most recently from 29ec97a to df38492 Compare June 19, 2024 11:35

michael-markl reviewed Jun 19, 2024

View reviewed changes

docs/CreateKoblenzHash.md Outdated Show resolved Hide resolved

1433: Create user hashing for Koblenz

38164c7

ztefanie force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch from df38492 to 38164c7 Compare June 19, 2024 12:03

f1sh1918 reviewed Jun 20, 2024

View reviewed changes

specs/card.proto Outdated Show resolved Hide resolved

...main/kotlin/app/ehrenamtskarte/backend/verification/webservice/schema/CardMutationService.kt Outdated Show resolved Hide resolved

ztefanie force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch 2 times, most recently from 2495c6a to ac86f6f Compare July 2, 2024 11:57

1433: Hash Koblenz data without cardinfo

f67524a

ztefanie force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch from ac86f6f to f67524a Compare July 2, 2024 12:05

ztefanie requested review from michael-markl and f1sh1918 July 2, 2024 12:11

michael-markl reviewed Jul 2, 2024

View reviewed changes

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/CanonicalJson.kt Outdated Show resolved Hide resolved

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt Outdated Show resolved Hide resolved

f1sh1918 reviewed Jul 2, 2024

View reviewed changes

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt Outdated Show resolved Hide resolved

michael-markl reviewed Jul 2, 2024

View reviewed changes

backend/src/main/kotlin/app/ehrenamtskarte/backend/verification/Argon2IdHasher.kt Outdated Show resolved Hide resolved

michael-markl mentioned this pull request Jul 3, 2024

Create endpoint for user data #1416

Closed

1433: Adjust canonical json creation

fbe2385

michael-markl reviewed Jul 9, 2024

View reviewed changes

1433: Remove salt from hash

dc04b09

ztefanie requested a review from michael-markl July 11, 2024 08:23

michael-markl reviewed Jul 11, 2024

View reviewed changes

Update docs/CreateKoblenzHash.md

eb2d7bb

Co-authored-by: Michael Markl <[email protected]>

f1sh1918 mentioned this pull request Jul 22, 2024

Create createCard endpoint #1421

Closed

maxammann reviewed Jul 23, 2024

View reviewed changes

ztefanie added 2 commits July 23, 2024 13:05

1433: Small adjustments

092df0e

seluianova reviewed Jul 23, 2024

View reviewed changes

backend/src/main/kotlin/app/ehrenamtskarte/backend/common/utils/Environment.kt Show resolved Hide resolved

f1sh1918 reviewed Jul 24, 2024

View reviewed changes

backend/src/main/kotlin/app/ehrenamtskarte/backend/regions/database/Setup.kt Outdated Show resolved Hide resolved

f1sh1918 reviewed Jul 29, 2024

View reviewed changes

docs/CreateKoblenzHash.md Outdated Show resolved Hide resolved

f1sh1918 force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch from eaacbc7 to 396b4c4 Compare July 29, 2024 13:41

f1sh1918 requested review from michael-markl and seluianova July 29, 2024 13:50

1433: add project config for koblenz, fix import, adjust hash in the …

d3879e2

…docs

f1sh1918 force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch from 396b4c4 to d3879e2 Compare July 29, 2024 14:00

1433: add regionIdentifier for koblenz, since its needed for user dat…

c630dec

…a import

f1sh1918 force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch from ba75f0c to c630dec Compare July 29, 2024 15:28

1433: increase memory size

6695e76

f1sh1918 force-pushed the 1433-create-argon2-user-hashing-for-koblenz branch from e8aa059 to 6695e76 Compare July 29, 2024 15:36

seluianova reviewed Jul 29, 2024

View reviewed changes

backend/src/main/kotlin/app/ehrenamtskarte/backend/user/KoblenzUser.kt Outdated Show resolved Hide resolved

seluianova reviewed Jul 29, 2024

View reviewed changes

docs/CreateKoblenzHash.md Show resolved Hide resolved

f1sh1918 and others added 2 commits July 30, 2024 09:54

Merge branch 'main' into 1433-create-argon2-user-hashing-for-koblenz

5ae6752

1433: move user class to userdata

47ea4d2

f1sh1918 requested a review from seluianova July 30, 2024 08:56

seluianova approved these changes Jul 30, 2024

View reviewed changes

f1sh1918 merged commit 10a234e into main Jul 30, 2024
1 check passed

f1sh1918 deleted the 1433-create-argon2-user-hashing-for-koblenz branch July 30, 2024 09:59

f1sh1918 added the not-testable label Aug 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1433: Create user hashing for Koblenz #1499

1433: Create user hashing for Koblenz #1499

ztefanie commented Jun 19, 2024

ztefanie Jun 19, 2024

f1sh1918 Jul 29, 2024

f1sh1918 left a comment •

edited

Loading

f1sh1918 left a comment •

edited

Loading

f1sh1918 commented Jul 3, 2024 •

edited

Loading

ztefanie commented Jul 9, 2024

michael-markl Jul 11, 2024

ztefanie Jul 15, 2024

maxammann Jul 23, 2024

f1sh1918 Jul 30, 2024

maxammann Jul 23, 2024

maxammann commented Jul 23, 2024

maxammann commented Jul 23, 2024

michael-markl commented Jul 23, 2024

maxammann commented Jul 23, 2024

ztefanie commented Jul 23, 2024

f1sh1918 commented Jul 29, 2024 •

edited

Loading

michael-markl commented Jul 30, 2024

		- The birthday is defined in our protobuf [card.proto](../frontend/card.proto) file: It counts the days since the birthday (calculated from 1970-01-01).
		All values of this field are valid, including the 0, which indicates that the birthday is on 1970-01-01. Birthdays before 1970-01-01 have negative values.

1433: Create user hashing for Koblenz #1499

1433: Create user hashing for Koblenz #1499

Conversation

ztefanie commented Jun 19, 2024

Short description

Proposed changes

Resolved issues

ztefanie Jun 19, 2024

Choose a reason for hiding this comment

f1sh1918 Jul 29, 2024

Choose a reason for hiding this comment

f1sh1918 left a comment • edited Loading

Choose a reason for hiding this comment

f1sh1918 left a comment • edited Loading

Choose a reason for hiding this comment

f1sh1918 commented Jul 3, 2024 • edited Loading

ztefanie commented Jul 9, 2024

michael-markl Jul 11, 2024

Choose a reason for hiding this comment

ztefanie Jul 15, 2024

Choose a reason for hiding this comment

maxammann Jul 23, 2024

Choose a reason for hiding this comment

f1sh1918 Jul 30, 2024

Choose a reason for hiding this comment

maxammann Jul 23, 2024

Choose a reason for hiding this comment

maxammann commented Jul 23, 2024

maxammann commented Jul 23, 2024

michael-markl commented Jul 23, 2024

maxammann commented Jul 23, 2024

ztefanie commented Jul 23, 2024

f1sh1918 commented Jul 29, 2024 • edited Loading

michael-markl commented Jul 30, 2024

f1sh1918 left a comment •

edited

Loading

f1sh1918 left a comment •

edited

Loading

f1sh1918 commented Jul 3, 2024 •

edited

Loading

f1sh1918 commented Jul 29, 2024 •

edited

Loading