Skip to content

cert check #19

Description

@ybrs

🛠️ Refactor suggestion

Improve error handling and certificate validation in TLS setup.

The TLS setup function has some areas for improvement:

  1. The certificate and key loading should validate that at least one certificate is present
  2. The ALPN protocol setting should handle potential errors
  3. Consider adding validation for certificate-key pair compatibility

Apply this diff to improve the implementation:

 fn setup_tls(cert_path: &str, key_path: &str) -> Result<Arc<TlsAcceptor>, IOError> {
     let cert = certs(&mut BufReader::new(File::open(cert_path)?))
         .collect::<Result<Vec<CertificateDer>, IOError>>()?;

+    if cert.is_empty() {
+        return Err(IOError::new(ErrorKind::InvalidInput, "No certificates found in file"));
+    }

     let key = pkcs8_private_keys(&mut BufReader::new(File::open(key_path)?))
         .map(|key| key.map(PrivateKeyDer::from))
-        .collect::<Result<Vec<PrivateKeyDer>, IOError>>()?
-        .remove(0);
+        .collect::<Result<Vec<PrivateKeyDer>, IOError>>()?;
+
+    if key.is_empty() {
+        return Err(IOError::new(ErrorKind::InvalidInput, "No private keys found in file"));
+    }
+
+    let key = key.into_iter().next().unwrap();

     let mut config = ServerConfig::builder()
         .with_no_client_auth()
         .with_single_cert(cert, key)
         .map_err(|err| IOError::new(ErrorKind::InvalidInput, err))?;

     config.alpn_protocols = vec![b"postgresql".to_vec()];

     Ok(Arc::new(TlsAcceptor::from(Arc::new(config))))
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

fn setup_tls(cert_path: &str, key_path: &str) -> Result<Arc<TlsAcceptor>, IOError> {
    let cert = certs(&mut BufReader::new(File::open(cert_path)?))
        .collect::<Result<Vec<CertificateDer>, IOError>>()?;

    if cert.is_empty() {
        return Err(IOError::new(ErrorKind::InvalidInput, "No certificates found in file"));
    }

    let key = pkcs8_private_keys(&mut BufReader::new(File::open(key_path)?))
        .map(|key| key.map(PrivateKeyDer::from))
        .collect::<Result<Vec<PrivateKeyDer>, IOError>>()?;

    if key.is_empty() {
        return Err(IOError::new(ErrorKind::InvalidInput, "No private keys found in file"));
    }

    let key = key.into_iter().next().unwrap();

    let mut config = ServerConfig::builder()
        .with_no_client_auth()
        .with_single_cert(cert, key)
        .map_err(|err| IOError::new(ErrorKind::InvalidInput, err))?;

    config.alpn_protocols = vec![b"postgresql".to_vec()];

    Ok(Arc::new(TlsAcceptor::from(Arc::new(config))))
}
🤖 Prompt for AI Agents
In src/lib.rs around lines 521 to 538, improve the TLS setup function by first
checking that the certificate vector is not empty after loading, returning an
appropriate error if no certificates are found. Similarly, verify that the
private key vector contains at least one key before removing the first element.
When setting the ALPN protocols, handle any potential errors that may arise from
this operation. Additionally, add validation to ensure the loaded certificate
and private key are compatible, returning an error if they do not match. These
changes will enhance error handling and ensure the TLS configuration is valid.

Originally posted by @coderabbitai[bot] in #18 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions