redo signfify options

2014-01-01 Thread Ted Unangst
I misinterpreted Theo's comments about the option letters before.

Revert to lowercase for most options, and change the verb option into
three distinct uppercase options, -G -S and -V.

Sorry Marc...

Index: signify.1
===
RCS file: /cvs/src/usr.bin/signify/signify.1,v
retrieving revision 1.5
diff -u -p -r1.5 signify.1
--- signify.1   31 Dec 2013 18:18:36 -  1.5
+++ signify.1   1 Jan 2014 15:00:28 -
@@ -22,47 +22,56 @@
 .Nd cryptographically sign and verify files
 .Sh SYNOPSIS
 .Nm signify
-.Op Fl N
-.Op Fl I Ar input
-.Op Fl O Ar output
-.Op Fl P Ar pubkey
-.Op Fl S Ar seckey
-.Fl V Ar generate | sign | verify
+.Op Fl n
+.Op Fl i Ar input
+.Op Fl o Ar output
+.Op Fl p Ar pubkey
+.Op Fl s Ar seckey
+.Fl G
+.Fl S
+.Fl V
 .Sh DESCRIPTION
 The
 .Nm
 utility creates and verifies cryptographic signatures.
 The mode of operation is selected by the
+.Fl G ,
+.Fl S ,
+or
 .Fl V
 option.
 .Pp
 The options are as follows:
 .Bl -tag -width Ds
-.It Fl I Ar input
+.It Fl G
+Generate a new keypair.
+.It Fl S
+Sign the input file.
+.It Fl V
+Verify the input file and signature match.
+.It Fl i Ar input
 Input file to sign or verify.
-.It Fl N
+.It Fl n
 Do not ask for a passphrase during key generation.
 Otherwise,
 .Nm
 will prompt the user for a passphrase on the terminal.
-.It Fl O Ar output
+.It Fl o Ar output
 The signature file to create or verify.
 The default is
 .Ar input Ns .sig .
-.It Fl P Ar pubkey
+.It Fl p Ar pubkey
 Public key produced by
 .Ar generate ,
 and used by
 .Ar verify
 to check a signature.
-.It Fl S Ar seckey
+.It Fl s Ar seckey
 Secret (private) key produced by
 .Ar generate ,
 and used by
 .Ar sign
 to sign a message.
-.It Fl V Ar generate | sign | verify
-Select the desired operation.
 .El
 .Pp
 The key and signature files created by
@@ -87,13 +96,13 @@ The message file is too large.
 .El
 .Sh EXAMPLES
 Create a new keypair:
-.Dl $ signify -P newkey.pub -S newkey.sec -V generate
+.Dl $ signify -p newkey.pub -s newkey.sec -G
 .Pp
 Sign a file, specifying a signature name:
-.Dl $ signify -S key.sec -I message.txt -O msg.sig -V sign
+.Dl $ signify -s key.sec -i message.txt -o msg.sig -S
 .Pp
 Verify a signature, using the default signature name:
-.Dl $ signify -P key.pub -I generalsorders.txt -V verify
+.Dl $ signify -p key.pub -i generalsorders.txt -V
 .Sh SEE ALSO
 .Xr cmp 1 ,
 .Xr sha256 1 ,
Index: signify.c
===
RCS file: /cvs/src/usr.bin/signify/signify.c,v
retrieving revision 1.5
diff -u -p -r1.5 signify.c
--- signify.c   31 Dec 2013 17:33:17 -  1.5
+++ signify.c   1 Jan 2014 15:00:28 -
@@ -64,8 +64,8 @@ extern char *__progname;
 static void
 usage(void)
 {
-   fprintf(stderr, usage: %s [-N] [-I input] [-O output] [-P pubkey] [-S 
seckey] 
-   -V generate | sign | verify\n, __progname);
+   fprintf(stderr, usage: %s [-n] [-i input] [-o output] [-p pubkey] [-s 
seckey] 
+   -G | -S | -V\n, __progname);
exit(1);
 }
 
@@ -316,41 +316,59 @@ verify(const char *pubkeyfile, const cha
 int
 main(int argc, char **argv)
 {
-   const char *verb = NULL;
const char *pubkeyfile = NULL, *seckeyfile = NULL, *inputfile = NULL,
*sigfile = NULL;
char sigfilebuf[1024];
int ch, rounds;
+   enum {
+   NONE,
+   GENERATE,
+   SIGN,
+   VERIFY
+   } verb = NONE;
+
 
rounds = 42;
 
-   while ((ch = getopt(argc, argv, I:NO:P:S:V:)) != -1) {
+   while ((ch = getopt(argc, argv, GSVi:no:p:s:)) != -1) {
switch (ch) {
-   case 'I':
+   case 'G':
+   if (verb)
+   usage();
+   verb = GENERATE;
+   break;
+   case 'S':
+   if (verb)
+   usage();
+   verb = SIGN;
+   break;
+   case 'V':
+   if (verb)
+   usage();
+   verb = VERIFY;
+   break;
+   case 'i':
inputfile = optarg;
break;
-   case 'N':
+   case 'n':
rounds = 0;
break;
-   case 'O':
+   case 'o':
sigfile = optarg;
break;
-   case 'P':
+   case 'p':
pubkeyfile = optarg;
break;
-   case 'S':
+   case 's':
seckeyfile = optarg;
break;
-   case 'V':
-   verb = optarg;
-   break;
default:
usage();

Re: redo signfify options

2014-01-01 Thread Marc Espie
Something like this for the program itself:

Index: signify.c
===
RCS file: /build/data/openbsd/cvs/src/usr.bin/signify/signify.c,v
retrieving revision 1.5
diff -u -p -r1.5 signify.c
--- signify.c   31 Dec 2013 17:33:17 -  1.5
+++ signify.c   1 Jan 2014 15:50:11 -
@@ -64,8 +64,8 @@ extern char *__progname;
 static void
 usage(void)
 {
-   fprintf(stderr, usage: %s [-N] [-I input] [-O output] [-P pubkey] [-S 
seckey] 
-   -V generate | sign | verify\n, __progname);
+   fprintf(stderr, usage: %s -G | -S | -V [-n] [-o output] 
+   [-p pubkey] [-s seckey] [file...]\n, __progname);
exit(1);
 }
 
@@ -316,64 +316,91 @@ verify(const char *pubkeyfile, const cha
 int
 main(int argc, char **argv)
 {
-   const char *verb = NULL;
-   const char *pubkeyfile = NULL, *seckeyfile = NULL, *inputfile = NULL,
-   *sigfile = NULL;
-   char sigfilebuf[1024];
-   int ch, rounds;
+   const char *pubkeyfile = NULL, *seckeyfile = NULL, 
+   *sigfilefmt = %s.sig;
+   int ch, rounds, needfmt, i;
+   enum {
+   NONE,
+   GENERATE,
+   SIGN,
+   VERIFY
+   } verb = NONE;
+
 
rounds = 42;
 
-   while ((ch = getopt(argc, argv, I:NO:P:S:V:)) != -1) {
+   while ((ch = getopt(argc, argv, GSVi:no:p:s:)) != -1) {
switch (ch) {
-   case 'I':
-   inputfile = optarg;
+   case 'G':
+   if (verb)
+   usage();
+   verb = GENERATE;
+   break;
+   case 'S':
+   if (verb)
+   usage();
+   verb = SIGN;
+   break;
+   case 'V':
+   if (verb)
+   usage();
+   verb = VERIFY;
break;
-   case 'N':
+   case 'n':
rounds = 0;
break;
-   case 'O':
-   sigfile = optarg;
+   case 'o':
+   sigfilefmt = optarg;
break;
-   case 'P':
+   case 'p':
pubkeyfile = optarg;
break;
-   case 'S':
+   case 's':
seckeyfile = optarg;
break;
-   case 'V':
-   verb = optarg;
-   break;
default:
usage();
break;
}
}
argc -= optind;
-   if (argc != 0 || verb == NULL)
-   usage();
-
-   if (inputfile  !sigfile) {
-   if (snprintf(sigfilebuf, sizeof(sigfilebuf), %s.sig,
-   inputfile) = sizeof(sigfilebuf))
-   errx(1, path too long);
-   sigfile = sigfilebuf;
-   }
 
-   if (streq(verb, generate)) {
-   if (!pubkeyfile || !seckeyfile)
+   if (verb == GENERATE) {
+   if (argc != 0 || !pubkeyfile || !seckeyfile)
usage();
generate(pubkeyfile, seckeyfile, rounds);
-   } else if (streq(verb, sign)) {
-   if (!seckeyfile || !inputfile)
-   usage();
-   sign(seckeyfile, inputfile, sigfile);
-   } else if (streq(verb, verify)) {
-   if (!pubkeyfile || !inputfile)
-   usage();
-   verify(pubkeyfile, inputfile, sigfile);
-   } else {
+   } else if (verb == NONE || argc == 0) {
usage();
+   }
+
+   needfmt = strstr(sigfilefmt, %s) != NULL;
+
+   if (!needfmt  argc != 1) {
+   errx(1, -o file for several inputs);
+   }
+
+   for (i = 0; i != argc; i++) {
+   char sigfilebuf[1024];
+   const char *inputfile = argv[i];
+   const char *sigfile = NULL;
+
+   if (needfmt) {
+   if (snprintf(sigfilebuf, sizeof(sigfilebuf),
+   sigfilefmt, inputfile) = sizeof(sigfilebuf))
+   errx(1, path too long);
+   sigfile = sigfilebuf;
+   } else {
+   sigfile = sigfilefmt;
+   }
+   if (verb == SIGN) {
+   if (!seckeyfile)
+   usage();
+   sign(seckeyfile, inputfile, sigfile);
+   } else if (verb == VERIFY) {
+   if (!pubkeyfile || !inputfile)
+   usage();
+   verify(pubkeyfile, inputfile, sigfile);
+   }
}
return 0;
 }



Re: redo signfify options

2014-01-01 Thread Ted Unangst
On Wed, Jan 01, 2014 at 16:25, Marc Espie wrote:
 Now that you're fixing it, I think you've still got it wrong.
 
 Instead of
 signify -V -p pubkey -i input
 you want to be able to do
 signify -V -p pubkey  input ...
 
 e.g., at this point, input would not need to be an option, so that you
 can verify several files in one go.

we can do that too if we need batch verification, but I left it out
of this commit.



Re: redo signfify options

2014-01-01 Thread Marc Espie
On Wed, Jan 01, 2014 at 12:53:09PM -0500, Ted Unangst wrote:
 On Wed, Jan 01, 2014 at 16:25, Marc Espie wrote:
  Now that you're fixing it, I think you've still got it wrong.
  
  Instead of
  signify -V -p pubkey -i input
  you want to be able to do
  signify -V -p pubkey  input ...
  
  e.g., at this point, input would not need to be an option, so that you
  can verify several files in one go.
 
 we can do that too if we need batch verification, but I left it out
 of this commit.

Well, at least make input not an option but an actual parameter.

Otherwise, you have a command with no parms.

And considering the command name, it makes little sense. As the name
says, that command is used to sign or verify a file. The *object* of that
command is the file. It's not an option !



Important bgpd fix

2014-01-01 Thread Claudio Jeker
There is a somewhat critical bug in bgpd which got hit by local friends
a few weeks ago. The problem is that on session with the graceful restart
capability stale routes are not properly flushed. This can lead to bad
FIB entries and black holes. This happens when a router does not reconnect
before the timeout fires.

As a quick fix disabling the graceful reload capability with
announce restart no will skip the probelmatic code and no stale routes
will remain. The session must be cleared first before the change becomes
active.

The proper fix is attached. in short this removes a check for the
CAPA_GR_FORWARD flag in the timeout and IMSG_SESSION_RESTARTED handler.
CAPA_GR_RESTARTING is indicating that bgpd is currently doing a graceful
restart for this neighbor and it is set for any session that must issue
a flush of stale routes (either after a successful restart or because of
hitting a timeout or change of configuration). So whenever the SE issues
an IMSG_SESSION_FLUSH or IMSG_SESSION_RESTARTED message the
CAPA_GR_RESTARTING flag needs to be cleared.

Please test and/or verify that my logic is now correct.
-- 
:wq Claudio

PS: I'm away for a bit more then a week so I will not be able to answer
any questions anytime soon.

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.326
diff -u -p -r1.326 rde.c
--- rde.c   13 Nov 2013 20:41:01 -  1.326
+++ rde.c   1 Jan 2014 02:30:04 -
@@ -3282,6 +3282,7 @@ peer_stale(u_int32_t id, u_int8_t aid)
return;
}
 
+   /* flush the now even staler routes out */
if (peer-staletime[aid])
peer_flush(peer, aid);
peer-staletime[aid] = now = time(NULL);
@@ -3317,7 +3318,14 @@ peer_recv_eor(struct rde_peer *peer, u_i
 {
peer-prefix_rcvd_eor++;
 
-   /* First notify SE to remove possible race with the timeout. */
+   /*
+* First notify SE to avert a possible race with the restart timeout.
+* If the timeout fires before this imsg is processed by the SE it will
+* result in the same operation since the timeout issues a FLUSH which
+* does the same as the RESTARTED action (flushing stale routes).
+* The logic in the SE is so that only one of FLUSH or RESTARTED will
+* be sent back to the RDE and so peer_flush is only called once.
+*/
if (imsg_compose(ibuf_se, IMSG_SESSION_RESTARTED, peer-conf.id,
0, -1, aid, sizeof(aid)) == -1)
fatal(imsg_compose error);
Index: session.c
===
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.333
diff -u -p -r1.333 session.c
--- session.c   13 Nov 2013 20:41:01 -  1.333
+++ session.c   1 Jan 2014 02:36:31 -
@@ -1729,12 +1729,11 @@ session_graceful_stop(struct peer *p)
 
for (i = 0; i  AID_MAX; i++) {
/*
-* Only flush if the peer is restarting and the peer indicated
-* it hold the forwarding state. In all other cases the
-* session was already flushed when the session came up.
+* Only flush if the peer is restarting and the timeout fired.
+* In all other cases the session was already flushed when the
+* session went down or when the new open message was parsed.
 */
-   if (p-capa.neg.grestart.flags[i]  CAPA_GR_RESTARTING 
-   p-capa.neg.grestart.flags[i]  CAPA_GR_FORWARD) {
+   if (p-capa.neg.grestart.flags[i]  CAPA_GR_RESTARTING) {
log_peer_warnx(p-conf, graceful restart of %s, 
time-out, flushing, aid2str(i));
if (imsg_compose(ibuf_rde, IMSG_SESSION_FLUSH,
@@ -2520,12 +2519,16 @@ capa_neg_calc(struct peer *p)
 */
 
for (i = 0; i  AID_MAX; i++) {
+   int8_t  negflags;
+
/* disable GR if the AFI/SAFI is not present */
if (p-capa.peer.grestart.flags[i]  CAPA_GR_PRESENT 
p-capa.neg.mp[i] == 0)
p-capa.peer.grestart.flags[i] = 0; /* disable */
/* look at current GR state and decide what to do */
-   if (p-capa.neg.grestart.flags[i]  CAPA_GR_RESTARTING) {
+   negflags = p-capa.neg.grestart.flags[i];
+   p-capa.neg.grestart.flags[i] = p-capa.peer.grestart.flags[i];
+   if (negflags  CAPA_GR_RESTARTING) {
if (!(p-capa.peer.grestart.flags[i] 
CAPA_GR_FORWARD)) {
if (imsg_compose(ibuf_rde, IMSG_SESSION_FLUSH,
@@ -2533,12 +2536,10 @@ capa_neg_calc(struct peer *p)
return (-1);
log_peer_warnx(p-conf, graceful restart of