mbedtls_x509_crt_parse_path() fails to load the ca-bundle on Fedora

[jp-bennett]

Type: Bug
Possibly related to #2409. I've tracked down an issue in OBS on Fedora where RTMPS streams fail to authenticate. See: https://obsproject.com/forum/threads/unable-to-live-stream-to-facebook-live-over-rtmps.102849/

The root issue is that on Fedora, the ca-bundle in /etc/ssl/certs/ is a symlink, and mbedtls_x509_crt_parse_path() explicitly skips loading anything that isn't a regular file.

https://github.com/ARMmbed/mbedtls/blob/ccdeb47cdf0f4f070c7d48d46e5e098f025c005b/library/x509_crt.c#L1616

Line 1616 seems to be the issue. I don't know of any security problems with allowing symlinked bundles, so perhaps it should read:
if( !( S_ISREG( sb.st_mode ) || S_IFLNK( sb.st_mode ) ) )

If that seems like a reasonable solution, I can submit a patch, or feel free to just make the change, as it's trivial.

[yanesca]

Hi @jp-bennett

Yes, it looks like a reasonable solution to me. Thank you for offering your help, please submit a PR to address this issue, we will be happy to take it.

[jp-bennett]

@yanesca Submitted the PR. I may have noticed a related issue, also in mbedtls_x509_crt_parse_path()
There doesn't seem to be a check that valid certificates were loaded. If that function doesn't attempt to parse any files, it returns 0. So, call parse_path on an empty folder, and you get a success result. That doesn't seem to be intended. The Windows implementation, for instance, returns an IO error if the given path is empty. It seems like there should be a final check just before returning, that at least one valid certificate was loaded onto the chain.

[yanesca]

Thank you very much for your contribution!

Indeed, that behaviour is not intended: the documentation of mbedtls_x509_crt_parse_path() function promises to load "one or more certificate files" and should return an error if it can't find any files at the given path.

Thank you for catching this! Could you please open a new issue for this matter?

[jp-bennett]

@yanesca @Patater I just discovered that I was wrong. The changes I suggested can be reverted. stat() will never return S_IFLNK as the file type, as stat() explicitly follows symlinks. The bug was in OBS. A positive return value was considered a total failure.

Sorry for the useless noise.

[yanesca]

@jp-bennett Thank you for reporting your findings!

I am reopening this issue as #3008 and its backports need reverting.