Skip to content

Conversation

@x032205
Copy link
Member

@x032205 x032205 commented Dec 12, 2025

certificate based auth support for ssh

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 12, 2025

Greptile Overview

Greptile Summary

This PR adds certificate-based authentication support for SSH connections in the PAM (Privileged Access Management) proxy. The implementation correctly parses SSH certificates and private keys to create a certificate signer for authentication.

Major changes:

  • Added Certificate field to PAM credentials data structures across API models, session management, and SSH proxy configuration
  • Implemented new "certificate" auth method case in SSH proxy that parses certificates using ssh.ParseAuthorizedKey() and creates a CertSigner
  • Updated error message to include "certificate" as a valid authentication method

Critical security concern:

  • The certificate authentication lacks validation checks for expiration (ValidBefore/ValidAfter), certificate type, and principals, potentially allowing unauthorized access with invalid certificates
  • Debug logging in pam-proxy.go:47-50 exposes sensitive credential data (private keys, passwords, tokens)

Documentation:

  • No /docs folder exists - how will customers discover this new certificate authentication feature?

Confidence Score: 2/5

  • This PR has critical security vulnerabilities related to missing certificate validation that could allow unauthorized access
  • Score reflects missing certificate validation (expiry, type, principals) which creates a security risk, plus credentials exposure in debug logs. The implementation is otherwise clean but these security issues must be addressed before merging.
  • Pay close attention to packages/pam/handlers/ssh/proxy.go - the certificate authentication case needs validation checks added before this can safely merge

Important Files Changed

File Analysis

Filename Score Overview
packages/pam/handlers/ssh/proxy.go 3/5 Added certificate authentication support for SSH with proper cert parsing and signer creation, but missing certificate validation checks
packages/pam/pam-proxy.go 5/5 Added certificate field to SSH proxy configuration and debug logging for credentials, straightforward changes

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

4 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@x032205 x032205 merged commit b39e859 into main Dec 19, 2025
3 checks passed
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.

3 participants