Skip to content

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

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

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

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

Scope SMB credential storage and lookup to the share name to ensure each share is authenticated with its own credentials.

Bug Fixes:

  • Isolate SMB keyring credentials by share name so different shares on the same server no longer reuse one another’s saved credentials.

Enhancements:

  • Preserve automatic credential retrieval for previously saved shares while leaving anonymous SMB and FTP/WebDAV mounting behavior unchanged.

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 isolation by deriving the share name from the URL and adding it as a Secret Service schema attribute used consistently when saving and retrieving credentials. Review the URL path parsing and compatibility implications: legacy entries without object will no longer match, requiring one-time reauthentication per share.

Sequence diagram for per-share SMB credential lookup and storage

sequenceDiagram
    participant Mounter as DNetworkMounter
    participant SecretService

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

File-Level Changes

Change Details Files
Adds the SMB share name to keyring credential identity and scopes both credential lookup and storage by that share.
  • Introduces the object schema attribute for SMB credentials.
  • Extracts the share component from the mount URL in login and save paths.
  • Includes the share attribute in Secret Service queries and stored credential attributes.
  • Preserves existing server/protocol/user/domain matching while preventing cross-share credential reuse.
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 found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/dfm-mount/private/dnetworkmounter.cpp" line_range="118" />
<code_context>
     QUrl u(address);
     QString protocol = u.scheme();
     QString host = u.host();
+    QString share = u.path().remove("/");

     GHashTable_autoptr query = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, g_free);
</code_context>
<issue_to_address>
**issue (bug_risk):** `u.path().remove("/")` removes every slash instead of extracting the SMB share component. For an address such as `smb://server/share/subdirectory`, the credential is keyed as `sharesubdirectory`, so it does not match the credential saved or queried for the actual share `share`; credentials therefore fail to auto-fill for valid SMB URLs containing a subpath, and distinct paths can be collapsed into the same key.

**Triggers:** When an SMB address contains a path below the share root, such as `smb://server/share/subdirectory`.

**Suggested fix:** Extract only the first non-empty path component as the share name, while preserving the rest of the URL path for mounting.
</issue_to_address>

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.

QUrl u(address);
QString protocol = u.scheme();
QString host = u.host();
QString share = u.path().remove("/");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (bug_risk): u.path().remove("/") removes every slash instead of extracting the SMB share component. For an address such as smb://server/share/subdirectory, the credential is keyed as sharesubdirectory, so it does not match the credential saved or queried for the actual share share; credentials therefore fail to auto-fill for valid SMB URLs containing a subpath, and distinct paths can be collapsed into the same key.

Triggers: When an SMB address contains a path below the share root, such as smb://server/share/subdirectory.

Suggested fix: Extract only the first non-empty path component as the share name, while preserving the rest of the URL path for mounting.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

AI 代码审查报告

总体评分:100分(通过阈值: 70分)

审查结果:通过


总体评价

本次提交为 Bug 修复(BUG-375291),目的为在 SMB 凭证管理中新增 share name 属性实现按共享级别隔离凭证。代码修改仅涉及单文件 src/dfm-mount/private/dnetworkmounter.cpp(+9/-2 行),在 smbSchema()loginPasswd()savePasswd() 三个函数中一致地新增 "object" 属性,逻辑正确,风格与现有代码完全一致,无安全漏洞。该修复本质上是一次安全改进,消除了同一服务器不同 SMB 共享之间凭证复用导致的信息泄露风险。


语法逻辑 ✓ 语法正确,逻辑清晰(25/25分)

本次修改目的明确:在 libsecret keyring 的 SMB 凭证 schema 中新增 "object" 属性(对应 share name),并在凭证的存储和查询流程中一致地使用该属性。

代码变更分析:

1.第46行新增常量 kSchemaObject 定义为 "object",与已有的 kSchemaUserkSchemaDomainkSchemaServerkSchemaProtocol 命名风格一致
2.第118行通过 u.path().remove("/") 从 URL 路径提取共享名称,对于 SMB URL(如 smb://server/share),QUrl::path() 返回 /shareremove("/") 正确去除前导斜杠得到 share
3.第121行将 share name 插入查询 hash table,与已有的 server、protocol 处理方式一致
4.第170-172行在 secret_password_lookup_sync 调用中新增 kSchemaObject, object.c_str() 参数,变参列表以 nullptr 正确终止
5.第189行在 savePasswd() 中同样提取 share name
6.第203-204行在 secret_password_store_sync 调用中新增 kSchemaObject 属性
7.第220行在 smbSchema() 中注册 sche.attributes[4],静态局部变量 sche 零初始化保证 attributes[5] 为哨兵值

未发现语法错误、逻辑缺陷或边界处理问题。


代码质量 ✓ 代码结构清晰,注释完整(25/25分)

1.新增代码完全遵循现有代码模式:常量命名使用 kSchema 前缀 + PascalCase,strdup + g_hash_table_insert 调用方式与已有属性处理一致
2.三个函数(loginPasswdsavePasswdsmbSchema)的修改保持了一致性,存储和查询两侧同步添加了新属性
3.无代码重复、无残留调试代码、无全局变量滥用
4.变量名 object 和常量名 kSchemaObject 语义清晰,与 libsecret 的 attribute 命名惯例一致


代码性能 ✓ 性能良好,资源使用合理(20/20分)

1.每次调用仅新增一次 QString::remove() 操作(O(n) 但 share name 通常很短)和一次 strdup + g_hash_table_insert(O(1)),开销可忽略
2.所有 strdup 分配的内存通过 GHashTableg_free destroy 函数自动释放,无内存泄漏风险
3.无阻塞主线程、无频繁系统调用、无重复计算


代码安全 ✓ 存在0个安全漏洞(30/30分)

存在0个安全漏洞

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

安全分析:

1.本次修改是一项安全改进:修复前,同一服务器上不同 SMB 共享的凭证会被错误复用(因 keyring 查询仅按 server + protocol 匹配),导致用户 A 的共享凭证可能被用于访问用户 B 的共享。修复后,凭证按 {server, protocol, user, domain, object(share name)} 五元组隔离
2.strdup(share.toStdString().c_str()) 调用安全:toStdString() 返回临时字符串,.c_str() 获取其内部指针,strdup 在临时对象销毁前完成拷贝,不存在悬空指针风险
3.secret_password_lookup_syncsecret_password_store_sync 的变参列表正确以 nullptr 终止,不存在参数越界
4.smbSchema()attributes[4] 赋值安全:SecretSchema 的属性数组大小为 SECRET_SCHEMA_ATTRIBUTE_N_ITEMS(通常为32),索引4远未越界

未发现安全漏洞。OCR 审查因网络问题未能执行代码拉取,使用空结果合并。


本报告由 AI 代码审查工具自动生成

@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 09afe91 into release/snipe Aug 28, 2026
28 of 29 checks passed
@Johnson-zs
Johnson-zs deleted the fix/smb-share-credential-isolation-snipe 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