Skip to content

Commit 0031b78

Browse files
abakiaydinmeta-codesync[bot]
abakiaydin
authored and
meta-codesync[bot]
committed
Fix CertMatch usage
Summary: CertManagerBase only provides scoped types, there is no shared interface between client and server cert managers. CertManagerBase does not define virtual constructor, potential issue if a pointer to CertManagerBase passed around for a child object. We can make the types needed available in fizz namespace. Reviewed By: frqiu Differential Revision: D88317627 fbshipit-source-id: e71b0bbf8e445e91513e6663717277a4271bff1a
1 parent ae96d6d commit 0031b78

9 files changed

Lines changed: 47 additions & 53 deletions

File tree

‎third-party/fizz/src/fizz/client/CertManager.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ using namespace folly;
1313
namespace fizz {
1414
namespace client {
1515

16-
CertManager::CertMatch CertManager::getCert(
16+
CertMatch CertManager::getCert(
1717
const folly::Optional<std::string>& /* sni */,
1818
const std::vector<SignatureScheme>& supportedSigSchemes,
1919
const std::vector<SignatureScheme>& peerSigSchemes,

‎third-party/fizz/src/fizz/client/CertManager.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,12 @@
1010

1111
#include <map>
1212

13-
#include <fizz/protocol/CertManagerBase.h>
13+
#include <fizz/protocol/CertMatch.h>
1414

1515
namespace fizz {
1616
namespace client {
1717

18-
class CertManager : public CertManagerBase {
18+
class CertManager {
1919
public:
2020
virtual ~CertManager() = default;
2121
/**
@@ -45,7 +45,7 @@ class CertManager : public CertManagerBase {
4545
protected:
4646
void addCert(std::shared_ptr<SelfCert> cert, bool overrideExistingEntry);
4747

48-
SigSchemeMap certs_;
48+
std::map<SignatureScheme, std::shared_ptr<SelfCert>> certs_;
4949
};
5050
} // namespace client
5151
} // namespace fizz

‎third-party/fizz/src/fizz/cmake/FizzSources.cmake‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ set(
201201
extensions/tokenbinding/Validator.h
202202
protocol/Actions.h
203203
protocol/AsyncFizzBase.h
204-
protocol/CertManagerBase.h
204+
protocol/CertMatch.h
205205
protocol/Certificate.h
206206
protocol/CertificateVerifier.h
207207
protocol/DefaultCertificateVerifier.h

‎third-party/fizz/src/fizz/extensions/delegatedcred/DelegatedCredentialCertManager.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ using namespace fizz::server;
1212
namespace fizz {
1313
namespace extensions {
1414

15-
CertManager::CertMatch DelegatedCredentialCertManager::getCert(
15+
CertMatch DelegatedCredentialCertManager::getCert(
1616
const folly::Optional<std::string>& sni,
1717
const std::vector<SignatureScheme>& supportedSigSchemes,
1818
const std::vector<SignatureScheme>& peerSigSchemes,

‎third-party/fizz/src/fizz/extensions/delegatedcred/DelegatedCredentialClientCertManager.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@
1111
namespace fizz {
1212
namespace extensions {
1313

14-
fizz::client::CertManager::CertMatch
15-
DelegatedCredentialClientCertManager::getCert(
14+
CertMatch DelegatedCredentialClientCertManager::getCert(
1615
const folly::Optional<std::string>& sni,
1716
const std::vector<SignatureScheme>& supportedSigSchemes,
1817
const std::vector<SignatureScheme>& peerSigSchemes,

third-party/fizz/src/fizz/protocol/CertManagerBase.h renamed to third-party/fizz/src/fizz/protocol/CertMatch.h

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,11 @@
1313
#include <fizz/protocol/Certificate.h>
1414

1515
namespace fizz {
16-
class CertManagerBase {
17-
public:
18-
enum class MatchType { Direct, Default };
19-
struct CertMatchStruct {
20-
std::shared_ptr<SelfCert> cert;
21-
SignatureScheme scheme;
22-
MatchType type;
23-
};
24-
using CertMatch = folly::Optional<CertMatchStruct>;
25-
26-
using SigSchemeMap = std::map<SignatureScheme, std::shared_ptr<SelfCert>>;
16+
enum class MatchType { Direct, Default };
17+
struct CertMatchStruct {
18+
std::shared_ptr<SelfCert> cert;
19+
SignatureScheme scheme;
20+
MatchType type;
2721
};
22+
using CertMatch = folly::Optional<CertMatchStruct>;
2823
} // namespace fizz

‎third-party/fizz/src/fizz/server/CertManager.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ namespace fizz {
1616
namespace server {
1717

1818
// Find a matching cert given a key.
19-
CertManager::CertMatch CertManager::findCert(
19+
CertMatch CertManager::findCert(
2020
const std::string& key,
2121
const std::vector<SignatureScheme>& supportedSigSchemes,
2222
const std::vector<SignatureScheme>& peerSigSchemes) const {
@@ -37,7 +37,7 @@ CertManager::CertMatch CertManager::findCert(
3737
return none;
3838
}
3939

40-
CertManager::CertMatch CertManager::getCert(
40+
CertMatch CertManager::getCert(
4141
const Optional<std::string>& sni,
4242
const std::vector<SignatureScheme>& supportedSigSchemes,
4343
const std::vector<SignatureScheme>& peerSigSchemes,

‎third-party/fizz/src/fizz/server/CertManager.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
#include <map>
1212
#include <unordered_map>
1313

14-
#include <fizz/protocol/CertManagerBase.h>
14+
#include <fizz/protocol/CertMatch.h>
1515

1616
namespace fizz {
1717
namespace server {
1818

19-
class CertManager : public CertManagerBase {
19+
class CertManager {
2020
public:
2121
virtual ~CertManager() = default;
2222
/**
@@ -43,6 +43,7 @@ class CertManager : public CertManagerBase {
4343

4444
bool hasCerts() const;
4545

46+
using SigSchemeMap = std::map<SignatureScheme, std::shared_ptr<SelfCert>>;
4647
const std::unordered_map<std::string, SigSchemeMap>&
4748
getCertificatesByIdentity() const;
4849

‎third-party/fizz/src/fizz/server/test/ServerProtocolTest.cpp‎

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,11 @@ class ServerProtocolTest : public ProtocolTest<ServerTypes, Actions> {
5656
context_->setClock(clock_);
5757

5858
ON_CALL(*certManager_, getCert(_, _, _, _))
59-
.WillByDefault(Return(
60-
CertManager::CertMatch(
61-
CertManager::CertMatchStruct{
62-
cert_,
63-
SignatureScheme::ecdsa_secp256r1_sha256,
64-
CertManager::MatchType::Direct})));
59+
.WillByDefault(Return(CertMatch(
60+
CertMatchStruct{
61+
cert_,
62+
SignatureScheme::ecdsa_secp256r1_sha256,
63+
MatchType::Direct})));
6564
ON_CALL(*certManager_, getCert(_)).WillByDefault(Return(cert_));
6665

6766
ON_CALL(*clock_, getCurrentTime())
@@ -745,11 +744,11 @@ TEST_F(ServerProtocolTest, TestClientHelloFullHandshakeFlow) {
745744
serverNameList->server_name_list.front()
746745
.hostname->to<std::string>(),
747746
"www.hostname.com");
748-
return CertManager::CertMatch(
749-
CertManager::CertMatchStruct{
747+
return CertMatch(
748+
CertMatchStruct{
750749
cert_,
751750
SignatureScheme::ecdsa_secp256r1_sha256,
752-
CertManager::MatchType::Direct});
751+
MatchType::Direct});
753752
}));
754753
EXPECT_CALL(*cert_, _getCertMessage(_));
755754
EXPECT_CALL(*mockHandshakeContext_, appendToTranscript(_))
@@ -1027,11 +1026,11 @@ TEST_F(ServerProtocolTest, TestClientHelloCompressedCertFlow) {
10271026
serverNameList->server_name_list.front()
10281027
.hostname->to<std::string>(),
10291028
"www.hostname.com");
1030-
return CertManager::CertMatch(
1031-
CertManager::CertMatchStruct{
1029+
return CertMatch(
1030+
CertMatchStruct{
10321031
cert_,
10331032
SignatureScheme::ecdsa_secp256r1_sha256,
1034-
CertManager::MatchType::Direct});
1033+
MatchType::Direct});
10351034
}));
10361035
context_->setSupportedCompressionAlgorithms(
10371036
{CertificateCompressionAlgorithm::zlib});
@@ -1369,11 +1368,11 @@ TEST_F(ServerProtocolTest, TestECHDecryptionSuccess) {
13691368
serverNameList->server_name_list.front()
13701369
.hostname->to<std::string>(),
13711370
"www.hostname.com");
1372-
return CertManager::CertMatch(
1373-
CertManager::CertMatchStruct{
1371+
return CertMatch(
1372+
CertMatchStruct{
13741373
cert_,
13751374
SignatureScheme::ecdsa_secp256r1_sha256,
1376-
CertManager::MatchType::Direct});
1375+
MatchType::Direct});
13771376
}));
13781377
EXPECT_CALL(*cert_, _getCertMessage(_));
13791378
EXPECT_CALL(*mockHandshakeContext_, appendToTranscript(_))
@@ -1708,11 +1707,11 @@ TEST_F(ServerProtocolTest, TestECHDecryptionFailure) {
17081707
serverNameList->server_name_list.front()
17091708
.hostname->to<std::string>(),
17101709
"www.fakehostname.com");
1711-
return CertManager::CertMatch(
1712-
CertManager::CertMatchStruct{
1710+
return CertMatch(
1711+
CertMatchStruct{
17131712
cert_,
17141713
SignatureScheme::ecdsa_secp256r1_sha256,
1715-
CertManager::MatchType::Direct});
1714+
MatchType::Direct});
17161715
}));
17171716
EXPECT_CALL(*cert_, _getCertMessage(_));
17181717
EXPECT_CALL(*mockHandshakeContext_, appendToTranscript(_))
@@ -1994,11 +1993,11 @@ TEST_F(ServerProtocolTest, TestClientHelloCertRequestFlow) {
19941993
serverNameList->server_name_list.front()
19951994
.hostname->to<std::string>(),
19961995
"www.hostname.com");
1997-
return CertManager::CertMatch(
1998-
CertManager::CertMatchStruct{
1996+
return CertMatch(
1997+
CertMatchStruct{
19991998
cert_,
20001999
SignatureScheme::ecdsa_secp256r1_sha256,
2001-
CertManager::MatchType::Direct});
2000+
MatchType::Direct});
20022001
}));
20032002
EXPECT_CALL(*cert_, _getCertMessage(_));
20042003
EXPECT_CALL(*mockHandshakeContext_, appendToTranscript(_))
@@ -2801,11 +2800,11 @@ TEST_F(ServerProtocolTest, TestRetryClientHelloFullHandshakeFlow) {
28012800
serverNameList->server_name_list.front()
28022801
.hostname->to<std::string>(),
28032802
"www.hostname.com");
2804-
return CertManager::CertMatch(
2805-
CertManager::CertMatchStruct{
2803+
return CertMatch(
2804+
CertMatchStruct{
28062805
cert_,
28072806
SignatureScheme::ecdsa_secp256r1_sha256,
2808-
CertManager::MatchType::Direct});
2807+
MatchType::Direct});
28092808
}));
28102809
EXPECT_CALL(*cert_, _getCertMessage(_));
28112810
EXPECT_CALL(*mockHandshakeContext_, appendToTranscript(_))
@@ -3340,11 +3339,11 @@ TEST_F(ServerProtocolTest, TestRetryClientHelloECHFlow) {
33403339
serverNameList->server_name_list.front()
33413340
.hostname->to<std::string>(),
33423341
"www.hostname.com");
3343-
return CertManager::CertMatch(
3344-
CertManager::CertMatchStruct{
3342+
return CertMatch(
3343+
CertMatchStruct{
33453344
cert_,
33463345
SignatureScheme::ecdsa_secp256r1_sha256,
3347-
CertManager::MatchType::Direct});
3346+
MatchType::Direct});
33483347
}));
33493348
EXPECT_CALL(*cert_, _getCertMessage(_));
33503349
EXPECT_CALL(*mockHandshakeContext_, appendToTranscript(_))
@@ -3634,11 +3633,11 @@ TEST_F(ServerProtocolTest, TestRetryClientHelloECHRejectedFlow) {
36343633
serverNameList->server_name_list.front()
36353634
.hostname->to<std::string>(),
36363635
"www.fakehostname.com");
3637-
return CertManager::CertMatch(
3638-
CertManager::CertMatchStruct{
3636+
return CertMatch(
3637+
CertMatchStruct{
36393638
cert_,
36403639
SignatureScheme::ecdsa_secp256r1_sha256,
3641-
CertManager::MatchType::Direct});
3640+
MatchType::Direct});
36423641
}));
36433642
EXPECT_CALL(*cert_, _getCertMessage(_));
36443643
EXPECT_CALL(*mockHandshakeContext_, appendToTranscript(_))

0 commit comments

Comments
 (0)