From 31a41eb89ad14ab4ebc577f8c99fe39b39fe59b8 Mon Sep 17 00:00:00 2001
From: Guilherme Amadio <amadio@cern.ch>
Date: Tue, 26 Nov 2024 09:48:09 +0100
Subject: [PATCH] [XrdCrypto] Reorder includes and fix stat vs open race
 condition

Make sure that the information is consistent by first opening and
subsequently using the same file descriptor for all operations.
Since we needed to add an include as well, make sure to include
our own headers first, then system headers.
---
 src/XrdCrypto/XrdCryptosslX509.cc    | 43 ++++++++++++++++++++--------
 src/XrdCrypto/XrdCryptosslX509Crl.cc | 28 +++++++++++-------
 2 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/src/XrdCrypto/XrdCryptosslX509.cc b/src/XrdCrypto/XrdCryptosslX509.cc
index 01fbdb25389..428302d70af 100644
--- a/src/XrdCrypto/XrdCryptosslX509.cc
+++ b/src/XrdCrypto/XrdCryptosslX509.cc
@@ -31,12 +31,6 @@
 /* OpenSSL implementation of XrdCryptoX509                                    */
 /*                                                                            */
 /* ************************************************************************** */
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <cerrno>
-#include <memory>
-
 #include "XrdCrypto/XrdCryptosslRSA.hh"
 #include "XrdCrypto/XrdCryptosslX509.hh"
 #include "XrdCrypto/XrdCryptosslAux.hh"
@@ -44,6 +38,14 @@
 
 #include <openssl/pem.h>
 
+#include <cerrno>
+#include <memory>
+
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
 #define BIO_PRINT(b,c) \
    BUF_MEM *bptr; \
    BIO_get_mem_ptr(b, &bptr); \
@@ -90,19 +92,28 @@ XrdCryptosslX509::XrdCryptosslX509(const char *cf, const char *kf)
    }
    // Make sure file exists;
    struct stat st;
-   if (stat(cf, &st) != 0) {
+   int fd = open(cf, O_RDONLY);
+
+   if (fd == -1) {
       if (errno == ENOENT) {
          DEBUG("file "<<cf<<" does not exist - do nothing");
       } else {
-         DEBUG("cannot stat file "<<cf<<" (errno: "<<errno<<")");
+         DEBUG("cannot open file "<<cf<<" (errno: "<<errno<<")");
       }
       return;
    }
+
+   if (fstat(fd, &st) != 0) {
+      DEBUG("cannot stat file "<<cf<<" (errno: "<<errno<<")");
+      close(fd);
+      return;
+   }
    //
    // Open file in read mode
-   FILE *fc = fopen(cf, "r");
+   FILE *fc = fdopen(fd, "r");
    if (!fc) {
-      DEBUG("cannot open file "<<cf<<" (errno: "<<errno<<")");
+      DEBUG("cannot fdopen file "<<cf<<" (errno: "<<errno<<")");
+      close(fd);
       return;
    }
    //
@@ -129,8 +140,14 @@ XrdCryptosslX509::XrdCryptosslX509(const char *cf, const char *kf)
    EVP_PKEY *evpp = 0;
    // Read the private key file, if specified
    if (kf) {
-      if (stat(kf, &st) == -1) {
+      int fd = open(kf, O_RDONLY);
+      if (fd == -1) {
+         DEBUG("cannot open file "<<kf<<" (errno: "<<errno<<")");
+         return;
+      }
+      if (fstat(fd, &st) == -1) {
          DEBUG("cannot stat private key file "<<kf<<" (errno:"<<errno<<")");
+         close(fd);
          return;
       }
       if (!S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) ||
@@ -138,12 +155,14 @@ XrdCryptosslX509::XrdCryptosslX509(const char *cf, const char *kf)
             (st.st_mode & (S_IWGRP)) != 0) {
          DEBUG("private key file "<<kf<<" has wrong permissions "<<
                (st.st_mode & 0777) << " (should be at most 0640)");
+         close(fd);
          return;
       }
       // Open file in read mode
-      FILE *fk = fopen(kf, "r");
+      FILE *fk = fdopen(fd, "r");
       if (!fk) {
          DEBUG("cannot open file "<<kf<<" (errno: "<<errno<<")");
+         close(fd);
          return;
       }
       // This call fills the full key, i.e. also the public part (not really documented, though)
diff --git a/src/XrdCrypto/XrdCryptosslX509Crl.cc b/src/XrdCrypto/XrdCryptosslX509Crl.cc
index 6ece76af59d..6c85b842b72 100644
--- a/src/XrdCrypto/XrdCryptosslX509Crl.cc
+++ b/src/XrdCrypto/XrdCryptosslX509Crl.cc
@@ -31,12 +31,6 @@
 /* OpenSSL implementation of XrdCryptoX509Crl                                 */
 /*                                                                            */
 /* ************************************************************************** */
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <unistd.h>
-#include <cerrno>
-#include <ctime>
-
 #include "XrdCrypto/XrdCryptosslRSA.hh"
 #include "XrdCrypto/XrdCryptosslX509Crl.hh"
 #include "XrdCrypto/XrdCryptosslAux.hh"
@@ -45,6 +39,14 @@
 #include <openssl/bn.h>
 #include <openssl/pem.h>
 
+#include <cerrno>
+#include <ctime>
+
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
 #if OPENSSL_VERSION_NUMBER < 0x10100000L
 #define X509_REVOKED_get0_revocationDate(x) (x)->revocationDate
 #define X509_REVOKED_get0_serialNumber(x) (x)->serialNumber
@@ -164,21 +166,25 @@ int XrdCryptosslX509Crl::Init(const char *cf)
       DEBUG("file name undefined");
       return -1;
    }
+
    // Make sure file exists;
-   struct stat st;
-   if (stat(cf, &st) != 0) {
+   int fd = open(cf, O_RDONLY);
+
+   if (fd == -1) {
       if (errno == ENOENT) {
          DEBUG("file "<<cf<<" does not exist - do nothing");
       } else {
-         DEBUG("cannot stat file "<<cf<<" (errno: "<<errno<<")");
+         DEBUG("cannot open file "<<cf<<" (errno: "<<errno<<")");
       }
       return -1;
    }
-   //
+
    // Open file in read mode
-   FILE *fc = fopen(cf, "r");
+   FILE *fc = fdopen(fd, "r");
+
    if (!fc) {
       DEBUG("cannot open file "<<cf<<" (errno: "<<errno<<")");
+      close(fd);
       return -1;
    }