Merge pull request #533 from martenrebane/MOPPAND-1346

Fix PIN2 in memory after NFC signing
open-eid · Jul 16, 2024 · 5397028 · 5397028
2 parents 8a1473c + 53d7aca
commit 5397028
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 16 deletions.
diff --git a/app/src/main/java/ee/ria/DigiDoc/android/signature/update/SignatureAddSource.java b/app/src/main/java/ee/ria/DigiDoc/android/signature/update/SignatureAddSource.java
@@ -7,6 +7,7 @@
 import androidx.annotation.Nullable;
 
 import java.io.File;
+import java.util.Arrays;
 
 import javax.inject.Inject;
 
@@ -167,14 +168,10 @@ Observable<? extends SignatureAddResponse> sign(File containerFile,
             settingsDataStore.setCan(nfcRequest.can());
             Single<SignedContainer> s = signatureContainerDataSource.get(containerFile, isSivaConfirmed);
             Observable<NFCResponse> obs = s.flatMapObservable(container -> {
-                    String can = nfcRequest.can();
-                    String pin2 = nfcRequest.pin2();
-                    NFCOnSubscribe nfcsub = new NFCOnSubscribe(navigator, container, can, pin2, roleData);
-                    return Observable.create(nfcsub);
+                NFCOnSubscribe nfcsub = new NFCOnSubscribe(navigator, container, nfcRequest.can(), nfcRequest.pin2(), roleData);
+                return Observable.create(nfcsub);
                 });
-            return obs.switchMap(response -> {
-                        return Observable.just(response);
-                    })
+            return obs.switchMap(response -> Observable.just(response))
                     .subscribeOn(Schedulers.io())
                     .observeOn(AndroidSchedulers.mainThread())
                     .startWithItem(NFCResponse.createWithStatus(SessionStatusResponse.ProcessStatus.OK, null))

diff --git a/app/src/main/java/ee/ria/DigiDoc/android/signature/update/nfc/NFCOnSubscribe.java b/app/src/main/java/ee/ria/DigiDoc/android/signature/update/nfc/NFCOnSubscribe.java
@@ -12,6 +12,7 @@
 
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
 
 import javax.annotation.Nullable;
 
@@ -32,12 +33,12 @@ public class NFCOnSubscribe implements ObservableOnSubscribe<NFCResponse> {
     private final Navigator navigator;
     private final SignedContainer container;
     private final String can;
-    private final String pin2;
+    private final byte[] pin2;
     private final RoleData role;
     private NFC nfc;
 
     public NFCOnSubscribe(Navigator navigator, SignedContainer container,
-                              String can, String pin2, @Nullable RoleData roleData) {
+                              String can, byte[] pin2, @Nullable RoleData roleData) {
         this.navigator = navigator;
         this.container = container;
         this.can = can;
@@ -104,9 +105,11 @@ private NFCResponse onTagDiscovered(NfcAdapter adapter, Tag tag) {
 
             // pad the PIN and use the chip for verification
             byte[] paddedPIN = Hex.decode("ffffffffffffffffffffffff");
-            byte[] pin2b = pin2.getBytes(StandardCharsets.UTF_8);
-            System.arraycopy(pin2b, 0, paddedPIN, 0, pin2b.length);
+            System.arraycopy(pin2, 0, paddedPIN, 0, pin2.length);
             r = nfc.communicateSecure(VER_PIN2_CMD, paddedPIN);
+            if (null != pin2 && pin2.length > 0) {
+                Arrays.fill(pin2, (byte) 0);
+            }
             Timber.log(Log.DEBUG, "Verify PIN2:%x %s", r.code, Hex.toHexString(r.data));
             if (r.code != 0x9000) {
                 if (r.code == 0x6983) {
@@ -121,7 +124,6 @@ private NFCResponse onTagDiscovered(NfcAdapter adapter, Tag tag) {
             r = nfc.communicateSecure(CMD_SET_ENV_SIGN, SET_ENV_SIGN);
             Timber.log(Log.DEBUG, "Set ENV:%x %s", r.code, Hex.toHexString(r.data));
 
-
             container.sign(cert.data(),
                     signData -> ByteString.of(nfc.calculateSignature(signData.toByteArray())), role);
         } catch (TagLostException exc) {

diff --git a/app/src/main/java/ee/ria/DigiDoc/android/signature/update/nfc/NFCRequest.java b/app/src/main/java/ee/ria/DigiDoc/android/signature/update/nfc/NFCRequest.java
@@ -9,9 +9,9 @@ public abstract class NFCRequest implements SignatureAddRequest {
 
     public abstract String can();
 
-    public abstract String pin2();
+    public abstract byte[] pin2();
 
-    public static NFCRequest create(String can, String pin2) {
+    public static NFCRequest create(String can, byte[] pin2) {
         return new AutoValue_NFCRequest(can, pin2);
     }
 }
diff --git a/app/src/main/java/ee/ria/DigiDoc/android/signature/update/nfc/NFCView.java b/app/src/main/java/ee/ria/DigiDoc/android/signature/update/nfc/NFCView.java
@@ -28,6 +28,8 @@
 import com.google.android.material.textfield.TextInputLayout;
 import com.google.android.material.textview.MaterialTextView;
 
+import java.nio.charset.StandardCharsets;
+
 import ee.ria.DigiDoc.R;
 import ee.ria.DigiDoc.android.ApplicationApp;
 import ee.ria.DigiDoc.android.Constants;
@@ -121,8 +123,11 @@ public void reset(SignatureUpdateViewModel viewModel) {
 
     @Override
     public NFCRequest request() {
-        return NFCRequest.create(canView.getText().toString(),
-                pinView.getText().toString());
+        NFCRequest nfcRequest = NFCRequest.create(canView.getText().toString(),
+                pinView.getText().toString().getBytes(StandardCharsets.UTF_8));
+        pinView.setText(null);
+
+        return nfcRequest;
     }
 
     @Override