Skip to content

Zeroize potentially sensitive bytes#49

Merged
lvkv merged 2 commits into
mainfrom
lvkv/fixes-9
May 27, 2026
Merged

Zeroize potentially sensitive bytes#49
lvkv merged 2 commits into
mainfrom
lvkv/fixes-9

Conversation

@lvkv
Copy link
Copy Markdown
Owner

@lvkv lvkv commented May 27, 2026

Some points in this codebase handle potential secrets. These need debug impls that don't leak and, ideally, bytes that are zeroized on drop or before being libc free'd (context-dependent). This PR:

  • Adds a "secret bytes" type that wraps potentially sensitive bytes and zeroizes on drop
  • Returns this type whenever the value is likely to contain a potential secret
  • Internally, zeroizes PAM/libc-allocated buffers before freeing them
  • Redacts debug implementations where appropriate

This required writing a new secret_cstr_item macro, modeled after the existing cstr_item macro, to handle the AuthTok and OldAuthTok types.

@lvkv lvkv requested a review from Copilot May 27, 2026 03:16
@lvkv lvkv self-assigned this May 27, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a dedicated “secret bytes” container and threads it through PAM conversation/item APIs to reduce accidental leakage (notably via Debug) and to zeroize sensitive buffers on drop / before free().

Changes:

  • Add SecretBytes (owned, redacted Debug, zeroize-on-drop) and export it via a new secret module.
  • Update PAM conversation (Conv::send) to return SecretBytes and explicitly zero PAM/libc buffers before freeing.
  • Replace AuthTok / OldAuthTok item wrappers with secret-aware wrappers that redact Debug and can produce owned secret bytes.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
pam/src/secret.rs Adds SecretBytes zeroize-on-drop wrapper + redacted Debug.
pam/src/lib.rs Exports the new secret module.
pam/src/items.rs Adds secret_cstr_item! and uses it for AuthTok / OldAuthTok.
pam/src/conv.rs Returns SecretBytes from send() and zeroizes response buffers before free().
pam/examples/quiz.rs Updates example to use the new SecretBytes API (as_str).
pam/Cargo.toml Adds the zeroize dependency.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pam/src/items.rs Outdated
Comment thread pam/src/items.rs
Comment thread pam/src/conv.rs Outdated
Comment thread pam/src/secret.rs
Comment thread pam/src/secret.rs
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comment thread pam/src/conv.rs Outdated
Comment thread pam/src/conv.rs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread pam/src/items.rs Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lvkv lvkv merged commit c09eca9 into main May 27, 2026
5 checks passed
@lvkv lvkv deleted the lvkv/fixes-9 branch May 27, 2026 18:58
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