Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check validity times in default auth policies #4786

Merged
merged 27 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
1fbeaf9
Insert default nbf and exp values into generated JWTs
eddyashton Dec 21, 2022
360c524
Require valid nbf and exp times in JWT tokens
eddyashton Dec 21, 2022
484a78d
Add test with invalid times
eddyashton Dec 21, 2022
a3cd27c
Validate Not Before and Not After times in cert auth
eddyashton Dec 21, 2022
92eb29a
e2e test of cert auth time validation
eddyashton Dec 21, 2022
70b0a2c
Lint and format
eddyashton Dec 22, 2022
880fdf1
Fixes for frontend_test - set current time
eddyashton Dec 22, 2022
acf4954
CHANGELOG
eddyashton Dec 22, 2022
ac5a84b
Wrong formatter
eddyashton Dec 22, 2022
708a2b6
Merge branch 'main' of github.com:microsoft/CCF into expiry_times_in_…
eddyashton Dec 22, 2022
4ce67aa
Reduce iterations
eddyashton Dec 22, 2022
bf5b11b
Merge branch 'main' of github.com:microsoft/CCF into expiry_times_in_…
eddyashton Jan 3, 2023
0a814fe
Add LRU caching validity periods
eddyashton Jan 4, 2023
a3ba8ce
Revert repetition reduction
eddyashton Jan 4, 2023
50e6d47
Waddit I done
eddyashton Jan 4, 2023
4c5f231
Merge branch 'main' into expiry_times_in_auth_policies
eddyashton Jan 5, 2023
6a69010
PR suggestions
eddyashton Jan 5, 2023
b92cb39
Expand jwt docs
eddyashton Jan 5, 2023
6ed8bc4
Merge branch 'main' of github.com:microsoft/CCF into expiry_times_in_…
eddyashton Jan 5, 2023
71f873c
Try extracting specific fields from JWT payload, rather than full obj…
eddyashton Jan 5, 2023
c6842ea
Format
eddyashton Jan 5, 2023
2051ef1
Merge branch 'main' of github.com:microsoft/CCF into expiry_times_in_…
eddyashton Jan 5, 2023
93a6085
Merge branch 'main' into expiry_times_in_auth_policies
eddyashton Jan 9, 2023
45c63b6
Merge branch 'main' into expiry_times_in_auth_policies
eddyashton Jan 11, 2023
4e8c59f
Merge branch 'main' of github.com:microsoft/CCF into expiry_times_in_…
eddyashton Jan 12, 2023
b031933
Revert "Try extracting specific fields from JWT payload, rather than …
eddyashton Jan 12, 2023
a784078
school for coders who don't write good
eddyashton Jan 12, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).

## Unreleased

### Changed

- The built-in authentication policies for JWTs and certs will now enforce expiry times, based on the current time received from the host. JWTs must contain "nbf" and "exp" claims, and if those are outside the current time then the request will get an authentication error (#4786).
eddyashton marked this conversation as resolved.
Show resolved Hide resolved

## [4.0.0-dev3]

[4.0.0-dev3]: https://github.com/microsoft/CCF/releases/tag/ccf-4.0.0-dev3
Expand Down
5 changes: 5 additions & 0 deletions include/ccf/ds/json.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ class JsonParseError : public std::invalid_argument
"#/{}",
fmt::join(pointer_elements.crbegin(), pointer_elements.crend(), "/"));
}

std::string describe() const
{
return fmt::format("At {}: {}", pointer(), what());
}
};

namespace std
Expand Down
2 changes: 1 addition & 1 deletion src/enclave/enclave_time.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the Apache 2.0 License.
#include "enclave/enclave_time.h"

namespace ccf
namespace ccf::enclavetime
{
std::atomic<long long>* host_time_us = nullptr;
std::chrono::microseconds last_value(0);
Expand Down
17 changes: 10 additions & 7 deletions src/enclave/enclave_time.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,24 @@

namespace ccf
{
extern std::atomic<long long>* host_time_us;
extern std::chrono::microseconds last_value;
namespace enclavetime
{
extern std::atomic<long long>* host_time_us;
extern std::chrono::microseconds last_value;
}

static std::chrono::microseconds get_enclave_time()
{
// Update cached value if possible, but never move backwards
if (host_time_us != nullptr)
if (enclavetime::host_time_us != nullptr)
{
const auto current_time = host_time_us->load();
if (current_time > last_value.count())
const auto current_time = enclavetime::host_time_us->load();
if (current_time > enclavetime::last_value.count())
{
last_value = std::chrono::microseconds(current_time);
enclavetime::last_value = std::chrono::microseconds(current_time);
}
eddyashton marked this conversation as resolved.
Show resolved Hide resolved
}

return last_value;
return enclavetime::last_value;
}
}
8 changes: 4 additions & 4 deletions src/enclave/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -166,20 +166,20 @@ extern "C"
// really are. lfence after these checks to prevent speculative
// execution
if (!ccf::pal::is_outside_enclave(
time_location, sizeof(*ccf::host_time_us)))
time_location, sizeof(*ccf::enclavetime::host_time_us)))
{
LOG_FAIL_FMT("Memory outside enclave: time_location");
return CreateNodeStatus::MemoryNotOutsideEnclave;
}

if (!is_aligned(time_location, 8, sizeof(*ccf::host_time_us)))
if (!is_aligned(time_location, 8, sizeof(*ccf::enclavetime::host_time_us)))
{
LOG_FAIL_FMT("Read source memory not aligned: time_location");
return CreateNodeStatus::UnalignedArguments;
}

ccf::host_time_us =
static_cast<decltype(ccf::host_time_us)>(time_location);
ccf::enclavetime::host_time_us =
static_cast<decltype(ccf::enclavetime::host_time_us)>(time_location);

ccf::pal::speculation_barrier();
}
Expand Down
54 changes: 54 additions & 0 deletions src/endpoints/authentication/cert_auth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,53 @@
#include "ccf/service/tables/members.h"
#include "ccf/service/tables/nodes.h"
#include "ccf/service/tables/users.h"
#include "ds/x509_time_fmt.h"
#include "enclave/enclave_time.h"

namespace ccf
{
static inline bool is_cert_valid_now(
eddyashton marked this conversation as resolved.
Show resolved Hide resolved
const std::vector<uint8_t>& der_cert, std::string& error_reason)
{
auto verifier = crypto::make_unique_verifier(der_cert);

const auto [valid_from_timestring, valid_to_timestring] =
verifier->validity_period();

using namespace std::chrono;

const auto valid_from_unix_time =
duration_cast<seconds>(
ds::time_point_from_string(valid_from_timestring).time_since_epoch())
.count();
const auto valid_to_unix_time =
duration_cast<seconds>(
ds::time_point_from_string(valid_to_timestring).time_since_epoch())
.count();

const auto time_now =
duration_cast<seconds>(ccf::get_enclave_time()).count();

if (time_now < valid_from_unix_time)
{
error_reason = fmt::format(
"Current time {} is before certificate's Not Before validity period {}",
time_now,
valid_from_unix_time);
return false;
}
else if (time_now > valid_to_unix_time)
{
error_reason = fmt::format(
"Current time {} is after certificate's Not After validity period {}",
time_now,
valid_from_unix_time);
return false;
}

return true;
}

std::unique_ptr<AuthnIdentity> UserCertAuthnPolicy::authenticate(
kv::ReadOnlyTx& tx,
const std::shared_ptr<ccf::RpcContext>& ctx,
Expand All @@ -22,6 +66,11 @@ namespace ccf
return nullptr;
}

if (!is_cert_valid_now(caller_cert, error_reason))
{
return nullptr;
}

auto caller_id = crypto::Sha256Hash(caller_cert).hex_str();

auto user_certs = tx.ro<UserCerts>(Tables::USER_CERTS);
Expand All @@ -48,6 +97,11 @@ namespace ccf
return nullptr;
}

if (!is_cert_valid_now(caller_cert, error_reason))
{
return nullptr;
}

auto caller_id = crypto::Sha256Hash(caller_cert).hex_str();

auto member_certs = tx.ro<MemberCerts>(Tables::MEMBER_CERTS);
Expand Down
43 changes: 34 additions & 9 deletions src/endpoints/authentication/jwt_auth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "ccf/rpc_context.h"
#include "ccf/service/tables/jwt.h"
#include "enclave/enclave_time.h"
#include "http/http_jwt.h"

namespace ccf
Expand All @@ -16,32 +17,56 @@ namespace ccf
{
const auto& headers = ctx->get_request_headers();

const auto token = http::JwtVerifier::extract_token(headers, error_reason);
const auto token_opt =
http::JwtVerifier::extract_token(headers, error_reason);

if (token.has_value())
if (token_opt.has_value())
{
auto& token = token_opt.value();
auto keys =
tx.ro<JwtPublicSigningKeys>(ccf::Tables::JWT_PUBLIC_SIGNING_KEYS);
auto key_issuers = tx.ro<JwtPublicSigningKeyIssuer>(
ccf::Tables::JWT_PUBLIC_SIGNING_KEY_ISSUER);
const auto key_id = token.value().header_typed.kid;
const auto key_id = token.header_typed.kid;
const auto token_key = keys->get(key_id);
if (!token_key.has_value())
{
error_reason = "JWT signing key not found";
}
else if (!http::JwtVerifier::validate_token_signature(
token.value(), token_key.value()))
token, token_key.value()))
{
error_reason = "JWT signature is invalid";
}
else
{
auto identity = std::make_unique<JwtAuthnIdentity>();
identity->key_issuer = key_issuers->get(key_id).value();
identity->header = std::move(token->header);
identity->payload = std::move(token->payload);
return identity;
// Check that the Not Before and Expiration Time claims are valid
const size_t time_now =
std::chrono::duration_cast<std::chrono::seconds>(
ccf::get_enclave_time())
.count();
if (time_now < token.payload_typed.nbf)
{
error_reason = fmt::format(
"Current time {} is before token's Not Before claim {}",
eddyashton marked this conversation as resolved.
Show resolved Hide resolved
time_now,
token.payload_typed.nbf);
}
else if (time_now > token.payload_typed.exp)
{
error_reason = fmt::format(
"Current time {} is after token's Expiration Time claim {}",
eddyashton marked this conversation as resolved.
Show resolved Hide resolved
time_now,
token.payload_typed.exp);
}
else
{
auto identity = std::make_unique<JwtAuthnIdentity>();
identity->key_issuer = key_issuers->get(key_id).value();
identity->header = std::move(token.header);
identity->payload = std::move(token.payload);
return identity;
}
}
}

Expand Down
31 changes: 28 additions & 3 deletions src/http/http_jwt.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ namespace http
DECLARE_JSON_TYPE(JwtHeader)
DECLARE_JSON_REQUIRED_FIELDS(JwtHeader, alg, kid)

struct JwtPayload
{
size_t nbf;
size_t exp;
};
DECLARE_JSON_TYPE(JwtPayload)
DECLARE_JSON_REQUIRED_FIELDS(JwtPayload, nbf, exp)

class JwtVerifier
{
public:
Expand All @@ -36,6 +44,7 @@ namespace http
nlohmann::json header;
JwtHeader header_typed;
nlohmann::json payload;
JwtPayload payload_typed;
std::vector<uint8_t> signature;
std::string_view signed_content;
};
Expand Down Expand Up @@ -116,14 +125,30 @@ namespace http
{
header_typed = header.get<JwtHeader>();
}
catch (const nlohmann::json::exception& e)
catch (const JsonParseError& e)
{
error_reason =
fmt::format("JWT header does not follow schema: {}", e.what());
fmt::format("JWT header does not follow schema: {}", e.describe());
return std::nullopt;
}
JwtPayload payload_typed;
try
{
payload_typed = payload.get<JwtPayload>();
}
catch (const JsonParseError& e)
{
error_reason = fmt::format(
"JWT payload is missing required field: {}", e.describe());
return std::nullopt;
}
Token parsed = {
header, header_typed, payload, signature_raw, signed_content};
header,
header_typed,
payload,
payload_typed,
signature_raw,
signed_content};
return parsed;
}

Expand Down
4 changes: 1 addition & 3 deletions src/node/rpc/frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -588,9 +588,7 @@ namespace ccf
catch (const JsonParseError& e)
{
ctx->set_error(
HTTP_STATUS_BAD_REQUEST,
ccf::errors::InvalidInput,
fmt::format("At {}: {}", e.pointer(), e.what()));
HTTP_STATUS_BAD_REQUEST, ccf::errors::InvalidInput, e.describe());
update_metrics(ctx);
return;
}
Expand Down
11 changes: 8 additions & 3 deletions src/node/rpc/test/frontend_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "ccf/serdes.h"
#include "consensus/aft/request.h"
#include "ds/files.h"
#include "enclave/enclave_time.h"
#include "frontend_test_infra.h"
#include "kv/test/null_encryptor.h"
#include "kv/test/stub_consensus.h"
Expand Down Expand Up @@ -467,16 +468,16 @@ nlohmann::json parse_response_body(
}

// callers used throughout
auto user_caller = kp -> self_sign("CN=name", valid_from, valid_to);
auto user_caller = kp->self_sign("CN=name", valid_from, valid_to);
auto user_caller_der = crypto::make_verifier(user_caller) -> cert_der();

auto member_caller_der = crypto::make_verifier(member_cert) -> cert_der();

auto node_caller = kp -> self_sign("CN=node", valid_from, valid_to);
auto node_caller = kp->self_sign("CN=node", valid_from, valid_to);
auto node_caller_der = crypto::make_verifier(node_caller) -> cert_der();

auto kp_other = crypto::make_key_pair();
auto invalid_caller = kp_other -> self_sign("CN=name", valid_from, valid_to);
auto invalid_caller = kp_other->self_sign("CN=name", valid_from, valid_to);
auto invalid_caller_der = crypto::make_verifier(invalid_caller) -> cert_der();

auto anonymous_caller_der = std::vector<uint8_t>();
Expand Down Expand Up @@ -1877,6 +1878,10 @@ TEST_CASE("Manual conflicts")

int main(int argc, char** argv)
{
ccf::enclavetime::last_value =
std::chrono::duration_cast<std::chrono::microseconds>(
std::chrono::system_clock::now().time_since_epoch());

doctest::Context context;
context.applyCommandLine(argc, argv);
int res = context.run();
Expand Down
2 changes: 1 addition & 1 deletion src/node/test/channels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
#include <doctest/doctest.h>

namespace ccf
namespace ccf::enclavetime
{
std::atomic<long long>* host_time_us = nullptr;
std::chrono::microseconds last_value(0);
Expand Down
14 changes: 11 additions & 3 deletions tests/infra/crypto.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,21 @@ def generate_eddsa_keypair() -> Tuple[str, str]:


def generate_cert(
priv_key_pem: str, cn=None, issuer_priv_key_pem=None, issuer_cn=None, ca=False
priv_key_pem: str,
cn=None,
issuer_priv_key_pem=None,
issuer_cn=None,
ca=False,
valid_from=None,
validity_days=10,
) -> str:
cn = cn or "dummy"
if issuer_priv_key_pem is None:
issuer_priv_key_pem = priv_key_pem
if issuer_cn is None:
issuer_cn = cn
if valid_from is None:
valid_from = datetime.datetime.utcnow()
priv = load_pem_private_key(priv_key_pem.encode("ascii"), None, default_backend())
pub = priv.public_key()
issuer_priv = load_pem_private_key(
Expand All @@ -139,8 +147,8 @@ def generate_cert(
.issuer_name(issuer)
.public_key(pub)
.serial_number(x509.random_serial_number())
.not_valid_before(datetime.datetime.utcnow())
.not_valid_after(datetime.datetime.utcnow() + datetime.timedelta(days=10))
.not_valid_before(valid_from)
.not_valid_after(valid_from + datetime.timedelta(days=validity_days))
)
if ca:
builder = builder.add_extension(
Expand Down
Loading