Skip to content

feat(pam): Kerberos authentication for MSSQL proxy#245

Open
saifsmailbox98 wants to merge 10 commits into
mainfrom
saif/pam-227-kerberos-auth-support-for-ms-sql
Open

feat(pam): Kerberos authentication for MSSQL proxy#245
saifsmailbox98 wants to merge 10 commits into
mainfrom
saif/pam-227-kerberos-auth-support-for-ms-sql

Conversation

@saifsmailbox98
Copy link
Copy Markdown
Contributor

@saifsmailbox98 saifsmailbox98 commented May 28, 2026

Description 📣

Gateway MSSQL proxy can now authenticate with SQL Server using Kerberos. Obtains tickets from the KDC via gokrb5/v8, wraps in SPNEGO, and sends in LOGIN7 — same SSPI slot as NTLM. The client-to-gateway leg is unchanged (dummy SQL auth).

Builds krb5.conf in-memory from realm and KDC address fields. Supports DNS-based KDC discovery when KDC address is omitted. Handles SQL Server's mutual auth round-trip. Wraps gokrb5 errors into actionable messages (KDC unreachable, clock skew, invalid credentials, unknown SPN).

Companion backend PR: Infisical/infisical#6638

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

# Tested against a real Windows Server 2022 + AD DC + SQL Server on AWS
# Verified: Kerberos auth, DNS discovery, wrong password/SPN/KDC errors,
# INI injection prevention, multiple clients (sqlcmd, DataGrip),
# auth_scheme=KERBEROS confirmed via sys.dm_exec_connections

Gateway MSSQL proxy now supports NTLM (Windows Authentication) in
addition to SQL auth. When AuthMethod is "ntlm", the proxy performs
a 3-message NTLM handshake with the server (negotiate, challenge,
authenticate) using go-ntlmssp, instead of sending SQL credentials
in LOGIN7.

The client-to-gateway leg is unchanged — clients always use SQL auth
with dummy credentials, and the proxy injects the real NTLM auth on
the server leg.
…x SSPI length sentinel

Extract SSPI token before checking for error tokens — the NTLM
challenge contains random binary that can match the 0xAA error byte.
Also fix the SSPI length boundary: 0xFFFF is a sentinel per TDS spec,
so use strict less-than.
go-ntlmssp uses splitNameForAuth to extract domain from the username.
Without the DOMAIN\ prefix, the domain is empty in the NTLMv2 hash
and authenticate message, which may fail on domain-member SQL Servers
authenticating against a remote DC.
Gateway MSSQL proxy now supports Kerberos via gokrb5/v8. Obtains
service ticket from KDC, wraps in SPNEGO, sends in LOGIN7. Handles
mutual auth round-trip. Actionable error messages for KDC/SPN/clock
issues.
…rt-for-mssql-in-pam' into saif/pam-227-kerberos-auth-support-for-ms-sql
Removed empty SSPI acknowledgement packet during Kerberos mutual auth
— server sends LoginAck without needing it. Added dial timeout and
i/o timeout patterns to wrapKerberosError for unreachable KDC.
@linear
Copy link
Copy Markdown

linear Bot commented May 28, 2026

PAM-227

@infisical-review-police
Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-245-feat-pam-kerberos-authentication-for-mssql-proxy

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

@gitguardian
Copy link
Copy Markdown

gitguardian Bot commented May 28, 2026

⚠️ GitGuardian has uncovered 4 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
9387833 Triggered Generic Password c15a998 packages/pam/handlers/rdp/native/src/rdcleanpath.rs View secret
33062794 Triggered Generic CLI Secret c15a998 packages/cmd/login_status_test.go View secret
33062794 Triggered Generic CLI Secret 28de0f5 packages/cmd/login_status_test.go View secret
9387833 Triggered Generic Password 28de0f5 packages/pam/handlers/rdp/native/src/rdcleanpath.rs View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR adds Kerberos (and NTLM) authentication support to the MSSQL PAM proxy. When authMethod is "kerberos", the gateway obtains a TGS ticket from the KDC via gokrb5/v8, wraps it in SPNEGO, and sends it in the TDS LOGIN7 SSPI slot; the client-facing leg is unchanged.

  • New auth paths: authenticateKerberos and authenticateNTLM are extracted alongside the existing authenticateSQL, each with a defer serverConn.Close() guard and appropriate multi-round-trip handling for Kerberos mutual auth.
  • krb5.conf built in-memory: buildKrb5Config constructs the configuration from realm and kdcAddress credentials, with basic INI-injection filtering; DNS-based KDC discovery is used when kdcAddress is empty.
  • New credential fields: Realm, KDCAddress, and SPN are added to the API model, session credentials struct, and proxy config; AuthMethod was already present.

Confidence Score: 3/5

The Kerberos path introduces an outbound connection to a backend-controlled host, which needs an allow-list before merging into a production gateway deployment.

The kdcAddress field flows from the backend API straight into a TCP dial with no network-scope enforcement. The gateway lives inside a private network, so a rogue or compromised backend can use that field to scan or contact internal hosts the gateway can reach. The rest of the changes — struct wiring, NTLM handshake, TDS encoding — look correct and the refactored defer serverConn.Close() pattern is a clean improvement over the previous scattered closes.

packages/pam/handlers/mssql/proxy.go — specifically buildKrb5Config and authenticateKerberos

Security Review

  • SSRF via kdcAddress (packages/pam/handlers/mssql/proxy.go, buildKrb5Config): The KDC address is taken directly from backend-provided credentials with no allow-list or network-scope validation. A compromised backend can supply any internal host, causing the gateway to open TCP connections to that host on port 88, enabling internal network probing via the gateway.

Important Files Changed

Filename Overview
packages/pam/handlers/mssql/proxy.go Adds NTLM and Kerberos authentication paths for MSSQL. SSRF risk: the backend-supplied kdcAddress is used to make outbound TCP connections without an allow-list.
packages/pam/handlers/mssql/tds.go Adds ExtractSSPIToken, SSPIData field in Login7Message, and SSPI encoding logic. ExtractSSPIToken returns a suffix slice rather than the exact NTLM token boundary.
packages/pam/session/credentials.go Adds Realm, KDCAddress, and SPN fields to PAMCredentials and maps them from the API response. Straightforward struct extension with no logic issues.
packages/api/model.go Adds Realm, KDCAddress, and SPN JSON fields to PAMSessionCredentials. Clean, consistent with existing optional-field pattern.
packages/pam/pam-proxy.go Wires Domain, Realm, KDCAddress, SPN, and AuthMethod credentials through to MssqlProxyConfig. No logic changes beyond field mapping.
go.mod Adds github.com/jcmturner/gokrb5/v8 and github.com/Azure/go-ntlmssp as direct dependencies, plus the transitive jcmturner sub-modules.

Reviews (1): Last reviewed commit: "fix(pam): remove empty SSPI ack + improv..." | Re-trigger Greptile

Comment on lines +486 to +516
func buildKrb5Config(realm, kdcAddress string) (*config.Config, error) {
for _, s := range []string{realm, kdcAddress} {
if strings.ContainsAny(s, "\n\r[]{}=") {
return nil, fmt.Errorf("invalid characters in Kerberos configuration")
}
}

dnsLookup := "true"
realmsSection := ""

if kdcAddress != "" {
dnsLookup = "false"
if !strings.Contains(kdcAddress, ":") {
kdcAddress = kdcAddress + ":88"
}
realmsSection = fmt.Sprintf(`
[realms]
%s = {
kdc = %s
}`, realm, kdcAddress)
}

cfgString := fmt.Sprintf(`
[libdefaults]
default_realm = %s
dns_lookup_kdc = %s
dns_lookup_realm = false
udp_preference_limit = 1
%s`, realm, dnsLookup, realmsSection)

return config.NewFromString(cfgString)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 security SSRF via backend-controlled KDC address

When kdcAddress is non-empty the gateway initiates a TCP connection to that host on port 88 as part of Kerberos ticket acquisition. Because kdcAddress originates from the Infisical backend (returned by CallPAMSessionCredentials), a compromised or rogue backend can direct the gateway to connect to any internal host — effectively using the gateway as an SSRF proxy. The gateway is typically deployed inside a private network segment, making this especially impactful: an attacker could probe internal services reachable from the gateway but not from the internet, or send Kerberos framing bytes to any service that happens to bind port 88. DNS-lookup mode (kdcAddress == "") is unaffected since no explicit host is dialled, but the explicit-KDC path has no allow-list or network-scope enforcement.

Context Used: Flag SSRF risks (source)

Comment on lines +58 to +65
// ExtractSSPIToken finds the NTLM token in a TDS server response by scanning for the NTLMSSP signature.
func ExtractSSPIToken(payload []byte) ([]byte, error) {
idx := bytes.Index(payload, ntlmsspSignature)
if idx < 0 {
return nil, fmt.Errorf("no NTLMSSP token found in server response")
}
return payload[idx:], nil
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 ExtractSSPIToken returns a suffix instead of the exact token

payload[idx:] includes every byte from the NTLMSSP signature to the end of the combined TDS payload, not just the NTLM challenge message itself. In typical SQL Server SSPI responses the challenge token is the last item in the stream, so this works in practice. However, if the server follows the SSPI token with other TDS tokens (e.g., an ENVCHANGE), those extra bytes are silently appended to the slice passed to ntlmssp.ProcessChallenge. Most NTLM libraries read only the bytes they need and tolerate trailing data, but this is an implicit assumption that could break with future library versions or unusual server configurations.

Comment on lines +486 to +491
func buildKrb5Config(realm, kdcAddress string) (*config.Config, error) {
for _, s := range []string{realm, kdcAddress} {
if strings.ContainsAny(s, "\n\r[]{}=") {
return nil, fmt.Errorf("invalid characters in Kerberos configuration")
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 INI injection validation is missing whitespace and comment characters

The validation blocks \n, \r, [, ], {, }, and =, but allows tab and space characters. In krb5.conf format, kdc = host1.example.com\t with a trailing tab is unusual but typically harmless. More importantly, the # character (comment marker in krb5.conf) is not filtered. A kdcAddress value like kdc.example.com # attacker-note would embed a comment stub in the kdc = line; this is cosmetically wrong but also not exploitable since newlines are filtered. Nonetheless, tightening the allow-list to [A-Za-z0-9._:\-]+ for kdcAddress and [A-Za-z0-9._\-]+ for realm would eliminate ambiguity and future edge cases without losing any valid input.

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.

1 participant