Skip to content

Commit

Permalink
Remove SSL_CTX_set_ed25519_enabled.
Browse files Browse the repository at this point in the history
We never ended up using this, and callers can still configure
SSL_CTX_set_verify_algorithm_prefs to enable Ed25519 on the receiving
side. (On the sending side, this API was never needed because it's a
function of what certificate you configure.) This was just a way to
tweak the default without requiring callers restate the order.

Change-Id: I38d7f91d85430f37fc7e278d77466e78a0cbfa0d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/39848
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
  • Loading branch information
davidben authored and CQ bot account: commit-bot@chromium.org committed Feb 6, 2020
1 parent 6ab75bf commit 1766935
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 65 deletions.
6 changes: 0 additions & 6 deletions include/openssl/ssl.h
Expand Up @@ -2518,12 +2518,6 @@ OPENSSL_EXPORT int SSL_set0_verify_cert_store(SSL *ssl, X509_STORE *store);
// reference to |store| will be taken.
OPENSSL_EXPORT int SSL_set1_verify_cert_store(SSL *ssl, X509_STORE *store);

// SSL_CTX_set_ed25519_enabled configures whether |ctx| advertises support for
// the Ed25519 signature algorithm when using the default preference list. It is
// disabled by default and may be enabled if the certificate verifier supports
// Ed25519.
OPENSSL_EXPORT void SSL_CTX_set_ed25519_enabled(SSL_CTX *ctx, int enabled);

// SSL_CTX_set_verify_algorithm_prefs configures |ctx| to use |prefs| as the
// preference list when verifying signatures from the peer's long-term key. It
// returns one on zero on error. |prefs| should not include the internal-only
Expand Down
3 changes: 0 additions & 3 deletions ssl/internal.h
Expand Up @@ -3309,9 +3309,6 @@ struct ssl_ctx_st {
// protocols from the peer.
bool allow_unknown_alpn_protos : 1;

// ed25519_enabled is whether Ed25519 is advertised in the handshake.
bool ed25519_enabled : 1;

// false_start_allowed_without_alpn is whether False Start (if
// |SSL_MODE_ENABLE_FALSE_START| is enabled) is allowed without ALPN.
bool false_start_allowed_without_alpn : 1;
Expand Down
1 change: 0 additions & 1 deletion ssl/ssl_lib.cc
Expand Up @@ -564,7 +564,6 @@ ssl_ctx_st::ssl_ctx_st(const SSL_METHOD *ssl_method)
channel_id_enabled(false),
grease_enabled(false),
allow_unknown_alpn_protos(false),
ed25519_enabled(false),
false_start_allowed_without_alpn(false),
ignore_tls13_downgrade(false),
handoff(false),
Expand Down
45 changes: 6 additions & 39 deletions ssl/t1_lib.cc
Expand Up @@ -413,7 +413,6 @@ bool tls1_check_group_id(const SSL_HANDSHAKE *hs, uint16_t group_id) {
// algorithms for verifying.
static const uint16_t kVerifySignatureAlgorithms[] = {
// List our preferred algorithms first.
SSL_SIGN_ED25519,
SSL_SIGN_ECDSA_SECP256R1_SHA256,
SSL_SIGN_RSA_PSS_RSAE_SHA256,
SSL_SIGN_RSA_PKCS1_SHA256,
Expand Down Expand Up @@ -455,41 +454,15 @@ static const uint16_t kSignSignatureAlgorithms[] = {
SSL_SIGN_RSA_PKCS1_SHA1,
};

struct SSLSignatureAlgorithmList {
bool Next(uint16_t *out) {
while (!list.empty()) {
uint16_t sigalg = list[0];
list = list.subspan(1);
if (skip_ed25519 && sigalg == SSL_SIGN_ED25519) {
continue;
}
*out = sigalg;
return true;
}
return false;
static Span<const uint16_t> tls12_get_verify_sigalgs(const SSL_HANDSHAKE *hs) {
if (hs->config->verify_sigalgs.empty()) {
return Span<const uint16_t>(kVerifySignatureAlgorithms);
}

Span<const uint16_t> list;
bool skip_ed25519 = false;
};

static SSLSignatureAlgorithmList tls12_get_verify_sigalgs(
const SSL_HANDSHAKE *hs) {
SSL *const ssl = hs->ssl;
SSLSignatureAlgorithmList ret;
if (!hs->config->verify_sigalgs.empty()) {
ret.list = hs->config->verify_sigalgs;
} else {
ret.list = kVerifySignatureAlgorithms;
ret.skip_ed25519 = !ssl->ctx->ed25519_enabled;
}
return ret;
return hs->config->verify_sigalgs;
}

bool tls12_add_verify_sigalgs(const SSL_HANDSHAKE *hs, CBB *out) {
SSLSignatureAlgorithmList list = tls12_get_verify_sigalgs(hs);
uint16_t sigalg;
while (list.Next(&sigalg)) {
for (uint16_t sigalg : tls12_get_verify_sigalgs(hs)) {
if (!CBB_add_u16(out, sigalg)) {
return false;
}
Expand All @@ -499,9 +472,7 @@ bool tls12_add_verify_sigalgs(const SSL_HANDSHAKE *hs, CBB *out) {

bool tls12_check_peer_sigalg(const SSL_HANDSHAKE *hs, uint8_t *out_alert,
uint16_t sigalg) {
SSLSignatureAlgorithmList list = tls12_get_verify_sigalgs(hs);
uint16_t verify_sigalg;
while (list.Next(&verify_sigalg)) {
for (uint16_t verify_sigalg : tls12_get_verify_sigalgs(hs)) {
if (verify_sigalg == sigalg) {
return true;
}
Expand Down Expand Up @@ -3871,7 +3842,3 @@ int SSL_early_callback_ctx_extension_get(const SSL_CLIENT_HELLO *client_hello,
*out_len = CBS_len(&cbs);
return 1;
}

void SSL_CTX_set_ed25519_enabled(SSL_CTX *ctx, int enabled) {
ctx->ed25519_enabled = !!enabled;
}
6 changes: 3 additions & 3 deletions ssl/test/runner/runner.go
Expand Up @@ -4880,7 +4880,7 @@ func addStateMachineCoverageTests(config stateMachineTestConfig) {
flags: []string{
"-cert-file", path.Join(*resourceDir, ed25519CertificateFile),
"-key-file", path.Join(*resourceDir, ed25519KeyFile),
"-enable-ed25519",
"-verify-prefs", strconv.Itoa(int(signatureEd25519)),
},
})

Expand Down Expand Up @@ -9720,7 +9720,7 @@ func addSignatureAlgorithmTests() {
UseLegacySigningAlgorithm: signatureEd25519,
},
},
flags: []string{"-enable-ed25519"},
flags: []string{"-verify-prefs", strconv.Itoa(int(signatureEd25519))},
shouldFail: true,
expectedError: ":PEER_ERROR_UNSUPPORTED_CERTIFICATE_TYPE:",
})
Expand Down Expand Up @@ -9749,7 +9749,7 @@ func addSignatureAlgorithmTests() {
},
},
flags: []string{
"-enable-ed25519",
"-verify-prefs", strconv.Itoa(int(signatureEd25519)),
"-require-any-client-certificate",
},
shouldFail: true,
Expand Down
5 changes: 0 additions & 5 deletions ssl/test/test_config.cc
Expand Up @@ -128,7 +128,6 @@ const Flag<bool> kBoolFlags[] = {
{"-no-op-extra-handshake", &TestConfig::no_op_extra_handshake},
{"-handshake-twice", &TestConfig::handshake_twice},
{"-allow-unknown-alpn-protos", &TestConfig::allow_unknown_alpn_protos},
{"-enable-ed25519", &TestConfig::enable_ed25519},
{"-use-custom-verify-callback", &TestConfig::use_custom_verify_callback},
{"-allow-false-start-without-alpn",
&TestConfig::allow_false_start_without_alpn},
Expand Down Expand Up @@ -1259,10 +1258,6 @@ bssl::UniquePtr<SSL_CTX> TestConfig::SetupCtx(SSL_CTX *old_ctx) const {
SSL_CTX_set_allow_unknown_alpn_protos(ssl_ctx.get(), 1);
}

if (enable_ed25519) {
SSL_CTX_set_ed25519_enabled(ssl_ctx.get(), 1);
}

if (!verify_prefs.empty()) {
std::vector<uint16_t> u16s(verify_prefs.begin(), verify_prefs.end());
if (!SSL_CTX_set_verify_algorithm_prefs(ssl_ctx.get(), u16s.data(),
Expand Down
1 change: 0 additions & 1 deletion ssl/test/test_config.h
Expand Up @@ -152,7 +152,6 @@ struct TestConfig {
bool no_op_extra_handshake = false;
bool handshake_twice = false;
bool allow_unknown_alpn_protos = false;
bool enable_ed25519 = false;
bool use_custom_verify_callback = false;
std::string expect_msg_callback;
bool allow_false_start_without_alpn = false;
Expand Down
7 changes: 0 additions & 7 deletions tool/client.cc
Expand Up @@ -136,9 +136,6 @@ static const struct argument kArguments[] = {
"this flag is the early data to send or if it starts with '@', the "
"file to read from for early data.",
},
{
"-ed25519", kBooleanArgument, "Advertise Ed25519 support",
},
{
"-http-tunnel", kOptionalArgument,
"An HTTP proxy server to tunnel the TCP connection through",
Expand Down Expand Up @@ -531,10 +528,6 @@ bool Client(const std::vector<std::string> &args) {
SSL_CTX_set_early_data_enabled(ctx.get(), 1);
}

if (args_map.count("-ed25519") != 0) {
SSL_CTX_set_ed25519_enabled(ctx.get(), 1);
}

if (args_map.count("-debug") != 0) {
SSL_CTX_set_info_callback(ctx.get(), InfoCallback);
}
Expand Down

0 comments on commit 1766935

Please sign in to comment.