Skip to content

Commit

Permalink
Ignore -named_curve auto value to improve backwards compatibility
Browse files Browse the repository at this point in the history
Fixes #3490

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #3518)
  • Loading branch information
t8m authored and mattcaswell committed Jun 8, 2017
1 parent 0b20ad1 commit 1c7aa0d
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGES
Expand Up @@ -14,6 +14,10 @@
than just the call where this user data is passed.
[Richard Levitte]

*) Ignore the '-named_curve auto' value for compatibility of applications
with OpenSSL 1.0.2.
[Tomas Mraz <tmraz@fedoraproject.org>]

*) Fragmented SSL/TLS alerts are no longer accepted. An alert message is 2
bytes long. In theory it is permissible in SSLv3 - TLSv1.2 to fragment such
alerts across multiple records (some of which could be empty). In practice
Expand Down
8 changes: 8 additions & 0 deletions ssl/ssl_conf.c
Expand Up @@ -227,6 +227,14 @@ static int cmd_ECDHParameters(SSL_CONF_CTX *cctx, const char *value)
EC_KEY *ecdh;
int nid;

/* Ignore values supported by 1.0.2 for the automatic selection */
if ((cctx->flags & SSL_CONF_FLAG_FILE) &&
strcasecmp(value, "+automatic") == 0)

This comment has been minimized.

Copy link
@trueqbit

trueqbit Aug 4, 2017

Contributor

Comparing to "+automatic" is problematic. Documentation states that ECDHParameters accepts a special value "Automatic", and openssl-1.0.2 checked using strcasecmp(value, "automatic")

This comment has been minimized.

Copy link
@richsalz

richsalz Aug 6, 2017

Contributor

No, the diff is correct. You're missing the "+" test a few lines before in the 1.0.2 code.
We want to error out on -automatic to avoid silent configuration mistakes.

This comment has been minimized.

Copy link
@trueqbit

trueqbit Aug 7, 2017

Contributor

Yep, you are right! However, with 1.0.2 the plus can be omitted and the option is then treated as 'on'. So, I happen to have a configuration file with ECDHParameters=Automatic and 1.1.0 errors out.

This comment has been minimized.

Copy link
@richsalz

richsalz Aug 7, 2017

Contributor

Please open a new issue with this; we should fix it, thanks.

This comment has been minimized.

Copy link
@trueqbit

trueqbit Aug 29, 2017

Contributor

Just cross-referencing issue 4113, which also includes a PR.

return 1;
if ((cctx->flags & SSL_CONF_FLAG_CMDLINE) &&
strcmp(value, "auto") == 0)
return 1;

nid = EC_curve_nist2nid(value);
if (nid == NID_undef)
nid = OBJ_sn2nid(value);
Expand Down

0 comments on commit 1c7aa0d

Please sign in to comment.