Merge pull request #26156 from vespa-engine/vekterli/allow-handshake-even-with-empty-capabilities

Allow handshake to go through as long as at least one policy matches

4 files changed, 55 insertions, 22 deletions

diff --git a/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp b/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp
index 0178443643e..068345b7254 100644
--- a/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp
+++ b/vespalib/src/tests/net/tls/openssl_impl/openssl_impl_test.cpp
@@ -735,6 +735,28 @@ TEST_F("Authz policy-derived peer capabilities are propagated to CryptoCodec", C
                                                  Capability::content_status_pages()}));
 }
 
+TEST_F("Handshake is allowed if at least one policy matches, even if resulting capability set is empty", CertFixture) {
+    auto server_ck = f.create_ca_issued_peer_cert({}, {{"DNS:hello.world.example.com"}});
+    auto authorized = authorized_peers({policy_with({required_san_dns("stale.memes.example.com")},
+                                                    CapabilitySet::make_empty()),
+                                        policy_with({required_san_dns("fresh.memes.example.com")},
+                                                    CapabilitySet::make_with_all_capabilities())});
+    f.reset_server_with_cert_opts(server_ck, std::move(authorized));
+    auto client_ck = f.create_ca_issued_peer_cert({}, {{"DNS:stale.memes.example.com"}});
+    f.reset_client_with_cert_opts(client_ck, AuthorizedPeers::allow_all_authenticated());
+
+    ASSERT_TRUE(f.handshake());
+
+    // Note: "inversion" of client <-> server is because the capabilities are that of the _peer_.
+    auto client_caps = f.server->granted_capabilities();
+    auto server_caps = f.client->granted_capabilities();
+    // Server (from client's PoV) implicitly has all capabilities since client doesn't specify any policies
+    EXPECT_EQUAL(server_caps, CapabilitySet::make_with_all_capabilities());
+    // Client (from server's PoV) only has capabilities for the rule matching its DNS SAN entry.
+    // In this case, it is the empty set.
+    EXPECT_EQUAL(client_caps, CapabilitySet::make_empty());
+}
+
 void reset_peers_with_server_authz_mode(CertFixture& f, AuthorizationMode authz_mode) {
     auto ck = f.create_ca_issued_peer_cert({"hello.world.example.com"}, {});
 
diff --git a/vespalib/src/vespa/vespalib/net/tls/policy_checking_certificate_verifier.cpp b/vespalib/src/vespa/vespalib/net/tls/policy_checking_certificate_verifier.cpp
index e280434c59f..a3f9b3f52c9 100644
--- a/vespalib/src/vespa/vespalib/net/tls/policy_checking_certificate_verifier.cpp
+++ b/vespalib/src/vespa/vespalib/net/tls/policy_checking_certificate_verifier.cpp
@@ -74,13 +74,15 @@ VerificationResult PolicyConfiguredCertificateVerifier::verify(const PeerCredent
         return VerificationResult::make_authorized_with_all_capabilities();
     }
     CapabilitySet caps;
+    bool matched_any_policy = false;
     for (const auto& policy : _authorized_peers.peer_policies()) {
         if (matches_all_policy_requirements(peer_creds, policy)) {
             caps.add_all(policy.granted_capabilities());
+            matched_any_policy = true;
         }
     }
-    if (!caps.empty()) {
-        return VerificationResult::make_authorized_with_capabilities(std::move(caps));
+    if (matched_any_policy) {
+        return VerificationResult::make_authorized_with_capabilities(caps);
     } else {
         return VerificationResult::make_not_authorized();
     }
diff --git a/vespalib/src/vespa/vespalib/net/tls/verification_result.cpp b/vespalib/src/vespa/vespalib/net/tls/verification_result.cpp
index f1e50d3115e..37b95c3c07a 100644
--- a/vespalib/src/vespa/vespalib/net/tls/verification_result.cpp
+++ b/vespalib/src/vespa/vespalib/net/tls/verification_result.cpp
@@ -6,14 +6,18 @@
 
 namespace vespalib::net::tls {
 
-VerificationResult::VerificationResult() = default;
+VerificationResult::VerificationResult() noexcept
+    : _granted_capabilities(),
+      _authorized(false)
+{}
 
-VerificationResult::VerificationResult(CapabilitySet granted_capabilities)
-    : _granted_capabilities(std::move(granted_capabilities))
+VerificationResult::VerificationResult(bool authorized, CapabilitySet granted_capabilities) noexcept
+    : _granted_capabilities(granted_capabilities),
+      _authorized(authorized)
 {}
 
-VerificationResult::VerificationResult(const VerificationResult&) = default;
-VerificationResult& VerificationResult::operator=(const VerificationResult&) = default;
+VerificationResult::VerificationResult(const VerificationResult&) noexcept = default;
+VerificationResult& VerificationResult::operator=(const VerificationResult&) noexcept = default;
 VerificationResult::VerificationResult(VerificationResult&&) noexcept = default;
 VerificationResult& VerificationResult::operator=(VerificationResult&&) noexcept = default;
 VerificationResult::~VerificationResult() = default;
@@ -29,18 +33,18 @@ void VerificationResult::print(asciistream& os) const {
 }
 
 VerificationResult
-VerificationResult::make_authorized_with_capabilities(CapabilitySet granted_capabilities) {
-    return VerificationResult(std::move(granted_capabilities));
+VerificationResult::make_authorized_with_capabilities(CapabilitySet granted_capabilities) noexcept {
+    return {true, granted_capabilities};
 }
 
 VerificationResult
-VerificationResult::make_authorized_with_all_capabilities() {
-    return VerificationResult(CapabilitySet::make_with_all_capabilities());
+VerificationResult::make_authorized_with_all_capabilities() noexcept {
+    return {true, CapabilitySet::make_with_all_capabilities()};
 }
 
 VerificationResult
-VerificationResult::make_not_authorized() {
-    return {};
+VerificationResult::make_not_authorized() noexcept {
+    return {false, CapabilitySet::make_empty()};
 }
 
 asciistream& operator<<(asciistream& os, const VerificationResult& res) {
diff --git a/vespalib/src/vespa/vespalib/net/tls/verification_result.h b/vespalib/src/vespa/vespalib/net/tls/verification_result.h
index 92b32ad92f7..896908f7c13 100644
--- a/vespalib/src/vespa/vespalib/net/tls/verification_result.h
+++ b/vespalib/src/vespa/vespalib/net/tls/verification_result.h
@@ -16,22 +16,27 @@ namespace vespalib::net::tls {
  * This result contains the union set of all capabilities granted by the matching
  * authorization rules. If no rules matched, the set will be empty. The capability
  * set will also be empty for a default-constructed instance.
+ *
+ * It is possible for a VerificationResult to be successful but with an empty
+ * capability set. If capabilities are enforced, this will effectively only
+ * allow mTLS handshakes to go through, allowing rudimentary health checking.
  */
 class VerificationResult {
     CapabilitySet _granted_capabilities;
+    bool          _authorized;
 
-    explicit VerificationResult(CapabilitySet granted_capabilities);
+    VerificationResult(bool authorized, CapabilitySet granted_capabilities) noexcept;
 public:
-    VerificationResult();
-    VerificationResult(const VerificationResult&);
-    VerificationResult& operator=(const VerificationResult&);
+    VerificationResult() noexcept; // Unauthorized by default
+    VerificationResult(const VerificationResult&) noexcept;
+    VerificationResult& operator=(const VerificationResult&) noexcept;
     VerificationResult(VerificationResult&&) noexcept;
     VerificationResult& operator=(VerificationResult&&) noexcept;
     ~VerificationResult();
 
-    // Returns true iff at least one capability been granted.
+    // Returns true iff the peer matched at least one policy or authorization is not enforced.
     [[nodiscard]] bool success() const noexcept {
-        return !_granted_capabilities.empty();
+        return _authorized;
     }
 
     [[nodiscard]] const CapabilitySet& granted_capabilities() const noexcept {
@@ -40,9 +45,9 @@ public:
 
     void print(asciistream& os) const;
 
-    static VerificationResult make_authorized_with_capabilities(CapabilitySet granted_capabilities);
-    static VerificationResult make_authorized_with_all_capabilities();
-    static VerificationResult make_not_authorized();
+    static VerificationResult make_authorized_with_capabilities(CapabilitySet granted_capabilities) noexcept;
+    static VerificationResult make_authorized_with_all_capabilities() noexcept;
+    static VerificationResult make_not_authorized() noexcept;
 };
 
 asciistream& operator<<(asciistream&, const VerificationResult&);