Skip to content

Commit

Permalink
[XrdCrypto] Reorder includes and fix stat vs open race condition
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
amadio committed Nov 26, 2024
1 parent 63c4cb3 commit 31a41eb
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 23 deletions.
43 changes: 31 additions & 12 deletions src/XrdCrypto/XrdCryptosslX509.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,21 @@
/* 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"
#include "XrdCrypto/XrdCryptosslTrace.hh"

#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); \
Expand Down Expand Up @@ -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;
}
//
Expand All @@ -129,21 +140,29 @@ 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) ||
(st.st_mode & (S_IROTH | S_IWOTH)) != 0 ||
(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)
Expand Down
28 changes: 17 additions & 11 deletions src/XrdCrypto/XrdCryptosslX509Crl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 31a41eb

Please sign in to comment.