Commit 4892f1bb authored by Sotirios Delimanolis's avatar Sotirios Delimanolis Committed by Facebook GitHub Bot

Fix OpenSSLUtils::getCommonName to correctly trim trailing null characters

Summary:
Previously `OpenSSLUtils::getCommonName` was allocating a 64 byte long string and copying the common name from the `X509` into it. We unexpectedly returned that string potentially containing trailing `\0`s.

This diff rewrites the function to allocate a big enough buffer, write to it, then return an appropriately trimmed new string from it.

Reviewed By: yfeldblum

Differential Revision: D22910431

fbshipit-source-id: 77b4fc3ccde4f7ddb0f62b884ecd7ff0c868c1ca
parent 85a8672b
......@@ -312,11 +312,15 @@ std::string OpenSSLUtils::getCommonName(X509* x509) {
return "";
}
X509_NAME* subject = X509_get_subject_name(x509);
std::string cn;
cn.resize(ub_common_name);
X509_NAME_get_text_by_NID(
subject, NID_commonName, const_cast<char*>(cn.data()), ub_common_name);
return cn;
char buf[ub_common_name + 1];
int length =
X509_NAME_get_text_by_NID(subject, NID_commonName, buf, sizeof(buf));
if (length == -1) {
// no CN
return "";
}
// length tells us where the name ends
return std::string(buf, length);
}
} // namespace ssl
......
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#include <folly/io/async/ssl/OpenSSLUtils.h>
#include <folly/String.h>
#include <folly/portability/GTest.h>
#include <folly/portability/OpenSSL.h>
#include <folly/ssl/OpenSSLPtrTypes.h>
using namespace ::testing;
using namespace folly::ssl;
namespace folly {
const std::string kSampleCommonName = "Folly Library";
// a certificate with a CN that uses 64 characters, the max length
const std::string kSampleCommonNameMaxLength =
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
// create and return an X509 object with only the subject's CN set, so tests can
// extract and compare it
X509UniquePtr createMinimalX509(const std::string& commonName) {
X509* x509;
x509 = X509_new();
X509_NAME* name;
name = X509_get_subject_name(x509);
X509_NAME_add_entry_by_txt(
name,
SN_commonName,
MBSTRING_ASC,
reinterpret_cast<const unsigned char*>(commonName.data()),
-1,
-1,
0);
return X509UniquePtr(x509);
}
// Tests that the common name is extracted from the x509 certificate with the
// correct length
TEST(OpenSSLUtilsTest, getCommonName) {
X509UniquePtr x509 = createMinimalX509(kSampleCommonName);
EXPECT_EQ(OpenSSLUtils::getCommonName(x509.get()), kSampleCommonName);
}
// Tests that the common name is extracted from the x509 certificate correctly
// when its length is the maximum, defined as ub_common_name in asn1.h (RFC2459)
TEST(OpenSSLUtilsTest, getCommonNameMaxLength) {
X509UniquePtr x509 = createMinimalX509(kSampleCommonNameMaxLength);
// read common name from certificate
EXPECT_EQ(
OpenSSLUtils::getCommonName(x509.get()), kSampleCommonNameMaxLength);
}
// Tests that getCommonName returns an empty string for a null X509 argument
TEST(OpenSSLUtilsTest, getCommonNameNullX509) {
EXPECT_EQ(OpenSSLUtils::getCommonName(nullptr), "");
}
// Tests that getCommonName returns an empty string because the given
// certificate has no CN
TEST(OpenSSLUtilsTest, getCommonNameEmpty) {
X509UniquePtr x509 = createMinimalX509("");
EXPECT_EQ(OpenSSLUtils::getCommonName(x509.get()), "");
}
} // namespace folly
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment