eu-digital-green-certificates / dgca-wallet-app-android

Repository for the dgca wallet app for android.
Apache License 2.0
58 stars 18 forks source link

Encrypted storage of DGCs not in line with spec #39

Closed fneur closed 3 years ago

fneur commented 3 years ago

Describe the bug

According to the specification (p. 25)

DGCs will be imported by scanning a base45-encoded QR code and decoding CBOR to JSON. Afterwards, it is symmetrically encrypted in the app’s sandbox and the symmetric key is stored in the system’s keychain.

However, the actual cipher used is RSA_ECB_PKCS1_PADDING, hence asymmetric. The relevant call chain is as follows:

[WalletRepositoryImpl.kt] claimCertificate(...) ---> [WalletRepositoryImpl.kt] keyStoreCryptor.encrypt(qrCode) ---> [DefaultKeyStoreCryptor.kt] getSecurityKeyWrapper(keyStore).encrypt(qrCode) ---> [SecurityKeyWrapper.kt] getCipher(...) = Cipher.getInstance(RSA_ECB_PKCS1_PADDING)

Expected behaviour

As stated in the specification, a symmetric encryption algorithm should be used.

Steps to reproduce the issue

Technical details

Possible Fix

Additional context

oleksandrsarapulovgl commented 3 years ago

@SchulzeStTSI Steffen could you please comment if we need to make any changes here?

fneur commented 3 years ago

@oleksandrsarapulovgl May I ask another question meanwhile (until Steffen finds the time to answer yours): Why not use RSA with Optimal Asymmetric Encryption Padding (OAEP)? Is there a specific reason? (For new applications, one would expect OAEP rather than the old PKCS1 padding)

oleksandrsarapulovgl commented 3 years ago

@fneur sure. There is not any specific reason, we can update that

fneur commented 3 years ago

@oleksandrsarapulovgl Thanks for your quick reply! But now let's wait and see what Steffen's decision will be. Cheers, F.

fneur commented 3 years ago

Steffen must be very busy. So I close this issue now.