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

Factory-resetting a kdf-enabled card cannot restore its PINs to default values on a J3R180 card #47

Open
wreps8Owt opened this issue Apr 5, 2023 · 55 comments

Comments

@wreps8Owt
Copy link

wreps8Owt commented Apr 5, 2023

Using the factory-reset command of GnuPG on a J3R180 card without kdf enabled does restore its PINs to default values, but factory-resetting a kdf-enabled card cannot. It seems that all parameters return to default, but PINs (especially the admin PIN) does not work. The only way to "restore" it is to reinstall the CAP file.

@wreps8Owt
Copy link
Author

Besides, smartpgp-cli seems unable to pass the PIN authentication against a kdf-enabled card.

@wreps8Owt wreps8Owt changed the title Factory-reset a kdf-enabled card cannot restore its PINs to default values Factory-resetting a kdf-enabled card cannot restore its PINs to default values Apr 6, 2023
@wreps8Owt
Copy link
Author

This issue is opened when my account remained flagged, so you might not be noticed then.

@af-anssi
Copy link
Contributor

af-anssi commented Apr 8, 2023

Using the factory-reset command of GnuPG on a card without kdf enabled does restore its PINs to default values, but factory-resetting a kdf-enabled card cannot. It seems that all parameters return to default, but PINs (especially the admin PIN) does not work. The only way to "restore" it is to reinstall the CAP file.

Which version of the applet are you using?

KDF parameters are actually reset within the applet during a factory reset:
https://github.com/ANSSI-FR/SmartPGP/blob/master/src/fr/anssi/smartpgp/Persistent.java#L226

After a factory reset did you remove the card from the reader and then reinsert it? I suspect a caching mechanism of GnuPG to not deal correctly with the reset of the KDF parameters.

@af-anssi
Copy link
Contributor

af-anssi commented Apr 8, 2023

Besides, smartpgp-cli seems unable to pass the PIN authentication against a kdf-enabled card.

Yes KDF-enabled PIN is not supported in smartpgp-cli.

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 8, 2023 via email

@af-anssi
Copy link
Contributor

af-anssi commented Apr 8, 2023

Yes. After a factory reset of a card with KDF enabled, reinserting, killing gpg-agent, or even restarting pcscd, cannot make default admin PIN work. In Persistent() of Persistent.java, key_derivation_function_length is set to 0, and (key_derivation_function_length > 0) is judged in many place , but during reset, it seems to be "reset" to Constants.KEY_DERIVATION_FUNCTION_DEFAULT.length, which may be 3. Is this okay?

When the Peristent instance is created the key_derivation_function_length is set to 0 https://github.com/ANSSI-FR/SmartPGP/blob/master/src/fr/anssi/smartpgp/Persistent.java#L131, but at the end of the constructor the reset function is called https://github.com/ANSSI-FR/SmartPGP/blob/master/src/fr/anssi/smartpgp/Persistent.java#L137 so the key_derivation_function_length is reset at https://github.com/ANSSI-FR/SmartPGP/blob/master/src/fr/anssi/smartpgp/Persistent.java#L233 to its default value https://github.com/ANSSI-FR/SmartPGP/blob/master/src/fr/anssi/smartpgp/Constants.java#L32.

@af-anssi
Copy link
Contributor

If you agree with my explanations could you close this issue?

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 11, 2023 via email

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 11, 2023 via email

@af-anssi
Copy link
Contributor

af-anssi commented Apr 11, 2023

SmartPGP is implemented such that its internal status after installation is strictly the same as after a factory reset with only two exceptions: the SM certificate (that you do not use) and the use (or not) of the transaction mechanism (which is not available during first installation).
The constructor of the Persistent instance, which holds all data in flash memory (that survives card unplug/power off), ends with a call to its reset function which is also the one called during a factory reset: https://github.com/ANSSI-FR/SmartPGP/blob/master/src/fr/anssi/smartpgp/SmartPGPApplet.java#L1492.

@af-anssi
Copy link
Contributor

af-anssi commented Apr 11, 2023

After a factory reset could you remove (after a backup if necessary) your .gnupg directory in addition to kill any daemons and finally reinsert your card?

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 11, 2023 via email

@af-anssi
Copy link
Contributor

How did you setup/enable KDF?

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 11, 2023 via email

@af-anssi
Copy link
Contributor

af-anssi commented Apr 11, 2023

On a blank J3H145 where I install SmartPGP-v1.22.1.

I setup KDF using ./bin/smartpgp-cli -I kdf-setup and define some PIN codes and obtain the following output where we can see everything went well (90 00):

Select OpenPGP Applet
90 00
Get KDF-DO
90 00
Enter User PIN:
Enter PUK PIN:
Enter Admin PIN:
Verify Admin PIN
90 00
Change PW1
90 00
Change PW3
90 00
Put KDF-DO
90 00

With gpg2 --card-status I obtain the following output where we see KDF is on:

Reader ...........: xxxx
Application ID ...: D276000124010304AFAF000000010000
Application type .: OpenPGP
Version ..........: 3.4
Manufacturer .....: unknown
Serial number ....: 00000001
Name of cardholder: [not set]
Language prefs ...: en
Salutation .......:
URL of public key : [not set]
Login data .......: [not set]
Signature PIN ....: forced
Key attributes ...: rsa4096 rsa4096 rsa4096
Max. PIN lengths .: 32 32 32
PIN retry counter : 3 0 3
Signature counter : 0
KDF setting ......: on
Signature key ....: [none]
Encryption key....: [none]
Authentication key: [none]
General key info..: [none]

The PIN codes set are correctly verified.
Using gpg2 --card-edit I do a factory reset and then verify user PIN (which is back to the default one) and obtain the following output where we can see KDF is correctly set to off and the default user PIN is verified correctly:

gpg/card> admin
Admin commands are allowed

gpg/card> factory-reset
gpg: OpenPGP card no. D276000124010304AFAF000000010000 detected

gpg: Note: This command destroys all keys stored on the card!

Continue? (y/N) y
Really do a factory reset? (enter "yes") yes

gpg/card> list

Reader ...........: xxxx
Application ID ...: D276000124010304AFAF000000010000
Application type .: OpenPGP
Version ..........: 3.4
Manufacturer .....: unknown
Serial number ....: 00000001
Name of cardholder: [not set]
Language prefs ...: en
Salutation .......:
URL of public key : [not set]
Login data .......: [not set]
Signature PIN ....: forced
Key attributes ...: rsa4096 rsa4096 rsa4096
Max. PIN lengths .: 127 127 127
PIN retry counter : 3 0 3
Signature counter : 0
KDF setting ......: off
Signature key ....: [none]
Encryption key....: [none]
Authentication key: [none]
General key info..: [none]

gpg/card> verify

Reader ...........: xxxx
Application ID ...: D276000124010304AFAF000000010000
Application type .: OpenPGP
Version ..........: 3.4
Manufacturer .....: unknown
Serial number ....: 00000001
Name of cardholder: [not set]
Language prefs ...: en
Salutation .......:
URL of public key : [not set]
Login data .......: [not set]
Signature PIN ....: forced
Key attributes ...: rsa4096 rsa4096 rsa4096
Max. PIN lengths .: 127 127 127
PIN retry counter : 3 0 3
Signature counter : 0
KDF setting ......: off
Signature key ....: [none]
Encryption key....: [none]
Authentication key: [none]
General key info..: [none]

At this point I am not able to reproduce your problem. Could you try the exact same steps and provide the corresponding output?

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 11, 2023 via email

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 11, 2023 via email

@wreps8Owt wreps8Owt changed the title Factory-resetting a kdf-enabled card cannot restore its PINs to default values Factory-resetting a kdf-enabled card cannot restore its PINs to default values on an J3R180 card Apr 11, 2023
@wreps8Owt wreps8Owt changed the title Factory-resetting a kdf-enabled card cannot restore its PINs to default values on an J3R180 card Factory-resetting a kdf-enabled card cannot restore its PINs to default values on a J3R180 card Apr 11, 2023
@af-anssi
Copy link
Contributor

af-anssi commented Apr 11, 2023

I do not have such card to make some tests. Could you checkout branch issue-47, compile, install and test if the patch 4df65b2 solves the problem?

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

在 2023/4/12 09:28, Driner 写道:

I do not have such card to make some tests. Could you checkout branch issue-47, compile, install and test the patch [4df65b2](4df65b2) solves the problem? > Sadly this patch does not solve the problem.
Besides, Using bin/smartpgp-cli get-kdf -o kdf to read the default kdf gets

00000000 c2 81 01 00 |....| 00000004

The problem does not come from the KDF DO but from the PIN objects.

@af-anssi
Copy link
Contributor

I do not have such card to make some tests. Could you checkout branch issue-47, compile, install and test the patch [4df65b2](4df65b2) solves the problem?
Sadly this patch does not solve the problem.

Could you confirm you properly uninstalled the applet AND the package from the card before testing the one built from branch issue-47?

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

af-anssi commented Apr 12, 2023

The KDF DO is correctly reset. It seems like the call to *_pin.update in the reset function of the Persistent object has no effect. I suspect the transaction mechanism.

Could try with last revision of branch issue-47 where I just committed a change to deactivate any transaction around the user and admin PIN changes (46da410) to:

  • setup-kdf with smartpgp-cli
  • use gpg2 --card-edit to change the user PIN to a non-default value
  • factory reset the card
  • verify the user PIN to check it is back ti its default value (123456)

Could you please provide all outputs of the above operations? Sorry for the nconvenience of making you make a lot of tests.

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

Sorry for the inconvenience, I have just fixed this.

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

It just means you entered the incorrect current admin PIN...

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

af-anssi commented Apr 12, 2023

Mea culpa I forgot to set the initial/default value.
It should be OK now.

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

af-anssi commented Apr 12, 2023

Yes, it is okay now.

This is really odd/strange to have to create a new OwnPIN object. This is clearly not the philosophy of JavaCard.

Besides, is it possible to commitTransaction() only once during Persistent.reset()?

No, the context would be too huge for the card, and there is no chance it accepts such a large transaction at once.

As already mentioned previously: I suspect you have exactly the same problem without KDF activated. On a released version of SmartPGP (NOT a compiled version from issue-47), without KDF enabled, can you try to change the PIN to a large value such as 32 digits, verify it is working, then factory reset. If the default PIN (123456) is restored and verifies correctly then I have no clue what's going one. Otherwise there is a strong problem in the implementation of the OwnerPIN class in this platform.

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

Ok, so there is definitely a strong bug in the implementation of the update procedure of the OwnerPIN on the J3R180.

To confirm it, could you try, without KDF enabled and without doing a factory reset, to simply change the user PIN twice:

  • from the default (123456) to a large one (123456789012345678901234567890)
  • verify the PIN has really been changed
  • from the large one to a smaller one (12345678)
  • verify the PIN has really been changed

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

Ok, so there is definitely a strong bug in the implementation of the update procedure of the OwnerPIN on the J3R180.

To confirm it, could you try, without KDF enabled and without doing a factory reset, to simply change the user PIN twice:

* from the default (123456) to a large one (123456789012345678901234567890)

* verify the PIN has really been changed

* from the large one to a smaller one (12345678)

* verify the PIN has really been changed

Could you try the test mentioned above?

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@af-anssi
Copy link
Contributor

So it confirms the (strong) bug in the OwnerPIN.update method on this platform.

I have updated the branch issue-47 to provide a single patch to you to have a working version: a new OwnerPIN instance is created before each call to udpate so everything should just work for your card. I cannot integrate such patch in the official releases of SmartPGP.

FYI @martinpaljak

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 12, 2023 via email

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 15, 2023 via email

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 17, 2023 via email

@af-anssi
Copy link
Contributor

af-anssi commented Apr 17, 2023

Unfortunately if you set the PIN_MAX_SIZE to 16 you will not be able to use KDF.
KDF in OpenPGP relies on SHA256 (respectively SH512) (https://gnupg.org/ftp/specs/OpenPGP-smart-card-application-3.4.pdf, page 19) which makes the real PIN to be stored 32 bytes (resp. 64 bytes) long :/

@wreps8Owt
Copy link
Author

After I set the PIN_MAX_SIZE to 16, set a PIN longer than 16 in gpg will fail with "Error changing the PIN: Invalid value" as expected, but running bin/smartpgp-cli setup-kdf will still seemingly succeed:

$ bin/smartpgp-cli setup-kdf
Select OpenPGP Applet
90 00
Get KDF-DO
90 00
Verify Admin PIN
90 00
Change PW1
67 00
Change PW3
67 00
Put KDF-DO
90 00

However, "Change PW1" and "Change PW3" are actually failed, but in highlevel.py the return value of change_reference_data_pw1() and change_reference_data_pw3() are not checked, so the smartpgp-cli does not stop, and the overall effect is only changing the kdf-do.

All possible error during setup-kdf had better be collected, like the patch below:

diff --git a/bin/smartpgp/highlevel.py b/bin/smartpgp/highlevel.py
--- a/bin/smartpgp/highlevel.py
+++ b/bin/smartpgp/highlevel.py
@@ -462,17 +462,17 @@ class CardConnectionContext:
             raise AdminPINFailed
         ####### step 3bis
         if nresetting_code != None:
-            set_resetting_code(self.connection, nresetting_code)
+            (_,sw1,sw2) = set_resetting_code(self.connection, nresetting_code)
             if sw1!=0x90 or sw2!=0x00:
                 print("set_resetting_code failed")
                 return
         ####### step 4
-        change_reference_data_pw1(self.connection, ascii_encode_pin(pw1), list(npw1))
+        (sw1,sw2) = change_reference_data_pw1(self.connection, ascii_encode_pin(pw1), list(npw1))
         if sw1!=0x90 or sw2!=0x00:
             print("change_reference_data_pw1 failed")
             return
         ####### step 4bis
-        change_reference_data_pw3(self.connection, ascii_encode_pin(pw3), list(npw3))
+        (sw1,sw2) = change_reference_data_pw3(self.connection, ascii_encode_pin(pw3), list(npw3))
         if sw1!=0x90 or sw2!=0x00:
             print("change_reference_data_pw3 failed")
             return

@af-anssi
Copy link
Contributor

Could you submit a pull request on branch javacard-3.0.1 with this patch? So that your contribution will be tracked in the repository :)

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 17, 2023

No, thanks. I know (and hate) the game rule of github very well, but I do not want to be conspicuous, for example, to be "tracked" in the commit log of a project. That is why I posted this patch in the issue in the first place, rather than forking the whole project to submit a mere "pull request".

You are totally free to take this patch as your own contribution.

@wreps8Owt
Copy link
Author

wreps8Owt commented Apr 18, 2023 via email

@mwalle
Copy link

mwalle commented Feb 14, 2024

I have the same problem with this exact same card. Setting a large PIN will render the card useless until you reinstall the applet, which is a PITA.

The bug seems to happen once the OwnerPIN is set to a larger value than 16. In that case, PINs smaller than or equal to 16 aren't possible anymore. As a workaround, also for an official release, what do you think about padding the PIN to 17 bytes (if it's smaller than 17 bytes).

@wreps8Owt
Copy link
Author

wreps8Owt commented Feb 14, 2024 via email

@mwalle
Copy link

mwalle commented Feb 14, 2024

Well it's not acceptable for me and future users will run into the same problem. Please read my answer again. I'm proposing a viable workaround to always set PINs greater than 16 bytes.

Maybe what you are missing is that, that there is only a bug if you use PINs shorter than or equal to 16 bytes after you've used a longer PIN. Therefore, my workaround proposal is to always use PINs longer than 16bytes by padding shorter PINs to at least 17bytes. Then you won't trigger that bug.

@github-af
Copy link
Owner

Even if the padding solution is purely internal to the applet and transparent to client applications, I would not implement such a workaround in the default release to deal with one specific card problem only. As for issue #40 I could produce a patch with a workaround (either padding, either max PIN sizes' limit) for those who really need to cope with this issue and cannot switch to another card.

@mwalle
Copy link

mwalle commented Feb 14, 2024

That's a shame, because what is the disadvantage of it, besides a slightly increased code size (given that the workaround will indeed work)?

Isn't it about the user experience, all software is full of workarounds for real hardware (bugs). Yes it might only affect this one card or it might affect more cards. Or maybe it will affect the whole j3r family. I could write that code myself, but IMHO it should be included by default. Keep in mind, you are building something that should help users and make it easy for the users.

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

No branches or pull requests

4 participants