Re: compare memcmp with 0
Hi Theo,
Theo de Raadt wrote on Thu, Jun 19, 2014 at 09:58:01PM -0600:
It could be argued that the bcmp manual page does a poor job
documenting this. It should use the mandoc blink tag.
OK?
It's not perfect yet because when you nest blink tags, it already
switches back to non-blinking mode after you close the inner block,
not after closing the outer block as it should.
But that can be fixed later, in the tree.
Using this requires
export LESS=-R
or something equivalent, depending on your setup, of course.
Maybe we should make that the default.
I guess i need to submit a similar patch to groff, as well.
Yours,
Ingo
Index: share/man/man7/mdoc.7
===
RCS file: /cvs/src/share/man/man7/mdoc.7,v
retrieving revision 1.110
diff -u -p -r1.110 mdoc.7
--- share/man/man7/mdoc.7 30 Mar 2014 23:57:43 - 1.110
+++ share/man/man7/mdoc.7 20 Jun 2014 14:43:02 -
@@ -534,6 +534,7 @@ in the alphabetical
.It Sx \Bq , \Bo , \Bc Ta enclose in square brackets: Bq text
.It Sx \Brq , \Bro , \Brc Ta enclose in curly braces: Brq text
.It Sx \Aq , \Ao , \Ac Ta enclose in angle brackets: Aq text
+.It Sx \Blq , \Blo , \Blc Ta blink: Blq text
.It Sx \Eo , \Ec Ta generic enclosure
.El
.Ss Text production
@@ -1077,6 +1078,16 @@ and
.Pp
See also
.Sx \Bo .
+.Ss \Blc
+Close a
+.Sx \Blo
+block.
+Does not have any tail arguments.
+.Ss \Blo
+Begin a blinking block.
+Does not have any head arguments.
+.Ss \Blq
+Makes its arguments blink.
.Ss \Brc
Close a
.Sx \Bro
@@ -2924,6 +2935,7 @@ end of the line.
.Bl -column MacroX CallableX ParsedX -offset indent
.It Em Macro Ta Em Callable Ta Em Parsed
.It Sx \Aq TaYes TaYes
+.It Sx \Blq TaYes TaYes
.It Sx \Bq TaYes TaYes
.It Sx \Brq TaYes TaYes
.It Sx \D1 Ta\No Ta\Yes
Index: usr.bin/mandoc/mdoc.c
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc.c,v
retrieving revision 1.104
diff -u -p -r1.104 mdoc.c
--- usr.bin/mandoc/mdoc.c 25 Apr 2014 14:10:59 - 1.104
+++ usr.bin/mandoc/mdoc.c 20 Jun 2014 14:43:02 -
@@ -62,7 +62,8 @@ const char *const __mdoc_macronames[MDOC
Lk, Mt, Brq, Bro,
Brc, %C, Es, En,
Dx, %Q, br, sp,
- %U, Ta, ll,
+ %U, Ta, ll, Blo,
+ Blc, Blq,
};
const char *const __mdoc_argnames[MDOC_ARG_MAX] = {
Index: usr.bin/mandoc/mdoc.h
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc.h,v
retrieving revision 1.53
diff -u -p -r1.53 mdoc.h
--- usr.bin/mandoc/mdoc.h 20 Apr 2014 16:44:44 - 1.53
+++ usr.bin/mandoc/mdoc.h 20 Jun 2014 14:43:02 -
@@ -141,6 +141,9 @@ enummdoct {
MDOC__U,
MDOC_Ta,
MDOC_ll,
+ MDOC_Blo,
+ MDOC_Blc,
+ MDOC_Blq,
MDOC_MAX
};
Index: usr.bin/mandoc/mdoc_argv.c
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc_argv.c,v
retrieving revision 1.50
diff -u -p -r1.50 mdoc_argv.c
--- usr.bin/mandoc/mdoc_argv.c 23 Apr 2014 21:06:33 - 1.50
+++ usr.bin/mandoc/mdoc_argv.c 20 Jun 2014 14:43:03 -
@@ -264,6 +264,9 @@ static const struct mdocarg mdocargs[MDO
{ ARGSFL_NONE, NULL }, /* %U */
{ ARGSFL_NONE, NULL }, /* Ta */
{ ARGSFL_NONE, NULL }, /* ll */
+ { ARGSFL_NONE, NULL }, /* Blo */
+ { ARGSFL_DELIM, NULL }, /* Blc */
+ { ARGSFL_DELIM, NULL }, /* Blq */
};
Index: usr.bin/mandoc/mdoc_html.c
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc_html.c,v
retrieving revision 1.73
diff -u -p -r1.73 mdoc_html.c
--- usr.bin/mandoc/mdoc_html.c 23 Apr 2014 16:07:06 - 1.73
+++ usr.bin/mandoc/mdoc_html.c 20 Jun 2014 14:43:03 -
@@ -242,6 +242,9 @@ static const struct htmlmdoc mdocs[MDOC_
{mdoc__x_pre, mdoc__x_post}, /* %U */
{NULL, NULL}, /* Ta */
{mdoc_ll_pre, NULL}, /* ll */
+ {mdoc_quote_pre, mdoc_quote_post}, /* Blo */
+ {NULL, NULL}, /* Blc */
+ {mdoc_quote_pre, mdoc_quote_post}, /* Blq */
};
static const char * const lists[LIST_MAX] = {
@@ -2063,6 +2066,12 @@ mdoc_quote_pre(MDOC_ARGS)
case MDOC_Aq:
print_text(h, \\(la);
break;
+ case MDOC_Blo:
+ /* FALLTHROUGH */
+ case MDOC_Blq:
+ PAIR_CLASS_INIT(tag, blink);
+ print_otag(h, TAG_SPAN, 1, tag);
+ break;
case MDOC_Bro:
/* FALLTHROUGH */
case MDOC_Brq:
@@ -2131,6 +2140,10 @@ mdoc_quote_post(MDOC_ARGS)
/* FALLTHROUGH */
case MDOC_Aq:
Re: compare memcmp with 0
OMFG..
Ingo you just made my morning. I'm laughing so hard.
And I needed the laugh
-Bob
On Fri, Jun 20, 2014 at 04:54:15PM +0200, Ingo Schwarze wrote:
Hi Theo,
Theo de Raadt wrote on Thu, Jun 19, 2014 at 09:58:01PM -0600:
It could be argued that the bcmp manual page does a poor job
documenting this. It should use the mandoc blink tag.
OK?
It's not perfect yet because when you nest blink tags, it already
switches back to non-blinking mode after you close the inner block,
not after closing the outer block as it should.
But that can be fixed later, in the tree.
Using this requires
export LESS=-R
or something equivalent, depending on your setup, of course.
Maybe we should make that the default.
I guess i need to submit a similar patch to groff, as well.
Yours,
Ingo
Index: share/man/man7/mdoc.7
===
RCS file: /cvs/src/share/man/man7/mdoc.7,v
retrieving revision 1.110
diff -u -p -r1.110 mdoc.7
--- share/man/man7/mdoc.7 30 Mar 2014 23:57:43 - 1.110
+++ share/man/man7/mdoc.7 20 Jun 2014 14:43:02 -
@@ -534,6 +534,7 @@ in the alphabetical
.It Sx \Bq , \Bo , \Bc Ta enclose in square brackets: Bq text
.It Sx \Brq , \Bro , \Brc Ta enclose in curly braces: Brq text
.It Sx \Aq , \Ao , \Ac Ta enclose in angle brackets: Aq text
+.It Sx \Blq , \Blo , \Blc Ta blink: Blq text
.It Sx \Eo , \Ec Ta generic enclosure
.El
.Ss Text production
@@ -1077,6 +1078,16 @@ and
.Pp
See also
.Sx \Bo .
+.Ss \Blc
+Close a
+.Sx \Blo
+block.
+Does not have any tail arguments.
+.Ss \Blo
+Begin a blinking block.
+Does not have any head arguments.
+.Ss \Blq
+Makes its arguments blink.
.Ss \Brc
Close a
.Sx \Bro
@@ -2924,6 +2935,7 @@ end of the line.
.Bl -column MacroX CallableX ParsedX -offset indent
.It Em Macro Ta Em Callable Ta Em Parsed
.It Sx \Aq TaYes TaYes
+.It Sx \Blq TaYes TaYes
.It Sx \Bq TaYes TaYes
.It Sx \Brq TaYes TaYes
.It Sx \D1 Ta\No Ta\Yes
Index: usr.bin/mandoc/mdoc.c
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc.c,v
retrieving revision 1.104
diff -u -p -r1.104 mdoc.c
--- usr.bin/mandoc/mdoc.c 25 Apr 2014 14:10:59 - 1.104
+++ usr.bin/mandoc/mdoc.c 20 Jun 2014 14:43:02 -
@@ -62,7 +62,8 @@ const char *const __mdoc_macronames[MDOC
Lk, Mt, Brq, Bro,
Brc, %C, Es, En,
Dx, %Q, br, sp,
- %U, Ta, ll,
+ %U, Ta, ll, Blo,
+ Blc, Blq,
};
constchar *const __mdoc_argnames[MDOC_ARG_MAX] = {
Index: usr.bin/mandoc/mdoc.h
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc.h,v
retrieving revision 1.53
diff -u -p -r1.53 mdoc.h
--- usr.bin/mandoc/mdoc.h 20 Apr 2014 16:44:44 - 1.53
+++ usr.bin/mandoc/mdoc.h 20 Jun 2014 14:43:02 -
@@ -141,6 +141,9 @@ enum mdoct {
MDOC__U,
MDOC_Ta,
MDOC_ll,
+ MDOC_Blo,
+ MDOC_Blc,
+ MDOC_Blq,
MDOC_MAX
};
Index: usr.bin/mandoc/mdoc_argv.c
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc_argv.c,v
retrieving revision 1.50
diff -u -p -r1.50 mdoc_argv.c
--- usr.bin/mandoc/mdoc_argv.c23 Apr 2014 21:06:33 - 1.50
+++ usr.bin/mandoc/mdoc_argv.c20 Jun 2014 14:43:03 -
@@ -264,6 +264,9 @@ staticconst struct mdocarg mdocargs[MDO
{ ARGSFL_NONE, NULL }, /* %U */
{ ARGSFL_NONE, NULL }, /* Ta */
{ ARGSFL_NONE, NULL }, /* ll */
+ { ARGSFL_NONE, NULL }, /* Blo */
+ { ARGSFL_DELIM, NULL }, /* Blc */
+ { ARGSFL_DELIM, NULL }, /* Blq */
};
Index: usr.bin/mandoc/mdoc_html.c
===
RCS file: /cvs/src/usr.bin/mandoc/mdoc_html.c,v
retrieving revision 1.73
diff -u -p -r1.73 mdoc_html.c
--- usr.bin/mandoc/mdoc_html.c23 Apr 2014 16:07:06 - 1.73
+++ usr.bin/mandoc/mdoc_html.c20 Jun 2014 14:43:03 -
@@ -242,6 +242,9 @@ staticconst struct htmlmdoc mdocs[MDOC_
{mdoc__x_pre, mdoc__x_post}, /* %U */
{NULL, NULL}, /* Ta */
{mdoc_ll_pre, NULL}, /* ll */
+ {mdoc_quote_pre, mdoc_quote_post}, /* Blo */
+ {NULL, NULL}, /* Blc */
+ {mdoc_quote_pre, mdoc_quote_post}, /* Blq */
};
static const char * const lists[LIST_MAX] = {
@@ -2063,6 +2066,12 @@ mdoc_quote_pre(MDOC_ARGS)
case MDOC_Aq:
print_text(h, \\(la);
break;
+ case MDOC_Blo:
+ /* FALLTHROUGH */
+ case MDOC_Blq:
+ PAIR_CLASS_INIT(tag, blink);
+
compare memcmp with 0
Always explicitly compare memcmp with 0. I find this adds clarity.
Index: s3_clnt.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/s3_clnt.c,v
retrieving revision 1.71
diff -u -p -r1.71 s3_clnt.c
--- s3_clnt.c 19 Jun 2014 21:29:51 - 1.71
+++ s3_clnt.c 19 Jun 2014 21:33:47 -
@@ -886,7 +886,7 @@ ssl3_get_server_hello(SSL *s)
timingsafe_memcmp(p, s-session-session_id, j) == 0) {
if (s-sid_ctx_length != s-session-sid_ctx_length ||
timingsafe_memcmp(s-session-sid_ctx,
- s-sid_ctx, s-sid_ctx_length)) {
+ s-sid_ctx, s-sid_ctx_length) != 0) {
/* actually a client application bug */
al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,
Index: ssl_sess.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/ssl_sess.c,v
retrieving revision 1.33
diff -u -p -r1.33 ssl_sess.c
--- ssl_sess.c 19 Jun 2014 21:29:51 - 1.33
+++ ssl_sess.c 19 Jun 2014 21:33:47 -
@@ -498,7 +498,7 @@ ssl_get_prev_session(SSL *s, unsigned ch
/* Now ret is non-NULL and we own one of its reference counts. */
if (ret-sid_ctx_length != s-sid_ctx_length
- || timingsafe_memcmp(ret-sid_ctx, s-sid_ctx,
ret-sid_ctx_length)) {
+ || timingsafe_memcmp(ret-sid_ctx, s-sid_ctx, ret-sid_ctx_length)
!= 0) {
/* We have the session requested by the client, but we don't
* want to use it in this context. */
goto err; /* treat like cache miss */
Index: t1_reneg.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/t1_reneg.c,v
retrieving revision 1.7
diff -u -p -r1.7 t1_reneg.c
--- t1_reneg.c 19 Jun 2014 21:29:51 - 1.7
+++ t1_reneg.c 19 Jun 2014 21:33:47 -
@@ -173,7 +173,7 @@ ssl_parse_clienthello_renegotiate_ext(SS
}
if (timingsafe_memcmp(d, s-s3-previous_client_finished,
- s-s3-previous_client_finished_len)) {
+ s-s3-previous_client_finished_len) != 0) {
SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,
SSL_R_RENEGOTIATION_MISMATCH);
*al = SSL_AD_HANDSHAKE_FAILURE;
@@ -260,7 +260,7 @@ ssl_parse_serverhello_renegotiate_ext(SS
}
if (timingsafe_memcmp(d, s-s3-previous_client_finished,
- s-s3-previous_client_finished_len)) {
+ s-s3-previous_client_finished_len) != 0) {
SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_RENEGOTIATE_EXT,
SSL_R_RENEGOTIATION_MISMATCH);
*al = SSL_AD_HANDSHAKE_FAILURE;
Re: compare memcmp with 0
On 20 Jun 2014, at 7:35, Ted Unangst t...@tedunangst.com wrote:
Always explicitly compare memcmp with 0. I find this adds clarity.
i agree.
ok by me if that has any value in this part of the tree.
Index: s3_clnt.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/s3_clnt.c,v
retrieving revision 1.71
diff -u -p -r1.71 s3_clnt.c
--- s3_clnt.c 19 Jun 2014 21:29:51 - 1.71
+++ s3_clnt.c 19 Jun 2014 21:33:47 -
@@ -886,7 +886,7 @@ ssl3_get_server_hello(SSL *s)
timingsafe_memcmp(p, s-session-session_id, j) == 0) {
if (s-sid_ctx_length != s-session-sid_ctx_length ||
timingsafe_memcmp(s-session-sid_ctx,
- s-sid_ctx, s-sid_ctx_length)) {
+ s-sid_ctx, s-sid_ctx_length) != 0) {
/* actually a client application bug */
al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,
Index: ssl_sess.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/ssl_sess.c,v
retrieving revision 1.33
diff -u -p -r1.33 ssl_sess.c
--- ssl_sess.c19 Jun 2014 21:29:51 - 1.33
+++ ssl_sess.c19 Jun 2014 21:33:47 -
@@ -498,7 +498,7 @@ ssl_get_prev_session(SSL *s, unsigned ch
/* Now ret is non-NULL and we own one of its reference counts. */
if (ret-sid_ctx_length != s-sid_ctx_length
- || timingsafe_memcmp(ret-sid_ctx, s-sid_ctx,
ret-sid_ctx_length)) {
+ || timingsafe_memcmp(ret-sid_ctx, s-sid_ctx, ret-sid_ctx_length)
!= 0) {
/* We have the session requested by the client, but we don't
* want to use it in this context. */
goto err; /* treat like cache miss */
Index: t1_reneg.c
===
RCS file: /cvs/src/lib/libssl/src/ssl/t1_reneg.c,v
retrieving revision 1.7
diff -u -p -r1.7 t1_reneg.c
--- t1_reneg.c19 Jun 2014 21:29:51 - 1.7
+++ t1_reneg.c19 Jun 2014 21:33:47 -
@@ -173,7 +173,7 @@ ssl_parse_clienthello_renegotiate_ext(SS
}
if (timingsafe_memcmp(d, s-s3-previous_client_finished,
- s-s3-previous_client_finished_len)) {
+ s-s3-previous_client_finished_len) != 0) {
SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,
SSL_R_RENEGOTIATION_MISMATCH);
*al = SSL_AD_HANDSHAKE_FAILURE;
@@ -260,7 +260,7 @@ ssl_parse_serverhello_renegotiate_ext(SS
}
if (timingsafe_memcmp(d, s-s3-previous_client_finished,
- s-s3-previous_client_finished_len)) {
+ s-s3-previous_client_finished_len) != 0) {
SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_RENEGOTIATE_EXT,
SSL_R_RENEGOTIATION_MISMATCH);
*al = SSL_AD_HANDSHAKE_FAILURE;
Re: compare memcmp with 0
On Thu, 19 Jun 2014, Ted Unangst wrote: Always explicitly compare memcmp with 0. I find this adds clarity. If you don't care which way a different comparison points, then why not use bcmp?
Re: compare memcmp with 0
Always explicitly compare memcmp with 0. I find this adds clarity. If you don't care which way a different comparison points, then why not use bcmp? Because knowledge of the difference in is scarce. Someone will screw it up. It could be argued that the bcmp manual page does a poor job documenting this. It should use the mandoc blink tag.
Re: compare memcmp with 0
On Fri, Jun 20, 2014 at 13:53, Damien Miller wrote: On Thu, 19 Jun 2014, Ted Unangst wrote: Always explicitly compare memcmp with 0. I find this adds clarity. If you don't care which way a different comparison points, then why not use bcmp? There are a couple points here. 1. we have a timingsafe_memcmp function that is a drop in replacement for memcmp. maybe a little slower, but behaviorly compatible. 2. i would like for more people to use constant time functions, even when there is not a proven side channel. currently the state of the art appears to be people publish papers exploiting timing, and then people fix bugs. comparisions are only a subset of side channels, but i think we can get ahead of the curve by using such functions even without proof of attack. 3. bcmp has a slightly difference interface, making it not always a drop in replacement. this means people have to think about their conversions, which i think interferes with point 2. Basically, I would like people to experiment with ideas such as compiling their code with -Dmemcmp=timingsafe_memcmp. Doing the same with timingsafe_bcmp would be a disaster, however, for software that depends on the ordering. If we use timingsafe_bcmp widely (safe as that may be), it's very hard to convey the idea that there are circumstances when it is not safe. Using timingsafe_memcmp raises its awareness and will make it other developers' default choice.
Re: compare memcmp with 0
If we use timingsafe_bcmp widely (safe as that may be), it's very hard to convey the idea that there are circumstances when it is not safe. Using timingsafe_memcmp raises its awareness and will make it other developers' default choice. Exactly. It is easier to develop a pattern/meme when the choice is simple to remember. If the decision tree is too complex, people simply walk away. The performance cost is totally irrelevant.
Re: compare memcmp with 0
On Thu, 19 Jun 2014 21:58:01 -0600 (MDT), Theo de Raadt wrote: It should use the mandoc blink tag. Look at what beck@ started with the libressl web page! 8-) *** NOTE *** Please DO NOT CC me. I am subscribed to the list. Mail to the sender address that does not originate at the list server is tarpitted. The reply-to: address is provided for those who feel compelled to reply off list. Thankyou. Rod/ --- This life is not the real thing. It is not even in Beta. If it was, then OpenBSD would already have a man page for it.
