From owner-freebsd-current@freebsd.org Sat Dec 28 04:42:57 2019 Return-Path: Delivered-To: freebsd-current@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 752781D9E83 for ; Sat, 28 Dec 2019 04:42:57 +0000 (UTC) (envelope-from freebsd-rwg@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (br1.CN84in.dnsmgr.net [69.59.192.140]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 47l9zg6ZsGz3D4h for ; Sat, 28 Dec 2019 04:42:55 +0000 (UTC) (envelope-from freebsd-rwg@gndrsh.dnsmgr.net) Received: from gndrsh.dnsmgr.net (localhost [127.0.0.1]) by gndrsh.dnsmgr.net (8.13.3/8.13.3) with ESMTP id xBS4grM1041519; Fri, 27 Dec 2019 20:42:53 -0800 (PST) (envelope-from freebsd-rwg@gndrsh.dnsmgr.net) Received: (from freebsd-rwg@localhost) by gndrsh.dnsmgr.net (8.13.3/8.13.3/Submit) id xBS4grnp041518; Fri, 27 Dec 2019 20:42:53 -0800 (PST) (envelope-from freebsd-rwg) From: "Rodney W. Grimes" Message-Id: <201912280442.xBS4grnp041518@gndrsh.dnsmgr.net> Subject: Re: OpenSSL breaks factor(6) In-Reply-To: <20191228035155.GA62416@troutmask.apl.washington.edu> To: sgk@troutmask.apl.washington.edu Date: Fri, 27 Dec 2019 20:42:53 -0800 (PST) CC: "Rodney W. Grimes" , freebsd-current@freebsd.org X-Mailer: ELM [version 2.4ME+ PL121h (25)] MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=US-ASCII X-Rspamd-Queue-Id: 47l9zg6ZsGz3D4h X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=none (mx1.freebsd.org: domain of freebsd-rwg@gndrsh.dnsmgr.net has no SPF policy when checking 69.59.192.140) smtp.mailfrom=freebsd-rwg@gndrsh.dnsmgr.net X-Spamd-Result: default: False [-0.49 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.76)[-0.762,0]; RCPT_COUNT_THREE(0.00)[3]; TO_DN_SOME(0.00)[]; FROM_HAS_DN(0.00)[]; MIME_GOOD(-0.10)[text/plain]; RCVD_TLS_LAST(0.00)[]; DMARC_NA(0.00)[dnsmgr.net]; AUTH_NA(1.00)[]; NEURAL_HAM_LONG(-0.66)[-0.661,0]; TO_MATCH_ENVRCPT_SOME(0.00)[]; R_SPF_NA(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+]; ASN(0.00)[asn:13868, ipnet:69.59.192.0/19, country:US]; MID_RHS_MATCH_FROM(0.00)[]; IP_SCORE(0.04)[ip: (0.14), ipnet: 69.59.192.0/19(0.07), asn: 13868(0.02), country: US(-0.05)]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 28 Dec 2019 04:42:57 -0000 > On Fri, Dec 27, 2019 at 07:00:04PM -0800, Rodney W. Grimes wrote: > > > On Fri, Dec 27, 2019 at 01:47:17PM -0800, Steve Kargl wrote: > > > > On Fri, Dec 27, 2019 at 01:25:30PM -0800, Steve Kargl wrote: > > > > > The use of OpenSSL in factor(6) breaks factor(6) with respect to > > > > > its documentation. > > > > > > > > > > % man factor > > > > > ... > > > > > Numbers may be preceded by a single '+'. > > > > > ... > > > > > > > > > > % factor +125 > > > > > factor: +125: illegal numeric format. > > > > > > > > > > > > > This fixes factor(6) for the above issue. The issue with > > > > hexadecimal is not easily fixed. > > > > > > > > > > This patch now includes a fix for hexadecimal conversion. It > > > simple scans the string for a hex digit in [a,...,f] and assumes > > > that a hexadecimal string has been entered. A string that includes > > > character from the decimal digits is assumed to by a decimal > > > representation. > > > > It looks to me that the old code did the common method of > > try to convert as decimal, if that fails, try it as hex, > > if that fails report an error. > > > > Why is is that this common logic no longer works? > > AFAICT, BN_dec2bn and BN_hex2bn from OpenSSL scan from left > to right, does a conversion with what is possible, and reports > success. That is, for 1abc, BN_dec2bn can convert 1 to 1 and > reports success. The local implementations of these functions, > when OpenSSL is not used, does not do this partial conversion. I think I see now, the local implementaton checks for whole string conversion with a test for newline or null as the last byte converted by strtoul, the OpenSSL does not do this. So why ever use the, um, IMHO broken for this application, SSL versions of these functions? Or if we do need to use them for some reason apply the whole string conversion checks as wrappers around them? > > > > > > Index: factor.c > > > =================================================================== > > > --- factor.c (revision 355983) > > > +++ factor.c (working copy) > > > @@ -71,6 +71,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -104,6 +105,7 @@ > > > > > > #endif > > > > > > +static bool is_hex(char *str); > > > static void BN_print_dec_fp(FILE *, const BIGNUM *); > > > > > > static void pr_fact(BIGNUM *); /* print factors of a value */ > > > @@ -148,21 +150,25 @@ > > > for (p = buf; isblank(*p); ++p); > > > if (*p == '\n' || *p == '\0') > > > continue; > > > + if (*p == '+') p++; > > > if (*p == '-') > > > errx(1, "negative numbers aren't permitted."); > > > - if (BN_dec2bn(&val, buf) == 0 && > > > - BN_hex2bn(&val, buf) == 0) > > > > Why does this logic fail? > > See BN_hex2bn manpage. C/C++ does shortcircuits. With 1abc, > BN_dec2bn converts the string to 1, puts it in val, and returns > nonzero. BN_hex2bn is never called. Flipping the conditionals, > of course, doesn't work because 0-9 are digits in the hexadecimal > set (e.g., 111 is a valid hex and decimal string). > > > > - errx(1, "%s: illegal numeric format.", buf); > > > + ch = is_hex(p) ? BN_hex2bn(&val, p) : > > > + BN_dec2bn(&val, p); > > > + if (ch == 0) > > > + errx(1, "%s: illegal numeric format.", p); > > > pr_fact(val); > > > } > > > /* Factor the arguments. */ > > > else > > > - for (; *argv != NULL; ++argv) { > > > - if (argv[0][0] == '-') > > > + for (p = *argv; p != NULL; p = *++argv) { > > > + if (*p == '-') > > > errx(1, "negative numbers aren't permitted."); > > > - if (BN_dec2bn(&val, argv[0]) == 0 && > > > - BN_hex2bn(&val, argv[0]) == 0) > > > - errx(1, "%s: illegal numeric format.", argv[0]); > > > + if (*p == '+') p++; > > > + ch = is_hex(p) ? BN_hex2bn(&val, p) : > > > + BN_dec2bn(&val, p); > > > + if (ch == 0) > > > + errx(1, "%s: illegal numeric format.", p); > > > pr_fact(val); > > > } > > > exit(0); > > > @@ -343,10 +349,9 @@ > > > BN_dec2bn(BIGNUM **a, const char *str) > > > { > > > char *p; > > > - > > This blank line is part of style(9) > > > > Whoops. Haven't had to worry about style(9) in a long time. > > > > errno = 0; > > > **a = strtoul(str, &p, 10); > > > - return (errno == 0 && (*p == '\n' || *p == '\0')); > > > + return (errno == 0 ? 1 : 0); /* OpenSSL returns 0 on error! */ > > > } > > > > > > static int > > > @@ -356,7 +361,7 @@ > > > > > > errno = 0; > > > **a = strtoul(str, &p, 16); > > > - return (errno == 0 && (*p == '\n' || *p == '\0')); > > > + return (errno == 0 ? 1 : 0); /* OpenSSL returns 0 on error! */ > > > } > > > > > > static BN_ULONG > > > @@ -370,3 +375,17 @@ > > > } > > > > > > #endif > > > + > > > +/* Check if the string contains a hexadecimal digit. */ > > > +static bool > > > +is_hex(char *str) > > This function is poorly named as it does not check for > > all valid hex digits, only for alpha hex digits. It > > also only accepts lower case hex alpha, I would expect > > hex input to be case insensitive. > > > > is_hexalpha? > > Feel free to rename it and improve. > > > > > > +{ > > > + char c, *p; > > > + for (p = str; *p; p++) { > > > + c = tolower(*p); > > > + if (c == 'a' || c == 'b' || c == 'c' || c == 'd' || > > > + c == 'e' || c == 'f') > > if ( c >= 'a' || c <= 'f') > > > > > + return true; > > > + } > > > + return false; > > > +} > > > > > > -- > > > Steve > > > > -- > > Rod Grimes rgrimes@freebsd.org > > -- > Steve > 20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4 > 20161221 https://www.youtube.com/watch?v=IbCHE-hONow > _______________________________________________ > freebsd-current@freebsd.org mailing list > https://lists.freebsd.org/mailman/listinfo/freebsd-current > To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org" > -- Rod Grimes rgrimes@freebsd.org