search

PeterMosmans / openssl

'Extra featured' OpenSSL with ChaCha20 and Poly1305 support
https://onwebsecurity.com/pages/openssl.html
Other
92 stars 18 forks source link

Conflict of cipher names #43

Closed drwetter closed 8 years ago

drwetter commented 8 years ago

Hi Peter,

as stated in https://github.com/drwetter/testssl.sh/issues/379 the names of the new CHACHA/POLY ciphers conflict with the old ones.

This IMO needs to be addressed in the current branch, too.. To shorten matters for me, enclosed is my suggestion. new-ciphers.diff.txt

Here I would prefer _OLD, as A) I read it somewhere before B) to restrict the length of the output.

Please note I also added for the TLS1_CK* defines the "WITH" which seem to be missing.

See also #38.

Cheers, Dirk

drwetter commented 8 years ago

Ahh, I think it was SSLlabs:

https://dev.ssllabs.com/ssltest/analyze.html?d=google.com&s=172.217.4.110&hideResults=on

However that was in front of the name, don't know whether there was a reason but it's quite ugly IMO.

drwetter commented 8 years ago

(but they have a leading OLD_ and it's in the RFC name, I'll try to clarify)

PeterMosmans commented 8 years ago

@drwetter - thanks for the req (feel free to send in pull requests :smile: ). However, to keep in line with SSLabs I'd suggest we go with 'their' naming of OLD_ .. - what would you say of that ? Better to have all names in line, to prevent even more confusion...

drwetter commented 8 years ago

Ivan replied https://twitter.com/ivanristic/status/742445496061546497. So there's no naming standard also according to him.

yeah, I am busy these days. Will do a PR...

PeterMosmans commented 8 years ago

Fixed slightly different, according to Ivan Ristic' naming:

"OLD-ECDHE-RSA-CHACHA20-POLY1305" "OLD-ECDHE-ECDSA-CHACHA20-POLY1305" "OLD-DHE-RSA-CHACHA20-POLY1305"

drwetter commented 8 years ago

Hi Peter,

Ivan uses the RFC/IANA naming scheme and not OpenSSL. So what we is being done here, is independent of Ivan. Can't tell whether the preceding OLD_ works with openssl. Can't you check? (I do not understand the magic here in openssl yet)

Then: The diff I proposed contained WITH for TLS_CK* macros, to be consistent with the other (newer) ciphers in ssl/tls1.h + ssl/s3_lib.c.

In the past openssl s_client -connect <target> -trace wasn't able to display the CHACHA/POLY ciphers, don't know whether this is related to the missing WITH.

Cheers, Dirk

v998 commented 8 years ago

Let's see..

CloudFlare's chacha20 patch:


+/* ChaCha20-Poly1305 ciphersuites draft-agl-tls-chacha20poly1305-01 */
+# define TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305_D     "ECDHE-RSA-CHACHA20-POLY1305-D"
+# define TLS1_TXT_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_D   "ECDHE-ECDSA-CHACHA20-POLY1305-D"
+# define TLS1_TXT_DHE_RSA_WITH_CHACHA20_POLY1305_D       "DHE-RSA-CHACHA20-POLY1305-D"
+
+/* Chacha20-Poly1305 ciphersuites from RFC */
+# define TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305       "ECDHE-RSA-CHACHA20-POLY1305"
+# define TLS1_TXT_ECDHE_ECDSA_WITH_CHACHA20_POLY1305     "ECDHE-ECDSA-CHACHA20-POLY1305"
+# define TLS1_TXT_DHE_RSA_WITH_CHACHA20_POLY1305         "DHE-RSA-CHACHA20-POLY1305"

LibreSSL:

/* ChaCha20-Poly1305 based ciphersuites. */
#define TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305_OLD   "ECDHE-RSA-CHACHA20-POLY1305-OLD"
#define TLS1_TXT_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_OLD "ECDHE-ECDSA-CHACHA20-POLY1305-OLD"
#define TLS1_TXT_DHE_RSA_WITH_CHACHA20_POLY1305_OLD "DHE-RSA-CHACHA20-POLY1305-OLD"
#define TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305   "ECDHE-RSA-CHACHA20-POLY1305"
#define TLS1_TXT_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 "ECDHE-ECDSA-CHACHA20-POLY1305"
#define TLS1_TXT_DHE_RSA_WITH_CHACHA20_POLY1305     "DHE-RSA-CHACHA20-POLY1305"

BoringSSL:

/* For convenience, the old and new CHACHA20_POLY1305 ciphers have the same
 * name. In cipher strings, both will be selected. This is temporary and will be
 * removed when the pre-standard construction is removed. */
#define TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305_OLD \
  "ECDHE-RSA-CHACHA20-POLY1305"
#define TLS1_TXT_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_OLD \
  "ECDHE-ECDSA-CHACHA20-POLY1305"
#define TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 \
  "ECDHE-RSA-CHACHA20-POLY1305"
#define TLS1_TXT_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 \
  "ECDHE-ECDSA-CHACHA20-POLY1305"
#define TLS1_TXT_ECDHE_PSK_WITH_CHACHA20_POLY1305_SHA256 \
  "ECDHE-PSK-CHACHA20-POLY1305"
PeterMosmans commented 8 years ago

Thanks @v998 for looking that up, much appreciated ! I'll go with the -OLD suffix then, and change it accordingly. @drwetter I'll take a look at your macro changes as well

drwetter commented 8 years ago

Hi Peter,

I'd would go with LibreSSL (BoringSSL is the mess we have). That would look like:

https://github.com/drwetter/testssl.sh/issues/379#issuecomment-226295810

Left hand side: OpenSSL (my diff = LibreSSL), right hand side patched according to @ivanr naming.

Cheers, Dirk