Mailing List Archive

https://bugs.exim.org/show_bug.cgi?id=2643

Phil Pennock <pdp@exim.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED

--- Comment #1 from Phil Pennock <pdp@exim.org> ---
Can you please provide some provenance for how you derive your proposed change?

For now, I am standing by the code as currently implemented in Exim, as being
derived by code from the constants as literally written in the RFC. If there's
a problem then it's a bug in util/gen_pkcs3.c and we need to fix the bug and
audit all other values.

Compiling gen_pkcs3.c on Ubuntu Focal requires `-D_XOPEN_SOURCE=1` in addition
to the command-line at the top of that file, because Linux headers are
obnoxious:

c99 -D_XOPEN_SOURCE=1 $(pkg-config --cflags openssl) gen_pkcs3.c
$(pkg-config --libs openssl)

% ./gen_pkcs3 -cCs ' FFFFFFFF FFFFFFFF ADF85458 A2BB4A9A AFDC5620 273D3CF1
D8B9C583 CE2D3695 A9E13641 146433FB CC939DCE 249B3EF9
7D2FE363 630C75D8 F681B202 AEC4617A D3DF1ED5 D5FD6561
2433F51F 5F066ED0 85636555 3DED1AF3 B557135E 7F57C935
984F0C70 E0E68B77 E2A689DA F3EFE872 1DF158A1 36ADE735
30ACCA4F 483A797A BC0AB182 B324FB61 D108A94B B2C8E3FB
B96ADAB7 60D7F468 1D4F42A3 DE394DF4 AE56EDE7 6372BB19
0B07A7C8 EE0A6D70 9E02FCE1 CDF7E2EC C03404CD 28342F61
9172FE9C E98583FF 8E4F1232 EEF28183 C3FE3B1B 4C6FAD73
3BB5FCBC 2EC22005 C58EF183 7D1683B2 C6F34A26 C1B2EFFA
886B4238 61285C97 FFFFFFFF FFFFFFFF
' 2 ' 7FFFFFFF FFFFFFFF D6FC2A2C 515DA54D 57EE2B10 139E9E78
EC5CE2C1 E7169B4A D4F09B20 8A3219FD E649CEE7 124D9F7C
BE97F1B1 B1863AEC 7B40D901 576230BD 69EF8F6A EAFEB2B0
9219FA8F AF833768 42B1B2AA 9EF68D79 DAAB89AF 3FABE49A
CC278638 707345BB F15344ED 79F7F439 0EF8AC50 9B56F39A
98566527 A41D3CBD 5E0558C1 59927DB0 E88454A5 D96471FD
DCB56D5B B06BFA34 0EA7A151 EF1CA6FA 572B76F3 B1B95D8C
8583D3E4 770536B8 4F017E70 E6FBF176 601A0266 941A17B0
C8B97F4E 74C2C1FF C7278919 777940C1 E1FF1D8D A637D6B9
9DDAFE5E 17611002 E2C778C1 BE8B41D9 6379A513 60D977FD
4435A11C 30942E4B FFFFFFFF FFFFFFFF
'
p =
FFFFFFFFFFFFFFFFADF85458A2BB4A9AAFDC5620273D3CF1D8B9C583CE2D3695A9E13641146433FBCC939DCE249B3EF97D2FE363630C75D8F681B202AEC4617AD3DF1ED5D5FD65612433F51F5F066ED0856365553DED1AF3B557135E7F57C935984F0C70E0E68B77E2A689DAF3EFE8721DF158A136ADE73530ACCA4F483A797ABC0AB182B324FB61D108A94BB2C8E3FBB96ADAB760D7F4681D4F42A3DE394DF4AE56EDE76372BB190B07A7C8EE0A6D709E02FCE1CDF7E2ECC03404CD28342F619172FE9CE98583FF8E4F1232EEF28183C3FE3B1B4C6FAD733BB5FCBC2EC22005C58EF1837D1683B2C6F34A26C1B2EFFA886B423861285C97FFFFFFFFFFFFFFFF
g = 2
q =
7FFFFFFFFFFFFFFFD6FC2A2C515DA54D57EE2B10139E9E78EC5CE2C1E7169B4AD4F09B208A3219FDE649CEE7124D9F7CBE97F1B1B1863AEC7B40D901576230BD69EF8F6AEAFEB2B09219FA8FAF83376842B1B2AA9EF68D79DAAB89AF3FABE49ACC278638707345BBF15344ED79F7F4390EF8AC509B56F39A98566527A41D3CBD5E0558C159927DB0E88454A5D96471FDDCB56D5BB06BFA340EA7A151EF1CA6FA572B76F3B1B95D8C8583D3E4770536B84F017E70E6FBF176601A0266941A17B0C8B97F4E74C2C1FFC7278919777940C1E1FF1D8DA637D6B99DDAFE5E17611002E2C778C1BE8B41D96379A51360D977FD4435A11C30942E4BFFFFFFFFFFFFFFFF
"-----BEGIN DH PARAMETERS-----\n"
"MIIBDAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz\n"
"+8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a\n"
"87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7\n"
"YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi\n"
"7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD\n"
"ssbzSibBsu/6iGtCOGEoXJf//////////wIBAgICB/8=\n"
"-----END DH PARAMETERS-----\n";


Those printed values look the same as the input values, so they've
round-tripped through the code just fine. With `-c`, `DH_check()` has been
called and passed.

We really need a more detailed report showing exactly how the current values
are wrong and how the proposed new constant was derived before we'll accept a
PR here: we're not taking changes to security-impacting constants with no
provenance on how the new code was derived.

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

https://bugs.exim.org/show_bug.cgi?id=2643

--- Comment #2 from Phil Pennock <pdp@exim.org> ---
The proposed replacement value appears to come from Mozilla at
<https://ssl-config.mozilla.org/ffdhe2048.txt> as noted by adsb on our IRC
channel.

I've asked the Mozilla folks for more details in:
https://github.com/mozilla/ssl-config-generator/issues/116

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

https://bugs.exim.org/show_bug.cgi?id=2643

--- Comment #3 from kylon94@protonmail.com ---
std-crypto.c line 510-518 is

static const char dh_ffdhe2048_pem[] =
"-----BEGIN DH PARAMETERS-----\n"
"MIH+AoH4DfhUWKK7Spqv3FYgJz088di5xYPOLTaVqeE2QRRkM/vMk53OJJs++X0v\n"
"42NjDHXY9oGyAq7EYXrT3x7V1f1lYSQz9R9fBm7QhWNlVT3tGvO1VxNef1fJNZhP\n"
"DHDg5ot34qaJ2vPv6HId8VihNq3nNTCsyk9IOnl6vAqxgrMk+2HRCKlLssjj+7lq\n"
"2rdg1/RoHU9Co945TfSuVu3nY3K7GQsHp8juCm1wngL84c334uzANATNKDQvYZFy\n"
"/pzphYP/jk8SMu7ygYPD/jsbTG+tczu1/LwuwiAFxY7xg30Wg7LG80omwbLv+ohr\n"
"QjhhKFyX//////////8CAQI=\n"
"-----END DH PARAMETERS-----\n";

and not
"-----BEGIN DH PARAMETERS-----\n"
"MIIBDAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz\n"
"+8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a\n"
"87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7\n"
"YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi\n"
"7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD\n"
"ssbzSibBsu/6iGtCOGEoXJf//////////wIBAgICB/8=\n"
"-----END DH PARAMETERS-----\n";

I derive this from the RFC definition and openssl dhparam -in [RFC 7919 value]
-text

My findings agree with
https://github.com/mozilla/ssl-config-generator/issues/116#issuecomment-694535540.
Looks like the current parameter in 510-518 is missing values.

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

https://bugs.exim.org/show_bug.cgi?id=2643

--- Comment #4 from Phil Pennock <pdp@exim.org> ---
Er yes, I should have updated this ticket too, sorry.

We have a fix on a branch, but I didn't merge it to the main branch because I
wanted more eyes on it. I don't understand how I went wrong before so it would
be hubris to imagine I've gotten it right this time.

See
https://git.exim.org/exim.git/shortlog/refs/heads/pdp/regen-dh-c
and the diff:

https://git.exim.org/exim.git/commitdiff/b48148d7da21797c3a0926013ea327b42b5f157b

There's just too many small differences to be sanely explained away here, given
that I wrote tooling to manage the conversion _precisely_ to avoid this sort of
issue.

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

https://bugs.exim.org/show_bug.cgi?id=2643

--- Comment #5 from Simon Arlott <bugzilla.exim.simon@arlott.org> ---
You're not terminating the string before calling BH_hex2bn():

==34241== Memcheck, a memory error detector
==34241== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==34241== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==34241== Command: ./a.out -cCs FFFFFFFF\ FFFFFFFF\ ADF85458\ A2BB4A9A\
AFDC5620\ 273D3CF1
==34241== \ \ \ \ D8B9C583\ CE2D3695\ A9E13641\ 146433FB\ CC939DCE\ 249B3EF9
==34241== \ \ \ \ 7D2FE363\ 630C75D8\ F681B202\ AEC4617A\ D3DF1ED5\ D5FD6561
==34241== \ \ \ \ 2433F51F\ 5F066ED0\ 85636555\ 3DED1AF3\ B557135E\ 7F57C935
==34241== \ \ \ \ 984F0C70\ E0E68B77\ E2A689DA\ F3EFE872\ 1DF158A1\ 36ADE735
==34241== \ \ \ \ 30ACCA4F\ 483A797A\ BC0AB182\ B324FB61\ D108A94B\ B2C8E3FB
==34241== \ \ \ \ B96ADAB7\ 60D7F468\ 1D4F42A3\ DE394DF4\ AE56EDE7\ 6372BB19
==34241== \ \ \ \ 0B07A7C8\ EE0A6D70\ 9E02FCE1\ CDF7E2EC\ C03404CD\ 28342F61
==34241== \ \ \ \ 9172FE9C\ E98583FF\ 8E4F1232\ EEF28183\ C3FE3B1B\ 4C6FAD73
==34241== \ \ \ \ 3BB5FCBC\ 2EC22005\ C58EF183\ 7D1683B2\ C6F34A26\ C1B2EFFA
==34241== \ \ \ \ 886B4238\ 61285C97\ FFFFFFFF\ FFFFFFFF 2 7FFFFFFF\ FFFFFFFF\
D6FC2A2C\ 515DA54D\ 57EE2B10\ 139E9E78
==34241== \ \ \ \ EC5CE2C1\ E7169B4A\ D4F09B20\ 8A3219FD\ E649CEE7\ 124D9F7C
==34241== \ \ \ \ BE97F1B1\ B1863AEC\ 7B40D901\ 576230BD\ 69EF8F6A\ EAFEB2B0
==34241== \ \ \ \ 9219FA8F\ AF833768\ 42B1B2AA\ 9EF68D79\ DAAB89AF\ 3FABE49A
==34241== \ \ \ \ CC278638\ 707345BB\ F15344ED\ 79F7F439\ 0EF8AC50\ 9B56F39A
==34241== \ \ \ \ 98566527\ A41D3CBD\ 5E0558C1\ 59927DB0\ E88454A5\ D96471FD
==34241== \ \ \ \ DCB56D5B\ B06BFA34\ 0EA7A151\ EF1CA6FA\ 572B76F3\ B1B95D8C
==34241== \ \ \ \ 8583D3E4\ 770536B8\ 4F017E70\ E6FBF176\ 601A0266\ 941A17B0
==34241== \ \ \ \ C8B97F4E\ 74C2C1FF\ C7278919\ 777940C1\ E1FF1D8D\ A637D6B9
==34241== \ \ \ \ 9DDAFE5E\ 17611002\ E2C778C1\ BE8B41D9\ 6379A513\ 60D977FD
==34241== \ \ \ \ 4435A11C\ 30942E4B\ FFFFFFFF\ FFFFFFFF
==34241==
==34241== Conditional jump or move depends on uninitialised value(s)
==34241== at 0x4F22FF5: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x4EFF14A: BN_hex2bn (in
/usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x1092CA: bn_from_text (in /home/simon/src/exim/src/util/a.out)
==34241== by 0x1096B6: main (in /home/simon/src/exim/src/util/a.out)
==34241==
==34241== Use of uninitialised value of size 8
==34241== at 0x4F23001: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x4EFF14A: BN_hex2bn (in
/usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x1092CA: bn_from_text (in /home/simon/src/exim/src/util/a.out)
==34241== by 0x1096B6: main (in /home/simon/src/exim/src/util/a.out)
==34241==
==34241== Invalid read of size 1
==34241== at 0x4EFF2FD: BN_hex2bn (in
/usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x1092CA: bn_from_text (in /home/simon/src/exim/src/util/a.out)
==34241== by 0x1096CD: main (in /home/simon/src/exim/src/util/a.out)
==34241== Address 0x5b1b491 is 0 bytes after a block of size 1 alloc'd
==34241== at 0x4C2FB0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==34241== by 0x10920E: bn_from_text (in /home/simon/src/exim/src/util/a.out)
==34241== by 0x1096CD: main (in /home/simon/src/exim/src/util/a.out)
==34241==
==34241== Conditional jump or move depends on uninitialised value(s)
==34241== at 0x4F22FF5: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x4EFF14A: BN_hex2bn (in
/usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x1092CA: bn_from_text (in /home/simon/src/exim/src/util/a.out)
==34241== by 0x1096EA: main (in /home/simon/src/exim/src/util/a.out)
==34241==
==34241== Use of uninitialised value of size 8
==34241== at 0x4F23001: ??? (in /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x4EFF14A: BN_hex2bn (in
/usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==34241== by 0x1092CA: bn_from_text (in /home/simon/src/exim/src/util/a.out)
==34241== by 0x1096EA: main (in /home/simon/src/exim/src/util/a.out)
==34241==
p =
FFFFFFFFFFFFFFFFADF85458A2BB4A9AAFDC5620273D3CF1D8B9C583CE2D3695A9E13641146433FBCC939DCE249B3EF97D2FE363630C75D8F681B202AEC4617AD3DF1ED5D5FD65612433F51F5F066ED0856365553DED1AF3B557135E7F57C935984F0C70E0E68B77E2A689DAF3EFE8721DF158A136ADE73530ACCA4F483A797ABC0AB182B324FB61D108A94BB2C8E3FBB96ADAB760D7F4681D4F42A3DE394DF4AE56EDE76372BB190B07A7C8EE0A6D709E02FCE1CDF7E2ECC03404CD28342F619172FE9CE98583FF8E4F1232EEF28183C3FE3B1B4C6FAD733BB5FCBC2EC22005C58EF1837D1683B2C6F34A26C1B2EFFA886B423861285C97FFFFFFFFFFFFFFFF
g = 2
q =
7FFFFFFFFFFFFFFFD6FC2A2C515DA54D57EE2B10139E9E78EC5CE2C1E7169B4AD4F09B208A3219FDE649CEE7124D9F7CBE97F1B1B1863AEC7B40D901576230BD69EF8F6AEAFEB2B09219FA8FAF83376842B1B2AA9EF68D79DAAB89AF3FABE49ACC278638707345BBF15344ED79F7F4390EF8AC509B56F39A98566527A41D3CBD5E0558C159927DB0E88454A5D96471FDDCB56D5BB06BFA340EA7A151EF1CA6FA572B76F3B1B95D8C8583D3E4770536B84F017E70E6FBF176601A0266941A17B0C8B97F4E74C2C1FFC7278919777940C1E1FF1D8DA637D6B99DDAFE5E17611002E2C778C1BE8B41D96379A51360D977FD4435A11C30942E4BFFFFFFFFFFFFFFFF
"-----BEGIN DH PARAMETERS-----\n"
"MIIBDAKCAQEA//////////+t+FRYortKmq/cViAnPTzx2LnFg84tNpWp4TZBFGQz\n"
"+8yTnc4kmz75fS/jY2MMddj2gbICrsRhetPfHtXV/WVhJDP1H18GbtCFY2VVPe0a\n"
"87VXE15/V8k1mE8McODmi3fipona8+/och3xWKE2rec1MKzKT0g6eXq8CrGCsyT7\n"
"YdEIqUuyyOP7uWrat2DX9GgdT0Kj3jlN9K5W7edjcrsZCwenyO4KbXCeAvzhzffi\n"
"7MA0BM0oNC9hkXL+nOmFg/+OTxIy7vKBg8P+OxtMb61zO7X8vC7CIAXFjvGDfRaD\n"
"ssbzSibBsu/6iGtCOGEoXJf//////////wIBAgICB/8=\n"
"-----END DH PARAMETERS-----\n";
==34241==
==34241== HEAP SUMMARY:
==34241== in use at exit: 1,963 bytes in 9 blocks
==34241== total heap usage: 229 allocs, 220 frees, 89,623 bytes allocated
==34241==
==34241== LEAK SUMMARY:
==34241== definitely lost: 1,351 bytes in 4 blocks
==34241== indirectly lost: 612 bytes in 5 blocks
==34241== possibly lost: 0 bytes in 0 blocks
==34241== still reachable: 0 bytes in 0 blocks
==34241== suppressed: 0 bytes in 0 blocks
==34241== Rerun with --leak-check=full to see details of leaked memory
==34241==
==34241== For counts of detected and suppressed errors, rerun with: -v
==34241== Use --track-origins=yes to see where uninitialised values come from
==34241== ERROR SUMMARY: 5 errors from 5 contexts (suppressed: 0 from 0)

Fix here:
https://github.com/nomis/exim/commit/820a704a91a626510258849c39f3cbd64e127c44

That doesn't explain how the first "FFFFFFFFFFFFFFFFA" are missing from the
value and that's not even a whole number of bytes. I can't reproduce it.

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

https://bugs.exim.org/show_bug.cgi?id=2643

--- Comment #6 from Simon Arlott <bugzilla.exim.simon@arlott.org> ---
Updated fix here: https://github.com/nomis/exim/commits/gen_pkcs3

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

https://bugs.exim.org/show_bug.cgi?id=2643

--- Comment #7 from Simon Arlott <bugzilla.exim.simon@arlott.org> ---
If I use "0DF85458" instead of "FFFFFFFF FFFFFFFF ADF85458" with OpenSSL 1.0.2g
(Ubuntu 16.04) then I get the same output.

You need an older version of OpenSSL to get rid of the
"recommended-private-length: 2047 bits" addition to the end, and you have to
disable the checking.

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

https://bugs.exim.org/show_bug.cgi?id=2643

--- Comment #8 from Phil Pennock <pdp@exim.org> ---
:facepalm:

Thank you.

Although if that had happened, we'd see "BN_hex2bn did not convert entire
input; took N of M bytes" with N > M. But we'll definitely take the fix,
thanks.

--
You are receiving this mail because:
You are on the CC list for the bug.
--
## List details at https://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##