Skip current directory and parent directory symbols when reading files in a directory

[olszomal]

SSL_add_dir_cert_subjects_to_stack() function always fails on Windows.
SSL_add_file_cert_subjects_to_stack() function fails on Windows when trying to open a directory as a file :

openssl/ssl/ssl_cert.c

Lines 767 to 768 in 7e55051

	if (BIO_read_filename(in, file) <= 0)
	goto err;

The following code shows this error. This PoC works in Linux but not Windows:

#include <openssl/x509.h>
#include <openssl/ssl.h>
#include <openssl/safestack.h>

int main(int argc, char **argv) {
    int n, i;
    STACK_OF(X509_NAME) *ca_dn=sk_X509_NAME_new_null();
    (void)argc;
    SSL_add_dir_cert_subjects_to_stack(ca_dn, argv[1]);
    if((n=sk_X509_NAME_num(ca_dn))==0) {
        printf("No trusted certificates found\n");
        sk_X509_NAME_pop_free(ca_dn, X509_NAME_free);
        return 1; /* FAILED */
    }
    for(i=0; i<n; i++) {
        char *ca_name=X509_NAME_oneline(sk_X509_NAME_value(ca_dn, i), NULL, 0);
        printf("CA #%d: %s\n", i, ca_name);
        OPENSSL_free(ca_name);
    }
    sk_X509_NAME_pop_free(ca_dn, X509_NAME_free);
    return 0; /* OK */
}

My improved PR fixes this by skipping current, parent and other subdirectories in SSL_add_dir_cert_subjects_to_stack().

[paulidale]

I'm not sure this is the right fix. Opening a directory as a file ought to fail which makes this additional check unnecessary, even if it did open, it shouldn't parse and should still fail.

[mtrojnar]

I'm not sure this is the right fix. Opening a directory as a file ought to fail which makes this additional check unnecessary, even if it did open, it shouldn't parse and should still fail.

@paulidale The proposed fix filters out current and parent subdirectories from the generated list of files within the directory provided as an SSL_add_dir_cert_subjects_to_stack() parameter. This is specifically meant to prevent opening those subdirectories as files with SSL_add_file_cert_subjects_to_stack(), so that it doesn't fail.

TL;DR: SSL_add_dir_cert_subjects_to_stack() currently always fails on Windows. AFAIK it never worked on that platform.

[paulidale]

Thanks for the explanation.

[levitte]

Hmmm... So it seems that on Linux, this call succeeds:

openssl/ssl/ssl_cert.c

Lines 767 to 768 in 7e55051

	if (BIO_read_filename(in, file) <= 0)
	goto err;

I assume that on Windows, it fails, thus causing an error indication to be returned...

On platforms where the directory can be opened as a file, it's expected that PEM_read_bio_X598 will return NULL, and thereby exit the function with success, just as it does with empty files:

openssl/ssl/ssl_cert.c

Lines 771 to 772 in 7e55051

	if (PEM_read_bio_X509(in, &x, NULL, NULL) == NULL)
	break;

The fix here is, unfortunately, incomplete. If the directory with certificates contains a subdirectory, calling SSL_add_dir_cert_subjects_to_stack() will fail as well. My I suggest using stat() instead, to check that the file is a "normal" file? Something like this should suffice (inspired from the inode(7) manual:

           stat(buf, &sb);
           if ((sb.st_mode & S_IFMT) != S_IFREG)
               continue;

Note that this needs to be done after the BIO_snprint() line

[openssl-machine]

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

[paulidale]

Merged, thanks for the fix.

[t8m]

This caused regression of no-posix-io builds. Fix in #20786