Skip to content

fix: SMB共享凭证加入share name属性隔离 - #383

Merged
deepin-bot[bot] merged 1 commit into
masterfrom
fix/smb-share-credential-isolation-master
Aug 28, 2026
Merged

fix: SMB共享凭证加入share name属性隔离#383
deepin-bot[bot] merged 1 commit into
masterfrom
fix/smb-share-credential-isolation-master

Conversation

@Johnson-zs

@Johnson-zs Johnson-zs commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

fix: include share name in SMB keyring credentials

  1. Root cause: DNetworkMounter's keyring credential management
    (savePasswd/loginPasswd/smbSchema) did not include share name,
    so loginPasswd searched by {server, protocol} only and returned
    all saved credentials for the same server regardless of share
  2. Fix: add "object" attribute to smbSchema for share name, extract
    share from URL path in savePasswd and loginPasswd, include it in
    both store and search operations to isolate credentials per share
  3. Impact: credentials are now stored and searched per-share; old
    credentials without "object" attribute won't be matched, users
    need to re-enter password once per share after upgrade
  4. 方案层级: 根因层修复(5-Why 在 Why 4 收敛,根因层与症状层一致)

Log: fix SMB share only authenticating once across different users

Influence:

  1. Test accessing non-anonymous shares of different users on the
    same server, second share should prompt for authentication
  2. Test re-accessing a saved share, credential should auto-fill
  3. Test anonymous share access is unaffected
  4. Test FTP/WebDAV mounting is unaffected

fix: SMB共享凭证加入share name属性隔离

  1. 根因:DNetworkMounter 的 keyring 凭证管理(savePasswd/loginPasswd/
    smbSchema)未包含 share name 属性,loginPasswd 仅按
    {server, protocol} 搜索,返回同一服务器所有共享的已保存凭证
  2. 方案:在 smbSchema 中新增 "object" 属性用于 share name,
    savePasswd 和 loginPasswd 从 URL 提取 share name 并在存储和
    查询时附带,实现凭证按共享级别隔离
  3. 影响:凭证按共享级别存储和搜索,旧凭证不含 "object" 属性
    无法命中,升级后用户需对每个已保存共享重新输入一次密码
  4. 方案层级:根因层修复(5-Why 在 Why 4 收敛,根因层与症状层一致)

Log: 修复访问不同用户不同密码的SMB共享时仅鉴权一次的问题

Influence:

  1. 测试同一服务器不同用户的非匿名共享,访问第二个共享应弹出鉴权窗口
  2. 测试重复访问已保存的共享,凭证应自动填充无需再次输入
  3. 测试匿名共享访问不受影响
  4. 测试 FTP/WebDAV 挂载不受影响

PMS: BUG-375291

Summary by Sourcery

Bug Fixes:

  • Isolate SMB keyring credentials by share name so different shares on the same server use the correct credentials.

1. Root cause: DNetworkMounter's keyring credential management
   (savePasswd/loginPasswd/smbSchema) did not include share name,
   so loginPasswd searched by {server, protocol} only and returned
   all saved credentials for the same server regardless of share
2. Fix: add "object" attribute to smbSchema for share name, extract
   share from URL path in savePasswd and loginPasswd, include it in
   both store and search operations to isolate credentials per share
3. Impact: credentials are now stored and searched per-share; old
   credentials without "object" attribute won't be matched, users
   need to re-enter password once per share after upgrade
4. 方案层级: 根因层修复(5-Why 在 Why 4 收敛,根因层与症状层一致)

Log: fix SMB share only authenticating once across different users

Influence:
1. Test accessing non-anonymous shares of different users on the
   same server, second share should prompt for authentication
2. Test re-accessing a saved share, credential should auto-fill
3. Test anonymous share access is unaffected
4. Test FTP/WebDAV mounting is unaffected

fix: SMB共享凭证加入share name属性隔离

1. 根因:DNetworkMounter 的 keyring 凭证管理(savePasswd/loginPasswd/
   smbSchema)未包含 share name 属性,loginPasswd 仅按
   {server, protocol} 搜索,返回同一服务器所有共享的已保存凭证
2. 方案:在 smbSchema 中新增 "object" 属性用于 share name,
   savePasswd 和 loginPasswd 从 URL 提取 share name 并在存储和
   查询时附带,实现凭证按共享级别隔离
3. 影响:凭证按共享级别存储和搜索,旧凭证不含 "object" 属性
   无法命中,升级后用户需对每个已保存共享重新输入一次密码
4. 方案层级:根因层修复(5-Why 在 Why 4 收敛,根因层与症状层一致)

Log: 修复访问不同用户不同密码的SMB共享时仅鉴权一次的问题

Influence:
1. 测试同一服务器不同用户的非匿名共享,访问第二个共享应弹出鉴权窗口
2. 测试重复访问已保存的共享,凭证应自动填充无需再次输入
3. 测试匿名共享访问不受影响
4. 测试 FTP/WebDAV 挂载不受影响

PMS: BUG-375291
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Johnson-zs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sourcery-ai

sourcery-ai Bot commented Aug 28, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Fixes SMB credential leakage across shares on the same server by deriving the share name from the URL and including it in the keyring schema, searches, and saves. Existing credentials without the new attribute will not match and must be re-entered per share once after upgrade.

Sequence diagram for per-share SMB credential lookup and storage

sequenceDiagram
    participant Mounter as DNetworkMounter
    participant Keyring as SecretServiceKeyring

    Mounter->>Mounter: QUrl::path()
    Mounter->>Keyring: secret_password_lookup_sync(server, protocol, object=share)
    alt matching share credential exists
        Keyring-->>Mounter: saved password
    else no matching share credential
        Keyring-->>Mounter: no credential
    end
    Mounter->>Keyring: secret_password_store_sync(server, protocol, user, object=share)
    Keyring-->>Mounter: credential stored per share
Loading

File-Level Changes

Change Details Files
Adds the SMB share name to keyring credential identity and scopes credential lookup and storage per share.
  • Defines a new keyring schema attribute for the share name.
  • Extracts the share from the SMB URL path during credential lookup and save.
  • Includes the share attribute in libsecret search and store operations.
  • Preserves existing behavior for other network protocols by limiting the change to SMB credential schema handling.
src/dfm-mount/private/dnetworkmounter.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

AI 代码审查报告

项目: linuxdeepin/util-dfm
PR: #383 fix: SMB共享凭证加入share name属性隔离
分支: fix/smb-share-credential-isolation-master → master
作者: Johnson-zs
提交: 648bd4b
分析时间: 2026-08-28 13:32
分析模式: 全量分析
关联Bug: PMS BUG-375291


总体评价

项目 结果
总体评分 99/100
风险等级 None
审查结论 代码审查通过
评价原因 未发现安全漏洞,代码逻辑正确,精准修复了 SMB 凭证按共享级别隔离的问题,变更范围小且与修复目标高度一致

修改文件清单

文件 变更类型 新增行 删除行
src/dfm-mount/private/dnetworkmounter.cpp 修改 +9 -2

跳过文件统计:共 1 个文件修改,0 个文件跳过。


四维度评分详情

维度1:语法逻辑(25分,权重25%)

评分:25/25 ✓ 语法正确,逻辑清晰

变更精准修复了 SMB 共享凭证隔离问题,修改涉及三个核心函数 loginPasswdsavePasswdsmbSchema,逻辑链路完整:

1.新增常量 kSchemaObject(第46行)用于 SecretSchema 的 share name 属性定义
2.loginPasswd 函数(第118行)从 URL path 提取 share name,加入查询 hash table 和 secret_password_lookup_sync 调用
3.savePasswd 函数(第189行)从 URL path 提取 share name,加入 secret_password_store_sync 调用
4.smbSchema 函数(第220行)在 schema attributes 数组索引 [4] 注册 kSchemaObject 属性

u.path().remove("/") 对于 SMB URL smb://server/share 正确提取 sharesecret_password_lookup_syncsecret_password_store_sync 的变参列表格式 key, value, ..., nullptr 使用正确。


维度2:代码质量(25分,权重25%)

评分:24/25 ✓ 代码结构清晰,注释完整

1.新增属性 kSchemaObject 命名与现有 kSchemaServerkSchemaProtocol 等保持一致,代码风格统一
2.修改点精准,仅涉及凭证存储/查询/Schema 定义三个关键位置,无冗余变更
3.PR 描述非常详尽,包含根因分析(5-Why 分析)、修复方案、影响范围和测试用例
4.使用 strduptoStdString().c_str() 与现有代码模式一致

1.新增常量 kSchemaObject 缺少行内注释说明其用途为 share name,建议补充注释提升可读性

// 建议添加注释
static constexpr char kSchemaObject[] { "object" };  // SMB share name for credential isolation

维度3:代码性能(20分,权重20%)

评分:20/20 ✓ 性能良好,资源使用合理

1.u.path().remove("/") 是 O(n) 字符串操作,但 URL path 通常很短(<100字符),无性能影响
2.g_hash_table_insert 新增一项,O(1) 操作
3.secret_password_lookup_syncsecret_password_store_sync 新增一个属性参数,对 libsecret 调用性能无显著影响
4.无额外内存分配开销


维度4:代码安全(30分,权重30%)

评分:30/30 ✓ 存在0个安全漏洞

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个

安全分析:

1.strdup(share.toStdString().c_str()) 使用安全,toStdString() 创建临时 std::stringstrdup 在临时对象析构前已完成内容复制,无悬空指针风险
2.此修复实际增强了安全性——修复前同一服务器不同共享的凭证可能混用,修复后实现了凭证按共享级别隔离
3.代码仅操作内存中的字符串和调用 libsecret API,不涉及命令执行或文件路径操作,无注入风险
4.向后兼容性:PR 描述已说明旧凭证不含 object 属性无法命中,用户需重新输入密码,属预期行为


改进建议

// 建议:为 kSchemaObject 添加注释说明用途
-static constexpr char kSchemaObject[] { "object" };
+// SMB share name attribute for per-share credential isolation
+static constexpr char kSchemaObject[] { "object" };

审查结论

本次提交精准修复了 SMB 共享凭证无法按共享名隔离的 BUG(PMS BUG-375291),通过在 libsecret SecretSchema 中新增 "object" 属性并在存储和查询时携带 share name,实现了凭证的共享级别隔离。代码变更范围小(+9/-2),逻辑正确,风格与现有代码一致,无安全漏洞。PR 描述质量很高,包含完整的根因分析和测试方案。

@Johnson-zs

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit acfa543 into master Aug 28, 2026
25 of 29 checks passed
@Johnson-zs
Johnson-zs deleted the fix/smb-share-credential-isolation-master branch August 28, 2026 06:23
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

Successfully merging this pull request may close these issues.

2 participants