New issue
Advanced search Search tips

Issue 803752 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Zip Archver: should forget password when shut down before unmounting

Project Member Reported by yamaguchi@chromium.org, Jan 19 2018

Issue description

Chrome Version: 65 dev and latest 64 release

Steps To Reproduce:
(1) Create a zip file with a text file, protected by a password.
(2) Open it in the Files app. See the zip file volume appears on LHS.
(3) Open the text file inside it. Enter the correct password with "remember" checkbox checked.
(4) Close the text file.
(5) Open the text file again. See password is not required.
(6) Reboot the device. See the zip file volume doesn't appear on LHS.
(7) Open the same zip file again. See the zip file volume appears on LHS.
(8) Open the text file again.

(My two cents: if you test with an image file instead of text, it may complicate the process as it automatically starts to read the file inside for creating thumbnails.)


Expected Result:
At step (8), passprase is required before opening the file.

Actual Result:
Password dialog doesn't appear. The file contents can be read without entering passphrase.

If user unmounts the zip file by the eject icon on the volume, it requires passphrase next time.



This happens because it persists the passphrase along with the mount info for resuming in the next session.
However, we changed it not to resume the mounted zip files upon sign-in (  Issue 789073  ) even when user rebooted the device without unmounting the zip files.
So now it would be more understandable to forget the passphrases every after login.

Technically, we can stop persisting the mount information containing the passphrase in the first place following the change for  Issue 789073 .
 
Blocking: 803694
Labels: -Pri-3 M-64 Pri-2
This is common to ZIP unpacker (in M64) as well.
Description: Show this description
Blocking: -803694
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3169366954d82dc6b35ad6a869907cde0d714cd

commit c3169366954d82dc6b35ad6a869907cde0d714cd
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Wed Jan 24 07:10:57 2018

Zip Archiver: Disable storing mount and password info.

We are not restoring mount files at session restart anymore.
Therefore the mount information and passwords should not be persisted.
Othrewise it will be persisted until the user opens the same archive
again manually after a long time and the files will be opened without
requiring passwords.

Bug:  789073 , 803752 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idadcff694b20109eb7e31710b57d88400601d30a
Reviewed-on: https://chromium-review.googlesource.com/876705
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531467}
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/css/passphrase-dialog.css
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/html/passphrase-dialog.html
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/js/background.js
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-dialog.js
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-manager.js
[modify] https://crrev.com/c3169366954d82dc6b35ad6a869907cde0d714cd/chrome/browser/resources/chromeos/zip_archiver/manifest.json

Status: Fixed (was: Assigned)
Labels: -M-64 M-66
The change goes to M66 unless it's cherrypicked.
Cc: weifangsun@chromium.org yawano@chromium.org
Labels: -M-66 M-65 Merge-Request-65
Status: Started (was: Fixed)
We'd like to cherrypick this fix (commit c316936) to M65 release branch, considering its importance and impact.
- We don't like to store zip file passphrase that is basically not needed in the majority of usage patterns.
- In addition to the issue noted in #0, there's been an issue that "remember" checkbox is normally meaningless since M64. The fix above will remove the checkbox to resolve that inconsistency. (Also see my comment in https://bugs.chromium.org/p/chromium/issues/detail?id=804679#c3)
Project Member

Comment 8 by sheriffbot@chromium.org, Feb 1 2018

Labels: -Merge-Request-65 Hotlist-Merge-Approved Merge-Approved-65
Your change meets the bar and is auto-approved for M65. Please go ahead and merge the CL to branch 3325 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Feb 1 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/506649901dd54feac291f89518bf86c8ae6f5b57

commit 506649901dd54feac291f89518bf86c8ae6f5b57
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Thu Feb 01 07:11:33 2018

Zip Archiver: Disable storing mount and password info.

We are not restoring mount files at session restart anymore.
Therefore the mount information and passwords should not be persisted.
Othrewise it will be persisted until the user opens the same archive
again manually after a long time and the files will be opened without
requiring passwords.

Bug:  789073 , 803752 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idadcff694b20109eb7e31710b57d88400601d30a
Reviewed-on: https://chromium-review.googlesource.com/876705
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531467}(cherry picked from commit c3169366954d82dc6b35ad6a869907cde0d714cd)
Reviewed-on: https://chromium-review.googlesource.com/897284
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#227}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/css/passphrase-dialog.css
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/html/passphrase-dialog.html
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/js/background.js
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-dialog.js
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-manager.js
[modify] https://crrev.com/506649901dd54feac291f89518bf86c8ae6f5b57/chrome/browser/resources/chromeos/zip_archiver/manifest.json

Status: Fixed (was: Started)
Status: Verified (was: Fixed)
Verified on Chrome OS version 10323.24.0, 65.0.3325.59 dev channel eve device.
Project Member

Comment 12 by bugdroid1@chromium.org, Feb 22 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/92067a9e7c3867cbfb832dd8d953a4ec54a59238

commit 92067a9e7c3867cbfb832dd8d953a4ec54a59238
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Thu Feb 22 03:47:06 2018

Revert "Zip Archiver: Disable storing mount and password info."

This reverts commit c3169366954d82dc6b35ad6a869907cde0d714cd.

Reason for revert: This caused regression  Issue 812123 .
This had caused that mount resuming succeeds superficially by
the caller despite it actually failed, thus leaving inaccessible
zip mount point.

Original change's description:
> Zip Archiver: Disable storing mount and password info.
>
> We are not restoring mount files at session restart anymore.
> Therefore the mount information and passwords should not be persisted.
> Othrewise it will be persisted until the user opens the same archive
> again manually after a long time and the files will be opened without
> requiring passwords.
>
> Bug:  789073 , 803752 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: Idadcff694b20109eb7e31710b57d88400601d30a
> Reviewed-on: https://chromium-review.googlesource.com/876705
> Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
> Reviewed-by: Yuki Awano <yawano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531467}

TBR=yawano@chromium.org,yamaguchi@chromium.org

Bug:  789073 ,  803752 
Change-Id: I387e16717b5187389d01c5d5bc4a8b6884dcf035
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/928063
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538338}
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/css/passphrase-dialog.css
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/html/passphrase-dialog.html
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/js/background.js
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-dialog.js
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-manager.js
[modify] https://crrev.com/92067a9e7c3867cbfb832dd8d953a4ec54a59238/chrome/browser/resources/chromeos/zip_archiver/manifest.json

Reopening as the fix (crrev.com/c/876705) has been reverted.
Status: Started (was: Verified)
Labels: CrOS-FilesApp-Zip
Project Member

Comment 16 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c0dfedfad98409de991ecadf83e31f47a2a47f82

commit c0dfedfad98409de991ecadf83e31f47a2a47f82
Author: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Date: Fri Feb 23 01:42:48 2018

Revert "Zip Archiver: Disable storing mount and password info."

This reverts commit c3169366954d82dc6b35ad6a869907cde0d714cd.

Reason for revert: This caused regression  Issue 812123 .
This had caused that mount resuming succeeds superficially by
the caller despite it actually failed, thus leaving inaccessible
zip mount point.

Original change's description:
> Zip Archiver: Disable storing mount and password info.
>
> We are not restoring mount files at session restart anymore.
> Therefore the mount information and passwords should not be persisted.
> Othrewise it will be persisted until the user opens the same archive
> again manually after a long time and the files will be opened without
> requiring passwords.
>
> Bug:  789073 , 803752 
> Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
> Change-Id: Idadcff694b20109eb7e31710b57d88400601d30a
> Reviewed-on: https://chromium-review.googlesource.com/876705
> Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
> Reviewed-by: Yuki Awano <yawano@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531467}

TBR=yawano@chromium.org,yamaguchi@chromium.org

Bug:  789073 ,  803752 
Change-Id: I387e16717b5187389d01c5d5bc4a8b6884dcf035
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Reviewed-on: https://chromium-review.googlesource.com/928063
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538338}(cherry picked from commit 92067a9e7c3867cbfb832dd8d953a4ec54a59238)


Bug:  812123 
Change-Id: I387e16717b5187389d01c5d5bc4a8b6884dcf035
Reviewed-on: https://chromium-review.googlesource.com/933681
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#565}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/css/passphrase-dialog.css
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/html/passphrase-dialog.html
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/background.js
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-dialog.js
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/js/passphrase-manager.js
[modify] https://crrev.com/c0dfedfad98409de991ecadf83e31f47a2a47f82/chrome/browser/resources/chromeos/zip_archiver/manifest.json

Labels: -CrOS-FilesApp-Zip CrOSFilesFeature-Zip
Labels: -M-65 M-67
Punting P2 issues to M67.
Project Member

Comment 19 by bugdroid1@chromium.org, Mar 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f8781418ce126ab08de6a82eef749d4c3bef5080

commit f8781418ce126ab08de6a82eef749d4c3bef5080
Author: Tatsuhisa Yamaguchi <yamaguchi@google.com>
Date: Wed Mar 14 11:10:42 2018

Zip Archiver: Stop persisting mount info.

Make Zip Archiver to forget password when requested to resume, as well
as returning error to make VolumeManager remove the persisted provided
volume info.
Remove all code for persisting zip mount state.

Bug:  789073 , 803752 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Icb9a6e9ca04b7ed633480a17973d1dcb126d02e0
Reviewed-on: https://chromium-review.googlesource.com/940742
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543048}
[modify] https://crrev.com/f8781418ce126ab08de6a82eef749d4c3bef5080/chrome/browser/resources/chromeos/zip_archiver/js/app.js
[modify] https://crrev.com/f8781418ce126ab08de6a82eef749d4c3bef5080/chrome/browser/resources/chromeos/zip_archiver/js/decompressor.js
[modify] https://crrev.com/f8781418ce126ab08de6a82eef749d4c3bef5080/chrome/browser/resources/chromeos/zip_archiver/js/main.js

Status: Fixed (was: Started)
I think removal of passwords memorized by ZIP unpacker (the older extension) should be done at the same time as removing the old extension itself. Filed  Issue 821761  for the task.
Project Member

Comment 22 by bugdroid1@chromium.org, May 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83eea24bb70cd3acb379a92e45003e5aa03075a7

commit 83eea24bb70cd3acb379a92e45003e5aa03075a7
Author: Tatsuhisa Yamaguchi <yamaguchi@google.com>
Date: Tue May 08 11:59:09 2018

Prevent runtime errors on suspend events.

unpacker.app.onSuspend was removed at f878141 and no longer exists.
https://chromium-review.googlesource.com/940742

Bug:  839158 , 789073 , 803752 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I973b7728095cddcceab2e58c06ca07986a6bd9e6
Reviewed-on: https://chromium-review.googlesource.com/1049417
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Yuki Awano <yawano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556763}
[modify] https://crrev.com/83eea24bb70cd3acb379a92e45003e5aa03075a7/chrome/browser/resources/chromeos/zip_archiver/js/background.js

Sign in to add a comment