New issue
Advanced search Search tips

Issue 888096 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 1
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Support disabling NTLM by policy in SMB

Project Member Reported by zentaro@chromium.org, Sep 21

Issue description

Support disabling NTLM for native SMB.

This includes;

- Creating a new proto for mount configuration options
- Passing a new struct to mount that can contain mount  configuration options down through mounttracker/mountmanager
- Applying this setting to the context during mount
- Adding a policy to configure this
- Default to NTLM enabled for consumers and disabled for enterprise
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/f63dc2e08f0c619b185cd48de3dcf758bcd9d109

commit f63dc2e08f0c619b185cd48de3dcf758bcd9d109
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Sep 26 17:32:58 2018

smbprovider: Add MountConfigProto to directory_entry

- Added MountConfigProto since MountConfig options will be passed from chrome-side.

BUG= chromium:888096 
TEST=emerges

Change-Id: Ia5514b53b4070738aeb9ff1b55f212d9b90071ec
Reviewed-on: https://chromium-review.googlesource.com/1239199
Commit-Ready: jimmy gong <jimmyxgong@chromium.org>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/f63dc2e08f0c619b185cd48de3dcf758bcd9d109/dbus/smbprovider/directory_entry.proto

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/cc05b599d7c165a2836f16b3a17efd92c27d5227

commit cc05b599d7c165a2836f16b3a17efd92c27d5227
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:54 2018

smbprovider: Create MountConfig struct

- MountConfig is a struct that holds the mount options a user can set.
- The first option is a bool to set NTLM as the fallback authentication
  protocol.
- Future CL's may include more options.

BUG= chromium:888096 
TEST=emerges

Change-Id: I5c456d967f5a5ff0dd9ca694416a9b9bca4e1497
Reviewed-on: https://chromium-review.googlesource.com/1239613
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[add] https://crrev.com/cc05b599d7c165a2836f16b3a17efd92c27d5227/smbprovider/mount_config.h

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/28116681e9c44fb674c7911a82dfd9158e866628

commit 28116681e9c44fb674c7911a82dfd9158e866628
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:55 2018

smbprovider: Add MountConfigOptions validation method

- MountConfigOptionsProto is a new proto used for MountConfig.

BUG= chromium:888096 
TEST=emerges

Change-Id: I004bb1f89fa8641a10a1e7f90bdb9fcf1c686c98
Reviewed-on: https://chromium-review.googlesource.com/1239614
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/28116681e9c44fb674c7911a82dfd9158e866628/smbprovider/proto.h
[modify] https://crrev.com/28116681e9c44fb674c7911a82dfd9158e866628/smbprovider/proto.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/3f54c73fe2428f7fb159e0f15cea35ba3de0b5fb

commit 3f54c73fe2428f7fb159e0f15cea35ba3de0b5fb
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:55 2018

smbprovider: Add CreateMountConfigProto to test helper

- Added in anticipation of testing MountConfig for SmbProviderTest.

BUG= chromium:888096 
TEST=emerges

Change-Id: Ib0d8f4723445da29556b2b23c19f91e9a1af5abd
Reviewed-on: https://chromium-review.googlesource.com/1239615
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/3f54c73fe2428f7fb159e0f15cea35ba3de0b5fb/smbprovider/smbprovider_test_helper.h
[modify] https://crrev.com/3f54c73fe2428f7fb159e0f15cea35ba3de0b5fb/smbprovider/smbprovider_test_helper.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/dc08cca287eab6c8753203d7b90c76ac28d569b8

commit dc08cca287eab6c8753203d7b90c76ac28d569b8
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:56 2018

smbprovider: Add MountConfig as a parameter to MountManager::AddMount

- MountManager::AddMount will now require a MountConfig as an input
  parameter.
- This is to allow AddMount to push a MountConfig down to
  CreateSambaInterface. Which will then create a SambaInterface based off
  of MountConfig's options.

BUG= chromium:888096 
TEST=emerges

Change-Id: I32a9f8904bf5a38a2534d973ccae12b3489eaaf5
Reviewed-on: https://chromium-review.googlesource.com/1239616
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/dc08cca287eab6c8753203d7b90c76ac28d569b8/smbprovider/mount_manager.cc
[modify] https://crrev.com/dc08cca287eab6c8753203d7b90c76ac28d569b8/smbprovider/mount_manager.h
[modify] https://crrev.com/dc08cca287eab6c8753203d7b90c76ac28d569b8/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/dc08cca287eab6c8753203d7b90c76ac28d569b8/smbprovider/smbprovider.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/fdb23fd16a90c66f2a9e68facb29940b73b8aa39

commit fdb23fd16a90c66f2a9e68facb29940b73b8aa39
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:57 2018

smbprovider: Add MountConfigProto to SmbProvider::Mount

- Since MountOptionProto has a new member variable, MountConfigProto, it will
  now be part of mounting a share.
- Creates a MountConfig from the proto and will pass it down to AddMount
  to correctly set the MountConfig options.

BUG= chromium:888096 
TEST=unit tests

Change-Id: I5c33859d32df8ccf2fc449bce148321e7e69dfdf
Reviewed-on: https://chromium-review.googlesource.com/1239617
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/fdb23fd16a90c66f2a9e68facb29940b73b8aa39/smbprovider/smbprovider.cc
[modify] https://crrev.com/fdb23fd16a90c66f2a9e68facb29940b73b8aa39/smbprovider/smbprovider.h
[modify] https://crrev.com/fdb23fd16a90c66f2a9e68facb29940b73b8aa39/smbprovider/proto.h

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/0541ef8087a1924210aa22dbc191e93039a4b332

commit 0541ef8087a1924210aa22dbc191e93039a4b332
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:56 2018

smbprovider: Add MountConfig as a parameter to MountManger::Remount

- MountManager::Remount will now require a MountConfig as an input parameter.

BUG= chromium:888096 
TEST=emerges

Change-Id: I8804adbfdaaa3a090f093f918e8b49df4d6f9e0c
Reviewed-on: https://chromium-review.googlesource.com/1249669
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/0541ef8087a1924210aa22dbc191e93039a4b332/smbprovider/mount_manager.cc
[modify] https://crrev.com/0541ef8087a1924210aa22dbc191e93039a4b332/smbprovider/mount_manager.h
[modify] https://crrev.com/0541ef8087a1924210aa22dbc191e93039a4b332/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/0541ef8087a1924210aa22dbc191e93039a4b332/smbprovider/smbprovider.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/20fceff819ff0f0aa73925f409d753fe90d1635e

commit 20fceff819ff0f0aa73925f409d753fe90d1635e
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:57 2018

smbprovider: Add MountConfigProto to SmbProvider::Remount

- MountConfigProto will now be part of Remount.
- Similar to AddMount, will create a MountConfig from the proto.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ieab9d9f7d43ad621d86b49817ec2d9658c6eb4c7
Reviewed-on: https://chromium-review.googlesource.com/1249670
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/20fceff819ff0f0aa73925f409d753fe90d1635e/smbprovider/smbprovider.cc
[modify] https://crrev.com/20fceff819ff0f0aa73925f409d753fe90d1635e/smbprovider/smbprovider.h

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/8a5968495174c65bc1ec0743882b804d078e20d4

commit 8a5968495174c65bc1ec0743882b804d078e20d4
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:58 2018

smbprovider: Add MountConfig to SambaInterfaceFactoryFunction's parameter

- Adding MountConfig to SambaInterfaceFactoryFunction adds no funcationality
  in this CL.
- Future CL's will use the MountConfig to set the SambaInterface
  options.
- Moved test SambaInterfaceFactoryFunction to public test space, this is in
  preparation of testing setting MountConfig.

BUG= chromium:888096 
TEST=unit tests

Change-Id: I75c11b853106283f6df6e302ac32f60770827343
Reviewed-on: https://chromium-review.googlesource.com/1249671
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/8a5968495174c65bc1ec0743882b804d078e20d4/smbprovider/mount_manager.cc
[modify] https://crrev.com/8a5968495174c65bc1ec0743882b804d078e20d4/smbprovider/mount_manager.h
[modify] https://crrev.com/8a5968495174c65bc1ec0743882b804d078e20d4/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/8a5968495174c65bc1ec0743882b804d078e20d4/smbprovider/smbprovider_main.cc
[modify] https://crrev.com/8a5968495174c65bc1ec0743882b804d078e20d4/smbprovider/smbprovider_test.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/7a1e5d7628b1aa8b0caa6fb365d504847f8e8669

commit 7a1e5d7628b1aa8b0caa6fb365d504847f8e8669
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 05 22:42:53 2018

system_api: Add MountConfigProto to RemountOptionsProto

Added in anticipation of using MountConfig in Remount.

BUG= chromium:888096 
TEST=emerges

Change-Id: I296cc0d4a05585d5965a8b5010cbd98a6f767d70
Reviewed-on: https://chromium-review.googlesource.com/1260223
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Reviewed-by: Bailey Berro <baileyberro@chromium.org>

[modify] https://crrev.com/7a1e5d7628b1aa8b0caa6fb365d504847f8e8669/system_api/dbus/smbprovider/directory_entry.proto

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 10

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/241df096ff43cc8c64052a19a5866b5a74a0d4d9

commit 241df096ff43cc8c64052a19a5866b5a74a0d4d9
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Wed Oct 10 04:14:23 2018

smbprovider: Add MountConfig to SambaInterfaceImpl::Create input params

- Use the newly added MountConfig argument to call SambaInterfaceImpl::Create
  which sets the bit enabling/disabling NTLM.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ie111a4c61b9af092ab5abf5a834556a4d972dd1e
Reviewed-on: https://chromium-review.googlesource.com/1249672
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/241df096ff43cc8c64052a19a5866b5a74a0d4d9/smbprovider/smbprovider_main.cc
[modify] https://crrev.com/241df096ff43cc8c64052a19a5866b5a74a0d4d9/smbprovider/samba_interface_impl.h
[modify] https://crrev.com/241df096ff43cc8c64052a19a5866b5a74a0d4d9/smbprovider/samba_interface_impl.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Oct 11

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

commit 34c9d33dbf6e3e658ba781c349a866de40246186
Author: jimmy <jimmyxgong@chromium.org>
Date: Thu Oct 11 16:26:34 2018

Add policy for NTLM authentication protocol for SMB mounts

This changes adds an enterprise policy that disables NTLM authentication
protocol for enterprise-managed users and allows them to manually turn
it on.
For non-enterprise users, it will be enabled by default.

Bug:  chromium:888096 
Change-Id: If42e57b9242600ab9b51d61874bf810e2d32a804
Reviewed-on: https://chromium-review.googlesource.com/c/1256002
Reviewed-by: Bartosz Fabianowski <bartfab@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598793}
[modify] https://crrev.com/34c9d33dbf6e3e658ba781c349a866de40246186/chrome/browser/chromeos/smb_client/smb_service.cc
[modify] https://crrev.com/34c9d33dbf6e3e658ba781c349a866de40246186/chrome/browser/policy/configuration_policy_handler_list_factory.cc
[modify] https://crrev.com/34c9d33dbf6e3e658ba781c349a866de40246186/chrome/common/pref_names.cc
[modify] https://crrev.com/34c9d33dbf6e3e658ba781c349a866de40246186/chrome/common/pref_names.h
[modify] https://crrev.com/34c9d33dbf6e3e658ba781c349a866de40246186/chrome/test/data/policy/policy_test_cases.json
[modify] https://crrev.com/34c9d33dbf6e3e658ba781c349a866de40246186/components/policy/resources/policy_templates.json
[modify] https://crrev.com/34c9d33dbf6e3e658ba781c349a866de40246186/tools/metrics/histograms/enums.xml

Project Member

Comment 13 by bugdroid1@chromium.org, Oct 11

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

commit fce291e7658f5e6e07230310af682b0bae376f16
Author: jimmy <jimmyxgong@chromium.org>
Date: Thu Oct 11 18:33:14 2018

Add IsNTLMAuthenticationEnabled to SmbService

IsNTLMAuthenticationEnabled checks if NTLM is enabled by policy.

Bug:  chromium:888096 
Change-Id: If42b2e2125590e3434cdb2be9d931cd4531f1ebe
Reviewed-on: https://chromium-review.googlesource.com/c/1268670
Commit-Queue: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598861}
[modify] https://crrev.com/fce291e7658f5e6e07230310af682b0bae376f16/chrome/browser/chromeos/smb_client/smb_service.cc
[modify] https://crrev.com/fce291e7658f5e6e07230310af682b0bae376f16/chrome/browser/chromeos/smb_client/smb_service.h

Project Member

Comment 16 by bugdroid1@chromium.org, Oct 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a53bdd8d0016724ec2302a4a1182ff2a987f3272

commit a53bdd8d0016724ec2302a4a1182ff2a987f3272
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Fri Oct 12 22:16:57 2018

smbprovider: Add MountConfig to MountManager::AddMount/Remount

- MountConfig will now be pushed to MountMananger::AddMount/Remount and then
  into a SambaInterfaceFactory in which will handle logic of enabling or
  disabling NTLM.
- MountManager::CreateSambaInterface now has a MountConfig input, which
  requires both AddMount and Remount to be updated at the same time.
- Added tests for testing enabling/disabling NTLM in Smbprovider and
  MountManager.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ifa0a81a8a0c9fe6a1ff1e22c7ab392667dd3e1df
Reviewed-on: https://chromium-review.googlesource.com/1239618
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/a53bdd8d0016724ec2302a4a1182ff2a987f3272/smbprovider/smbprovider_test_helper.h
[modify] https://crrev.com/a53bdd8d0016724ec2302a4a1182ff2a987f3272/smbprovider/mount_manager.cc
[modify] https://crrev.com/a53bdd8d0016724ec2302a4a1182ff2a987f3272/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/a53bdd8d0016724ec2302a4a1182ff2a987f3272/smbprovider/smbprovider_test_helper.cc
[modify] https://crrev.com/a53bdd8d0016724ec2302a4a1182ff2a987f3272/smbprovider/mount_manager.h
[modify] https://crrev.com/a53bdd8d0016724ec2302a4a1182ff2a987f3272/smbprovider/smbprovider_test.cc

Labels: Merge-Request-71
Project Member

Comment 18 by sheriffbot@chromium.org, Oct 13

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 19 by bugdroid1@chromium.org, Oct 15

Labels: merge-merged-release-R71-11151.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/22d7e232cc153c2fb2f1238ef8997d8f1f49dca9

commit 22d7e232cc153c2fb2f1238ef8997d8f1f49dca9
Author: Jimmy Gong <jimmyxgong@google.com>
Date: Mon Oct 15 19:49:28 2018

smbprovider: Add MountConfig to MountManager::AddMount/Remount

- MountConfig will now be pushed to MountMananger::AddMount/Remount and then
  into a SambaInterfaceFactory in which will handle logic of enabling or
  disabling NTLM.
- MountManager::CreateSambaInterface now has a MountConfig input, which
  requires both AddMount and Remount to be updated at the same time.
- Added tests for testing enabling/disabling NTLM in Smbprovider and
  MountManager.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ifa0a81a8a0c9fe6a1ff1e22c7ab392667dd3e1df
Reviewed-on: https://chromium-review.googlesource.com/1239618
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
(cherry picked from commit a53bdd8d0016724ec2302a4a1182ff2a987f3272)
Reviewed-on: https://chromium-review.googlesource.com/c/1281184
Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/22d7e232cc153c2fb2f1238ef8997d8f1f49dca9/smbprovider/smbprovider_test_helper.h
[modify] https://crrev.com/22d7e232cc153c2fb2f1238ef8997d8f1f49dca9/smbprovider/mount_manager.cc
[modify] https://crrev.com/22d7e232cc153c2fb2f1238ef8997d8f1f49dca9/smbprovider/mount_manager_test.cc
[modify] https://crrev.com/22d7e232cc153c2fb2f1238ef8997d8f1f49dca9/smbprovider/smbprovider_test_helper.cc
[modify] https://crrev.com/22d7e232cc153c2fb2f1238ef8997d8f1f49dca9/smbprovider/mount_manager.h
[modify] https://crrev.com/22d7e232cc153c2fb2f1238ef8997d8f1f49dca9/smbprovider/smbprovider_test.cc

Status: Fixed (was: Started)
Project Member

Comment 21 by sheriffbot@chromium.org, Oct 17

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 22 by sheriffbot@chromium.org, Oct 22

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Started (was: Fixed)
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/68369a8203f5ebb3798c40d5a4e8a7fa56f3826d

commit 68369a8203f5ebb3798c40d5a4e8a7fa56f3826d
Author: jimmy <jimmyxgong@google.com>
Date: Tue Oct 23 20:01:31 2018

smbprovider: Fix leftover TODOs for ConvertToMountConfig

- This addresses leftover TODOs from ConvertToMountConfig. Because there
  should now be a MountConfig for every MountOption, there is no need to
  check for the MountConfigProto.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ie44bfb2c1065b3e0f224a9747f6da572ae33eb4e
Reviewed-on: https://chromium-review.googlesource.com/1281183
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/68369a8203f5ebb3798c40d5a4e8a7fa56f3826d/smbprovider/smbprovider_test_helper.h
[modify] https://crrev.com/68369a8203f5ebb3798c40d5a4e8a7fa56f3826d/smbprovider/smbprovider.cc
[modify] https://crrev.com/68369a8203f5ebb3798c40d5a4e8a7fa56f3826d/smbprovider/proto.h
[modify] https://crrev.com/68369a8203f5ebb3798c40d5a4e8a7fa56f3826d/smbprovider/smbprovider_test_helper.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac

commit a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac
Author: jimmy <jimmyxgong@google.com>
Date: Tue Oct 23 20:01:30 2018

smbprovider: Add check to verify that MountConfigProto is in MountOptionsProto

- Addresses previous TODO comment about verifying that MountConfigProto
  must exist in MountOptionsProto.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ic340956e4c82b64c05ee2ab956973d6107227701
Reviewed-on: https://chromium-review.googlesource.com/1282007
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac/smbprovider/proto.cc
[modify] https://crrev.com/a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac/smbprovider/smbprovider_test_helper.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Oct 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac

commit a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac
Author: jimmy <jimmyxgong@google.com>
Date: Tue Oct 23 20:01:30 2018

smbprovider: Add check to verify that MountConfigProto is in MountOptionsProto

- Addresses previous TODO comment about verifying that MountConfigProto
  must exist in MountOptionsProto.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ic340956e4c82b64c05ee2ab956973d6107227701
Reviewed-on: https://chromium-review.googlesource.com/1282007
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac/smbprovider/proto.cc
[modify] https://crrev.com/a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac/smbprovider/smbprovider_test_helper.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Oct 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/05e2e4e9c1f3413639d7874e87f9f91e18d7f294

commit 05e2e4e9c1f3413639d7874e87f9f91e18d7f294
Author: jimmy <jimmyxgong@google.com>
Date: Fri Oct 26 21:29:13 2018

smbprovider: Add check to verify that MountConfigProto is in MountOptionsProto

- Addresses previous TODO comment about verifying that MountConfigProto
  must exist in MountOptionsProto.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ic340956e4c82b64c05ee2ab956973d6107227701
Reviewed-on: https://chromium-review.googlesource.com/1282007
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
(cherry picked from commit a1a6be1a4bf2bf14fc373d8f5b96f92ddd1c3eac)
Reviewed-on: https://chromium-review.googlesource.com/c/1303115
Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/05e2e4e9c1f3413639d7874e87f9f91e18d7f294/smbprovider/proto.cc
[modify] https://crrev.com/05e2e4e9c1f3413639d7874e87f9f91e18d7f294/smbprovider/smbprovider_test_helper.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Nov 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/78beb378ede9d1baf9ae61ed1249cbffb13bd0a6

commit 78beb378ede9d1baf9ae61ed1249cbffb13bd0a6
Author: jimmy <jimmyxgong@google.com>
Date: Thu Nov 01 18:55:51 2018

smbprovider: Fix leftover TODOs for ConvertToMountConfig

- This addresses leftover TODOs from ConvertToMountConfig. Because there
  should now be a MountConfig for every MountOption, there is no need to
  check for the MountConfigProto.

BUG= chromium:888096 
TEST=unit tests

Change-Id: Ie44bfb2c1065b3e0f224a9747f6da572ae33eb4e
Reviewed-on: https://chromium-review.googlesource.com/1281183
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: jimmy gong <jimmyxgong@chromium.org>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
(cherry picked from commit 68369a8203f5ebb3798c40d5a4e8a7fa56f3826d)
Reviewed-on: https://chromium-review.googlesource.com/c/1312648
Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org>
Tested-by: Zentaro Kavanagh <zentaro@chromium.org>

[modify] https://crrev.com/78beb378ede9d1baf9ae61ed1249cbffb13bd0a6/smbprovider/smbprovider_test_helper.h
[modify] https://crrev.com/78beb378ede9d1baf9ae61ed1249cbffb13bd0a6/smbprovider/smbprovider.cc
[modify] https://crrev.com/78beb378ede9d1baf9ae61ed1249cbffb13bd0a6/smbprovider/proto.h
[modify] https://crrev.com/78beb378ede9d1baf9ae61ed1249cbffb13bd0a6/smbprovider/smbprovider_test_helper.cc

Status: Fixed (was: Started)
Everything is merged now to 71.
Hi Jimmy,

Does it require manual check? If yes, could you please provide verification steps?
Yes this needs a manual check.

You need a file share that is not domain joined. If you are using a NAS device it won't be, but if you are using a Windows device as the share you can't use one that is joined to a domain because it will use Kerberos instead of NTLM.

To test, make sure your NAS has a share that is setup to use authentication with a username and password. 

network_file_shares.ntlm_share_authentication.enabled

By default this policy should be false, and you will not be able to authenticate to the share.

By enabling the policy you will be able to connect.


Status: Verified (was: Fixed)
Thanks, Zentaro!

I was able to verify this using YAPS and our NAS setup (go/smb-test-setup):

- NTLMShareAuthenticationEnabled = false -> not able to add file share with username and password
- NTLMShareAuthenticationEnabled = true -> able to add file share with username and password

Chrome OS: 11151.22.0
Chrome: 71.0.3578.36
Device: Santa

Sign in to add a comment