Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pgdog/src/auth/gssapi/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ impl GssapiContext {

// Create the desired mechanisms set
let mut desired_mechs = OidSet::new()
.map_err(|e| GssapiError::LibGssapi(format!("Failed to create OidSet: {}", e)))?;
.map_err(|e| GssapiError::LibGssapi(format!("failed to create OidSet: {}", e)))?;
desired_mechs
.add(&GSS_MECH_KRB5)
.map_err(|e| GssapiError::LibGssapi(format!("Failed to add mechanism: {}", e)))?;
.map_err(|e| GssapiError::LibGssapi(format!("failed to add mechanism: {}", e)))?;

// Acquire credentials from the cache that TicketManager populated
// Pass None to use the default principal from the cache
Expand All @@ -67,7 +67,7 @@ impl GssapiContext {
Some(&desired_mechs),
)
.map_err(|e| {
GssapiError::CredentialAcquisitionFailed(format!("Failed for {}: {}", principal, e))
GssapiError::CredentialAcquisitionFailed(format!("failed for {}: {}", principal, e))
})?;

// Parse target service principal (use KRB5_PRINCIPAL to avoid hostname canonicalization)
Expand Down Expand Up @@ -123,7 +123,7 @@ impl GssapiContext {
self.inner
.source_name()
.map(|name| name.to_string())
.map_err(|e| GssapiError::ContextError(format!("Failed to get client name: {}", e)))
.map_err(|e| GssapiError::ContextError(format!("failed to get client name: {}", e)))
}
}

Expand Down
50 changes: 13 additions & 37 deletions pgdog/src/auth/gssapi/error.rs
Original file line number Diff line number Diff line change
@@ -1,79 +1,55 @@
//! GSSAPI-specific error types

use std::fmt;
use std::path::PathBuf;
use thiserror::Error;

/// Result type for GSSAPI operations
pub type Result<T> = std::result::Result<T, GssapiError>;

/// GSSAPI-specific errors
#[derive(Debug)]
#[derive(Debug, Error)]
pub enum GssapiError {
/// Keytab file not found
#[error("keytab file not found: {0}")]
KeytabNotFound(PathBuf),

/// Invalid principal name
#[error("invalid principal: {0}")]
InvalidPrincipal(String),

/// Ticket has expired
#[error("kerberos ticket has expired")]
TicketExpired,

/// Failed to acquire credentials
#[error("failed to acquire credentials: {0}")]
CredentialAcquisitionFailed(String),

/// GSSAPI context error
#[error("GSSAPI context error: {0}")]
ContextError(String),

/// Token processing error
#[error("token processing error: {0}")]
TokenError(String),

/// Refresh failed
#[error("ticket refresh failed: {0}")]
RefreshFailed(String),

/// Internal libgssapi error
#[error("GSSAPI library error: {0}")]
LibGssapi(String),

/// I/O error
Io(std::io::Error),
#[error("{0}")]
Io(#[from] std::io::Error),

/// Configuration error
#[error("configuration error: {0}")]
Config(String),
}

impl fmt::Display for GssapiError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::KeytabNotFound(path) => write!(f, "Keytab file not found: {}", path.display()),
Self::InvalidPrincipal(principal) => write!(f, "Invalid principal: {}", principal),
Self::TicketExpired => write!(f, "Kerberos ticket has expired"),
Self::CredentialAcquisitionFailed(msg) => {
write!(f, "Failed to acquire credentials: {}", msg)
}
Self::ContextError(msg) => write!(f, "GSSAPI context error: {}", msg),
Self::TokenError(msg) => write!(f, "Token processing error: {}", msg),
Self::RefreshFailed(msg) => write!(f, "Ticket refresh failed: {}", msg),
Self::LibGssapi(msg) => write!(f, "GSSAPI library error: {}", msg),
Self::Io(err) => write!(f, "I/O error: {}", err),
Self::Config(msg) => write!(f, "Configuration error: {}", msg),
}
}
}

impl std::error::Error for GssapiError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self {
Self::Io(err) => Some(err),
_ => None,
}
}
}

impl From<std::io::Error> for GssapiError {
fn from(err: std::io::Error) -> Self {
Self::Io(err)
}
}

// Convert libgssapi errors when we implement the actual functionality
#[cfg(feature = "gssapi")]
impl From<libgssapi::error::Error> for GssapiError {
Expand Down
18 changes: 7 additions & 11 deletions pgdog/src/auth/gssapi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,19 @@ pub async fn handle_gssapi_auth(
);
let mut server = server.lock().await;

tracing::debug!("Calling server.accept()");
tracing::debug!("calling server.accept()");
match server.accept(&client_token)? {
Some(response_token) => {
// Check if authentication is complete despite having a token
if server.is_complete() {
let principal = server
.client_principal()
.ok_or_else(|| {
tracing::error!("Context complete but no principal found");
GssapiError::ContextError("No client principal found".to_string())
GssapiError::ContextError("no client principal found".to_string())
})?
.to_string();

tracing::info!(
tracing::debug!(
"Authentication complete (with final token), principal: {}",
principal
);
Expand All @@ -61,7 +60,7 @@ pub async fn handle_gssapi_auth(
Ok(response)
} else {
// More negotiation needed
tracing::info!(
tracing::debug!(
"server.accept returned token of {} bytes - negotiation continues",
response_token.len()
);
Expand All @@ -78,16 +77,13 @@ pub async fn handle_gssapi_auth(
}
None => {
// Authentication complete
tracing::info!("server.accept returned None - authentication complete");
tracing::debug!("server.accept returned None - authentication complete");
let principal = server
.client_principal()
.ok_or_else(|| {
tracing::error!("No client principal found in completed context");
GssapiError::ContextError("No client principal found".to_string())
})?
.ok_or_else(|| GssapiError::ContextError("No client principal found".to_string()))?
.to_string();

tracing::info!("Successfully extracted principal: {}", principal);
tracing::debug!("successfully extracted principal: {}", principal);
let response = GssapiResponse {
is_complete: true,
token: None,
Expand Down
30 changes: 15 additions & 15 deletions pgdog/src/auth/gssapi/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use super::error::{GssapiError, Result};
use std::path::Path;
#[cfg(feature = "gssapi")]
use std::sync::Arc;

#[cfg(feature = "gssapi")]
Expand Down Expand Up @@ -51,10 +52,10 @@ impl GssapiServer {

// Create the desired mechanisms set
let mut desired_mechs = OidSet::new()
.map_err(|e| GssapiError::LibGssapi(format!("Failed to create OidSet: {}", e)))?;
.map_err(|e| GssapiError::LibGssapi(format!("failed to create OidSet: {}", e)))?;
desired_mechs
.add(&GSS_MECH_KRB5)
.map_err(|e| GssapiError::LibGssapi(format!("Failed to add mechanism: {}", e)))?;
.map_err(|e| GssapiError::LibGssapi(format!("failed to add mechanism: {}", e)))?;

// Acquire credentials for the specified principal
Cred::acquire(
Expand All @@ -65,15 +66,15 @@ impl GssapiServer {
)
.map_err(|e| {
GssapiError::CredentialAcquisitionFailed(format!(
"Failed to acquire credentials for {}: {}",
"failed to acquire credentials for {}: {}",
principal, e
))
})?
} else {
// Use default service principal
Cred::acquire(None, None, CredUsage::Accept, None).map_err(|e| {
GssapiError::CredentialAcquisitionFailed(format!(
"Failed to acquire default credentials: {}",
"failed to acquire default credentials: {}",
e
))
})?
Expand Down Expand Up @@ -101,24 +102,24 @@ impl GssapiServer {
if self.is_complete {
tracing::warn!("GssapiServer::accept called but context already complete");
return Err(GssapiError::ContextError(
"Context already complete".to_string(),
"context already complete".to_string(),
));
}

// Create or reuse the server context
let mut ctx = match self.inner.take() {
Some(ctx) => {
tracing::debug!("Reusing existing server context");
tracing::debug!("reusing existing server context");
ctx
}
None => {
tracing::debug!("Creating new server context");
tracing::debug!("creating new server context");
ServerCtx::new(Some(self.credential.as_ref().clone()))
}
};

// Process the client token
tracing::debug!("Calling ctx.step with client token");
tracing::debug!("calling ctx.step with client token");
match ctx.step(client_token) {
Ok(Some(response)) => {
// More negotiation needed
Expand All @@ -133,20 +134,20 @@ impl GssapiServer {

// Check if context is actually established despite returning a token
if ctx.is_complete() {
tracing::warn!("Context is complete but still returned a token - this might confuse the client");
tracing::warn!("context is complete but still returned a token - this might confuse the client");
// Mark as complete and extract the principal
self.is_complete = true;
match ctx.source_name() {
Ok(name) => {
let principal = name.to_string();
tracing::info!(
tracing::debug!(
"Extracted client principal (with token): {}",
principal
);
self.client_principal = Some(principal);
}
Err(e) => {
tracing::error!("Failed to get client principal: {}", e);
tracing::error!("failed to get client principal: {}", e);
}
}
}
Expand All @@ -156,20 +157,19 @@ impl GssapiServer {
}
Ok(None) => {
// Context established successfully
tracing::info!("ctx.step returned None - GSSAPI context established successfully");
tracing::debug!("ctx.step returned None - GSSAPI context established successfully");
self.is_complete = true;

// Extract the client principal
match ctx.source_name() {
Ok(name) => {
let principal = name.to_string();
tracing::info!("Extracted client principal: {}", principal);
tracing::debug!("extracted client principal: {}", principal);
self.client_principal = Some(principal);
}
Err(e) => {
tracing::error!("Failed to get client principal: {}", e);
return Err(GssapiError::ContextError(format!(
"Failed to get client principal: {}",
"failed to get client principal: {}",
e
)));
}
Expand Down
7 changes: 4 additions & 3 deletions pgdog/src/auth/gssapi/ticket_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use super::error::{GssapiError, Result};
use parking_lot::RwLock;
use std::path::PathBuf;
#[cfg(feature = "gssapi")]
use std::sync::Arc;
use std::time::{Duration, Instant};

Expand Down Expand Up @@ -85,10 +86,10 @@ impl TicketCache {

// Create the desired mechanisms set
let mut desired_mechs = OidSet::new()
.map_err(|e| GssapiError::LibGssapi(format!("Failed to create OidSet: {}", e)))?;
.map_err(|e| GssapiError::LibGssapi(format!("failed to create OidSet: {}", e)))?;
desired_mechs
.add(&GSS_MECH_KRB5)
.map_err(|e| GssapiError::LibGssapi(format!("Failed to add mechanism: {}", e)))?;
.map_err(|e| GssapiError::LibGssapi(format!("failed to add mechanism: {}", e)))?;

// Acquire credentials from the keytab
let credential = Cred::acquire(
Expand All @@ -99,7 +100,7 @@ impl TicketCache {
)
.map_err(|e| {
GssapiError::CredentialAcquisitionFailed(format!(
"Failed for {}: {}",
"failed for {}: {}",
self.principal, e
))
})?;
Expand Down
21 changes: 12 additions & 9 deletions pgdog/src/auth/gssapi/ticket_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl TicketManager {
/// Get or acquire a ticket for a server
/// Returns Ok(()) when the credential cache is ready to use
#[cfg(feature = "gssapi")]
pub fn get_ticket(
pub async fn get_ticket(
&self,
server: impl Into<String>,
keytab: impl AsRef<Path>,
Expand All @@ -63,15 +63,16 @@ impl TicketManager {
std::env::set_var("KRB5CCNAME", &cache_path);

// Use kinit to get a ticket from the keytab into the unique cache
let output = std::process::Command::new("kinit")
let output = tokio::process::Command::new("kinit")
.arg("-kt")
.arg(&keytab_path)
.arg(&principal)
.env("KRB5CCNAME", &cache_path)
.env("KRB5_CONFIG", "/opt/homebrew/etc/krb5.conf")
.output()
.await
.map_err(|e| {
super::error::GssapiError::LibGssapi(format!("Failed to run kinit: {}", e))
super::error::GssapiError::LibGssapi(format!("failed to run kinit: {}", e))
})?;

if !output.status.success() {
Expand Down Expand Up @@ -99,7 +100,7 @@ impl TicketManager {

/// Get or acquire a ticket for a server (mock version)
#[cfg(not(feature = "gssapi"))]
pub fn get_ticket(
pub async fn get_ticket(
&self,
_server: impl Into<String>,
_keytab: impl AsRef<Path>,
Expand All @@ -126,10 +127,10 @@ impl TicketManager {
if cache.needs_refresh() {
match cache.refresh() {
Ok(()) => {
tracing::info!("Refreshed ticket for {}", server_clone);
tracing::info!("[gssapi] refreshed ticket for \"{}\"", server_clone);
}
Err(e) => {
tracing::error!("Failed to refresh ticket for {}: {}", server_clone, e);
tracing::error!("failed to refresh ticket for {}: {}", server_clone, e);
// Continue trying - the old ticket might still be valid
}
}
Expand Down Expand Up @@ -219,12 +220,14 @@ mod tests {
assert!(Arc::ptr_eq(&manager1, &manager2));
}

#[test]
fn test_cache_management() {
#[tokio::test]
async fn test_cache_management() {
let manager = TicketManager::new();

// This will fail because the keytab doesn't exist, but it tests the structure
let result = manager.get_ticket("server1:5432", "/nonexistent/keytab", "test@REALM");
let result = manager
.get_ticket("server1:5432", "/nonexistent/keytab", "test@REALM")
.await;
assert!(result.is_err());

// Even though ticket acquisition failed, the cache should not be stored
Expand Down
Loading
Loading