Greg Veldman wrote:
On Fri, Nov 13, 2020 at 03:52:54PM +0300, Yuri Pankov wrote:
OK, I was able to reproduce this; actually, it hit that assertion for
all Windows 10 20H2 installs I have, all are updated to latest patches,
so I can't tell when this started to happen. Adding a debug print
before that assert() shows the following:
$ DISPLAY=mercury:0.0
/usr/ports/net/rdesktop/work/rdesktop-1.9.0/rdesktop orion
len=128 size=128
Assertion failed: ((len * 2) < size), function _utils_data_to_hex, file
utils.c, line 501.
So we need the len of 128 we need size of digest buf to be > 256, the
following patch worked for me:
polaris:ypankov:/usr/ports/net/rdesktop$ cat files/patch-utils.c
--- utils.c.orig 2020-11-13 12:40:51 UTC
+++ utils.c
@@ -584,7 +584,7 @@ _utils_cert_get_info(gnutls_x509_crt_t cert, char *out
{
char buf[128];
size_t buf_size;
- char digest[128];
+ char digest[512];
gnutls_x509_dn_t dn;
time_t expire_ts, activated_ts;
This seems like a reasonable change, but I'll make one minor
note. Later on in _utils_cert_get_info() the contents of
digest are written to sha1 and sha256, which are both size
256. In the initial implementation both buf and digest are
of the same size, which would seem to indicate that buf was
not expected to be completely full (in fact less than half
full based on the assertion in _utils_data_to_hex()). However
if defined to 512 chars and ever somehow filled beyond 256
(in fact a bit less than that, since a static string is
prepended to the call to snprintf()), the writes to sha1 and
sha256 would be incomplete, which I haven't completely read
through but would very likely lead to Bad Things(tm) later on.
All this to say, perhaps a more conservative approach would be
to increase digest to 256 chars instead of 512. I don't currently
have a Windows 10 20H2 machine to test on, but would you mind
trying with 256 instead of 512 and see if that also fixes the
issue? If so, let me know and I'll submit a PR to get this
patch added.
size is 128 now, so with digest of 256 we fail the ((len * 2) < size)
assertion, 257 works though (or changing the assertion to be <=).
Actually, this does not look like a real fix. I have looked more into
it and something is still very wrong -- _utils_cert_get_info() calls
gnutls_x509_crt_get_fingerprint() and does NOT check the return value
while it is -72 (GNUTLS_E_ASN1_VALUE_NOT_VALID), while the only
*documented* return values are 0 (got fingerprint successfully) and -51
(GNUTLS_E_SHORT_MEMORY_BUFFER, not enough buffer space). I don't know
if it was the same previously as I don't have a system to test against.
If yes, then this patch could be used in ports at least until upstream
fixes the problem properly; if no, then we would need a proper fix first.
Also note that sha1 and sha256 fingerprints reported are the *same*,
which doesn't look right with this approach.
_______________________________________________
freebsd-ports@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-ports
To unsubscribe, send any mail to "freebsd-ports-unsubscr...@freebsd.org"