From 32ffb2b8bc9f9229a83681ae4f1a21f15d0bd0eb Mon Sep 17 00:00:00 2001
From: Eduardo Silva <eduardo@treaure-data.com>
Date: Thu, 25 Apr 2019 10:43:26 -0600
Subject: [PATCH] x509_crt: handle properly broken links when looking for
 certificates

On non-windows environments, when loading certificates from a given
path through mbedtls_x509_crt_parse_path() function, if a symbolic
link is found and is broken (meaning the target file don't exists),
the function is returning MBEDTLS_ERR_X509_FILE_IO_ERROR which is
not honoring the default behavior of just skip the bad certificate file
and increase the counter of wrong files.

The problem have been raised many times in our open source project
called Fluent Bit which depends on MbedTLS:

https://github.com/fluent/fluent-bit/issues/843#issuecomment-486388209

The expected behavior is that if a simple certificate cannot be processed,
it should just be skipped.

This patch implements a workaround with lstat(2) and stat(2) to determinate
first if the entry found in the directory is a symbolic link or not, if is
a simbolic link, do a proper stat(2) for the target file, otherwise process
normally. Upon find a broken symbolic link it will increase the counter of
not processed certificates.

Signed-off-by: Eduardo Silva <eduardo@treaure-data.com>
---
 library/x509_crt.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/library/x509_crt.c b/library/x509_crt.c
index 911644b7da..97a908c134 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -77,6 +77,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <dirent.h>
+#include <errno.h>
 #endif /* !_WIN32 || EFIX64 || EFI32 */
 #endif
 
@@ -1638,10 +1639,39 @@ cleanup:
             ret = MBEDTLS_ERR_X509_BUFFER_TOO_SMALL;
             goto cleanup;
         }
-        else if( stat( entry_name, &sb ) == -1 )
+        else
         {
-            ret = MBEDTLS_ERR_X509_FILE_IO_ERROR;
-            goto cleanup;
+            /* determinate if the file entry could be a link, using lstat(2)
+             * is safer than just stat(2), otherwise a broken link will
+             * give us a false positive. */
+            if( lstat( entry_name, &sb ) == -1 )
+            {
+                ret = MBEDTLS_ERR_X509_FILE_IO_ERROR;
+                goto cleanup;
+            }
+
+            /* if the file is a symbolic link, we need to validate the real
+             * information using stat(2). */
+            if( S_ISLNK( sb.st_mode ) )
+            {
+                /* if stat(2) fails it could be a broken link or a generic
+                 * error, if the link is broken, just report it as a
+                 * certificate that could not be processed, otherwise
+                 * just set a MBEDTLS_ERR_X509_FILE_IO_ERROR. */
+                if( stat( entry_name, &sb ) == -1 )
+                {
+                    if( errno == ENOENT )
+                    {
+                        /* Broken link */
+                        ret++;
+                    }
+                    else
+                    {
+                        ret = MBEDTLS_ERR_X509_FILE_IO_ERROR;
+                        goto cleanup;
+                    }
+                }
+            }
         }
 
         if( !S_ISREG( sb.st_mode ) )