sshd(8) should also log which account is trying a revoked key (was Re: sshd - also log account trying a revoked key)
(Re-phrased for clarity.)
Please also log the user accounts which attempt to use revoked keys.
It would much more easily identify the problem account in question by
listing it on the same line as the offending revoked key in the log,
instead of in a separate log entry as it the case now.
The log entry could be changed to look something
like this:
Oct 21 18:14:14 server sshd[73078]: error: User foo authentication key
RSA SHA256:CMHiAcoUM2tSS0ENOFvMLBvjhwhaVsmQVvhuvhPxVy4 revoked by file
/etc/ssh/ssh_revoked_keys
Oct 21 18:14:28 server sshd[73078]: Connection closed by
authenticating user foo 198.51.100.95 port 55644 [preauth]
Currently only the key is logged logged and not the user:
Oct 21 18:07:00 server sshd[79743]: error: Authentication key RSA
SHA256:CMHiAcoUM2tSS0ENOFvMLBvjhwhaVsmQVvhuvhPxVy4 revoked by file
/etc/ssh/ssh_revoked_keys
Oct 21 18:07:06 server sshd[79743]: Connection closed by
authenticating user foo 198.51.100.95 port 55634 [preauth]
So I would propose consideration of something approximately like the
changes below.
/Lars
Index: usr.bin/ssh//auth.c
===
RCS file: /cvs/src/usr.bin/ssh/auth.c,v
retrieving revision 1.133
diff -u -p -u -r1.133 auth.c
--- usr.bin/ssh//auth.c 12 Sep 2018 01:19:12 - 1.133
+++ usr.bin/ssh//auth.c 21 Oct 2018 15:27:04 -
@@ -507,7 +507,7 @@ getpwnamallow(const char *user)
/* Returns 1 if key is revoked by revoked_keys_file, 0 otherwise */
int
-auth_key_is_revoked(struct sshkey *key)
+auth_key_is_revoked(struct passwd *pw, struct sshkey *key)
{
char *fp = NULL;
int r;
@@ -526,8 +526,9 @@ auth_key_is_revoked(struct sshkey *key)
case 0:
break; /* not revoked */
case SSH_ERR_KEY_REVOKED:
- error("Authentication key %s %s revoked by file %s",
- sshkey_type(key), fp, options.revoked_keys_file);
+ error("User %s authentication key %s %s revoked by file %s",
+ pw->pw_name, sshkey_type(key), fp,
+ options.revoked_keys_file);
goto out;
default:
error("Error checking authentication key %s %s in "
Index: usr.bin/ssh//auth.h
===
RCS file: /cvs/src/usr.bin/ssh/auth.h,v
retrieving revision 1.96
diff -u -p -u -r1.96 auth.h
--- usr.bin/ssh//auth.h 10 Apr 2018 00:10:49 - 1.96
+++ usr.bin/ssh//auth.h 21 Oct 2018 15:27:04 -
@@ -175,7 +175,7 @@ char*authorized_principals_file(struct
FILE *auth_openkeyfile(const char *, struct passwd *, int);
FILE *auth_openprincipals(const char *, struct passwd *, int);
-int auth_key_is_revoked(struct sshkey *);
+int auth_key_is_revoked(struct passwd *, struct sshkey *);
const char *auth_get_canonical_hostname(struct ssh *, int);
Index: usr.bin/ssh//auth2-hostbased.c
===
RCS file: /cvs/src/usr.bin/ssh/auth2-hostbased.c,v
retrieving revision 1.38
diff -u -p -u -r1.38 auth2-hostbased.c
--- usr.bin/ssh//auth2-hostbased.c 20 Sep 2018 03:28:06 - 1.38
+++ usr.bin/ssh//auth2-hostbased.c 21 Oct 2018 15:27:04 -
@@ -175,7 +175,7 @@ hostbased_key_allowed(struct passwd *pw,
int len;
char *fp;
- if (auth_key_is_revoked(key))
+ if (auth_key_is_revoked(pw, key))
return 0;
resolvedname = auth_get_canonical_hostname(ssh, options.use_dns);
Index: usr.bin/ssh//auth2-pubkey.c
===
RCS file: /cvs/src/usr.bin/ssh/auth2-pubkey.c,v
retrieving revision 1.86
diff -u -p -u -r1.86 auth2-pubkey.c
--- usr.bin/ssh//auth2-pubkey.c 20 Sep 2018 03:28:06 - 1.86
+++ usr.bin/ssh//auth2-pubkey.c 21 Oct 2018 15:27:04 -
@@ -1001,10 +1001,10 @@ user_key_allowed(struct ssh *ssh, struct
if (authoptsp != NULL)
*authoptsp = NULL;
- if (auth_key_is_revoked(key))
+ if (auth_key_is_revoked(pw, key))
return 0;
if (sshkey_is_cert(key) &&
- auth_key_is_revoked(key->cert->signature_key))
+ auth_key_is_revoked(pw, key->cert->signature_key))
return 0;
if ((success = user_cert_trusted_ca(ssh, pw, key, &opts)) != 0)
Re: change reboot behaviour in vmd
On Thu, Dec 06, 2018 at 10:33:24AM +0100, Claudio Jeker wrote:
> So doing autoinstall with -B net is great but one thing I was missing is
> changing the reboot behaviour of vmd to exit at a guest reboot.
> I came up with this minimal diff that does the trick for me. Now maybe it
> would be better to have a proper flag for this instead of overloading
> vmc_bootdevice with it.
>
> What is the preferred way of doing this?
> --
> :wq Claudio
I'm ok with this.
reyk@, what are your thoughts?
+--+
Carlos
>
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.107
> diff -u -p -r1.107 vmd.c
> --- vmd.c 4 Dec 2018 08:15:09 - 1.107
> +++ vmd.c 4 Dec 2018 09:11:51 -
> @@ -452,7 +452,8 @@ vmd_dispatch_vmm(int fd, struct privsep_
> __func__, vmr.vmr_id);
> break;
> }
> - if (vmr.vmr_result != EAGAIN) {
> + if (vmr.vmr_result != EAGAIN ||
> + vm->vm_params.vmc_bootdevice) {
> if (vm->vm_from_config)
> vm_stop(vm, 0, __func__);
> else
>
Re: sed(1) make -i behave a little nicer
On Fri, Dec 07, 2018 at 08:41:21AM +0100, Martijn van Duren wrote:
> I ran into a few minor nuisances with sed's -i mode, which are mostly
> compatible with gnu sed, but I reckon we should address.
>
> The problem is sed works by writing the output to a second file in the
> same directory as the original and after completion renaming the file
> to the original. This has two disadvantages:
> 1) The inode number changes, resulting in loss of carefully crafted
>hardlinks.
> 2) We require to have write permission in the same directory as the
>original file, even if we don't want to have a backup file.
>
> Diff below tries to solve this by doing the following.
> Copy the file to the backup location (/tmp/sedXX if no
> extension) and use the backup as the infile and the original as the
> outfile.
>
> Furthermore I changed the lstat to fstat, so we can edit symlinks (gsed
> supports symlinks by replacing the symlink by a new real file, which is
> also fun), and I extended the warning messages in process to show the
> backup file if we crash during operation, so people will always know
> where to recover the file in case of disaster.
>
> Because process also error(FATAL, ...)s during process and we always
> have a backup file I don't think the warning in sed.1 is worth keeping.
>
> The only downside to this new approach (that I can think of) is that we
> now temporarily have a file that is in an inconsistent state, but that's
> not much different to writing a file with any other editor.
>
> $ echo test > /usr/obj/test && dd if=/dev/zero of=/usr/obj/bloat
> /usr/obj: write failed, file system is full
> dd: /usr/obj/bloat: No space left on device
> 614113+0 records in
> 614112+0 records out
> 314425344 bytes transferred in 2.206 secs (142470196 bytes/sec)
> $ ./obj/sed -i -f /tmp/test.sed /usr/obj/test
>
>
> /usr/obj: write failed, file system is full
> sed: /usr/obj/test: No space left on device (backup at /tmp/sedgRPSLImG9N)
> $ cat /tmp/test.sed
> s/test/aaa/
> $ cat /tmp/sedgRPSLImG9N
> test
> $ ls -i /tmp/test
> 104 /tmp/test
> $ sed -i s/test/foo/ /tmp/test
> $ ls -i /tmp/test
> 104 /tmp/test
> $ doas touch /etc/test
> $ doas chown martijn /etc/test
> $ echo test > /etc/test
> $ sed -i s/test/foo/ /etc/test
> $ cat /etc/test
> foo
>
> The diff does change quite a few mechanics, so some scrutiny is welcome.
>
> martijn@
>
> Index: main.c
> ===
> RCS file: /cvs/src/usr.bin/sed/main.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 main.c
> --- main.c6 Dec 2018 20:16:04 - 1.39
> +++ main.c7 Dec 2018 07:31:54 -
> @@ -35,6 +35,7 @@
>
> #include
> #include
> +#include
> #include
>
> #include
> @@ -94,13 +95,13 @@ static int rval; /* Exit status */
> */
> const char *fname; /* File name. */
> const char *outfname;/* Output file name */
> -static char oldfname[PATH_MAX]; /* Old file name (for in-place editing)
> */
> -static char tmpfname[PATH_MAX]; /* Temporary file name (for in-place
> editing) */
> +char oldfname[PATH_MAX]; /* Old file name (for in-place editing) */
> char *inplace; /* Inplace edit file extension */
> u_long linenum;
>
> static void add_compunit(enum e_cut, char *);
> static void add_file(char *);
> +int copy(FILE *, FILE *);
> static int next_files_have_lines(void);
>
> int termwidth;
> @@ -310,26 +311,46 @@ again:
> return (NULL);
> }
>
> +int
> +copy(FILE *src, FILE *dst)
> +{
> + unsigned char buf[MAXBSIZE];
> + size_t r, w, tw;
> +
> + while(1) {
> + if ((r = fread(buf, sizeof(*buf), sizeof(buf), src)) == 0) {
> + if (feof(src)) {
> + if (fflush(dst) == EOF)
> + return 0;
> + return 1;
> + }
> + if (errno != EINTR)
> + return 0;
> + continue;
> + }
> + tw = 0;
> + while(tw < r) {
> + w = fwrite(buf + tw, sizeof(*buf), r - tw, dst);
> + if (w == 0) {
> + if (errno != EINTR)
> + return 0;
> + continue;
> + }
> + tw += w;
> + }
> + }
> +}
> +
> void
> finish_file(void)
> {
> if (infile != NULL) {
> fclose(infile);
> - if (*oldfname != '\0') {
> - if (rename(fname, oldfname) != 0) {
> - warning("rename()");
> - unlink(tmpfname);
> - exit(1);
> - }
> + if (inplace != NULL) {
> +
Small usr.bin/libtool cleanup
We don't support vax any more. ok?
Index: libtool
===
--- libtool.orig
+++ libtool
@@ -52,24 +52,14 @@ sub new
ltdir => $ltdir,
version => $version,
objdir => $ltdir,
+ build_libtool_libs => 'no',
build_old_libs => 'yes',
pic_flags => join(' ', @picflags),
+ elf => 1,
+ noshared => 0,
}, $class;
($self->{gnu_arch} = $self->{machine_arch}) =~ s/amd64/x86_64/;
- if (grep { $_ eq $self->{machine_arch} } qw(vax)) {
- $self->{build_libtool_libs} = 'yes';
- $self->{noshared} = 1;
- } else {
- $self->{build_libtool_libs} = 'no';
- $self->{noshared} = 0;
- }
- if (grep { $_ eq $self->{machine_arch} } qw(vax)) {
- $self->{elf} = 0;
- } else {
- $self->{elf} = 1;
- }
-
return $self;
}
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: Small usr.bin/libtool cleanup
On Fri, Dec 07 2018, Jeremie Courreges-Anglas wrote:
> We don't support vax any more. ok?
On top of previous diff, build_libtool_libs and build_old_libs aren't
used in the code. Can't we delete those as well?
Index: libtool
===
--- libtool.orig
+++ libtool
@@ -51,9 +51,7 @@ sub new
machine_arch => $Config{ARCH},
ltdir => $ltdir,
version => $version,
- objdir => $ltdir,
- build_libtool_libs => 'no',
- build_old_libs => 'yes',
+ objdir => $ltdir,
pic_flags => join(' ', @picflags),
elf => 1,
noshared => 0,
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: [Patch] Specify unit for 'every' in ifstated.conf.5
On Fri, Dec 07, 2018 at 07:48:39PM +1100, Ross L Richardson wrote: > > On Fri, Dec 07, 2018 at 06:42:51AM +, Jason McIntyre wrote: > >[...] > > > > morning. > > > > i'm afraid i think this reads quite poorly - it is hard to actually > > understand what is meant. > > > > here's the thing - someone has already written the text. if we want to > > change the author's text, i think we need to demonstrate that their text > > is wrong, inaccurate, or somehow misleading. > > > > so far, i don;t believe anyone has done this. so i am reluctant to > > change the author's words. > >[...] > > Fair enough. > > The reason I couldn't make it work with "frequency" is that, as Otto > indicated, the second is not a unit of measurement for a frequency. > the frequency is once, every "every" seconds. jmc > "tests / second" would be an appropriate unit for a frequency, but that > is _not_ what we are specifying when using "every". Rather, we are > specifying a "period" - the interval between occurrences of tests. > > > I'm not trying to complicate this, just trying to explain why "frequency" > is an incorrect term. > > Having stated my case, I'm happy to leave the wording to others (if that's > what's preferred). > > Thanks, > Ross >
Re: [Patch] Specify unit for 'every' in ifstated.conf.5
On Fri, Dec 07, 2018 at 06:42:51AM +, Jason McIntyre wrote: >[...] > > morning. > > i'm afraid i think this reads quite poorly - it is hard to actually > understand what is meant. > > here's the thing - someone has already written the text. if we want to > change the author's text, i think we need to demonstrate that their text > is wrong, inaccurate, or somehow misleading. > > so far, i don;t believe anyone has done this. so i am reluctant to > change the author's words. >[...] Fair enough. The reason I couldn't make it work with "frequency" is that, as Otto indicated, the second is not a unit of measurement for a frequency. "tests / second" would be an appropriate unit for a frequency, but that is _not_ what we are specifying when using "every". Rather, we are specifying a "period" - the interval between occurrences of tests. I'm not trying to complicate this, just trying to explain why "frequency" is an incorrect term. Having stated my case, I'm happy to leave the wording to others (if that's what's preferred). Thanks, Ross
