Improve security/teleport build

[seanc]

Summary

Upgrade gravitational teleport to 2.6.6.

Explicitly specify the git sha when building teleport. Restrict builds to amd64.

Test Plan

doas poudriere testport -j 12amd64 -o security/teleport -p dev

Result is clean.

[mat]

You need to bump PORTREVISION.

Also, could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

[seanc]

Upgrade gravitational teleport to 2.5.7.

Explicitly specify the git sha when building teleport. Restrict builds to amd64.

[seanc]

In D15311#323813, @mat wrote:

You need to bump PORTREVISION.

Also, could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

2.5.7 was released so I bumped that the PORTVERSION instead.

[swills]

LGTM

[mat]

head/security/teleport/Makefile

13

Please add ONLY_FOR_ARCHS_REASON to explain.

15–17

Remove ${LOCALBASE}/bin/.

23

Remove, default.

32

Use @sample?

35

Why?

46

It would probably be easier to simply do something like:

's|^GITREF=.*|GITREF=${GH_TAG_COMMIT}|'

and remove the patch.

55

Remove -s, builds must not be silent.

[seanc]

Updated port with feedback from @mat

head/security/teleport/Makefile

15–17

I looked for a way to see if there is a test to verify that a version of go is greater than a given version. If a user only has go1.4 in their path then the build will fail, whereas using ${LOCALBASE}/bin implies someone is keeping current with their ports maintenance.

I've made the adjustment, but that's why I used ${LOCALBASE}/bin.

35

The teleport binary includes extra UI elements appended to the binary that is removed when the binary is stripped.

[mat]

Could you use devel/arcanist, or at least generate a diff with full context like it does, with svn diff -x -U9999 or git diff -U9999.

head/security/teleport/Makefile

15–17

I don't understand. If you remove ${LOCALBASE}/bin/ as I asked, the framework will look for a binary named go in PATH. If a user has a go14 binary, well, it will not be used, as it is not called go.

Please remove ${LOCALBASE}/bin/.

35

Then please add a comment before the STRIP variable saying so.

[seanc]

Paste with context

[seanc]

head/security/teleport/Makefile

15–17

I understand. My comment is with regards to common development patterns where ~/go/bin/go may not be the latest and greatest and may not meet the build requirements. It's common to use PATH=$GOPATH/bin:$PATH which means a user's local go would be used. I was trying to guard against that particular outcome. I fixed it in the patch but was explaining why I used ${LOCALBASE}/bin/go vs just go.

[mat]

Mmmm, right, but I really do not feel this edge case needs to be addressed, packages are supposed to be built in a clean environment in poudriere/synth, or as root, I think it's safe to assume that there will not be a ~root/.../go.

[seanc]

Agreed, and I have already removed ${LOCALBASE}/bin/go from BUILD_DEPENDS - I wasn't arguing with you, I was only explaining why I added it in the first place. :)

[seanc]

Incorporate @swills' feedback. Work around daemon(8)'s lack of syslog functionality in 10-STABLE.

[mat]

head/security/teleport/Makefile

48–49

Please use -exec ... + instead of -exec ... ;, also, fold the two sed commands into one line. It will make it run one or two sed commands instead of twice the number of files there are in WRKDIR.

59–60

run sed only once.

head/security/teleport/Makefile

 #endif
 #endif /* DEV_ISA */
 
 static MALLOC_DEFINE(M_FPUKERN_CTX, "fpukern_ctx",
     "Kernel contexts for FPU state");
 
 #define FPU_KERN_CTX_NPXINITDONE 0x01
 #define FPU_KERN_CTX_DUMMY       0x02
+#define FPU_KERN_CTX_INUSE       0x04
 
 struct fpu_kern_ctx {
         union savefpu *prev;
         uint32_t flags;
         char hwstate1[];
 };
 
 struct fpu_kern_ctx *
 fpu_kern_alloc_ctx(u_int flags)
 {
         struct fpu_kern_ctx *res;
         size_t sz;
 
         sz = sizeof(struct fpu_kern_ctx) + XSAVE_AREA_ALIGN +
             cpu_max_ext_state_size;
         res = malloc(sz, M_FPUKERN_CTX, ((flags & FPU_KERN_NOWAIT) ?
             M_NOWAIT : M_WAITOK) | M_ZERO);
         return (res);
 }
 
 void
 fpu_kern_free_ctx(struct fpu_kern_ctx *ctx)
 {
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("free'ing inuse ctx"));
         /* XXXKIB clear the memory ? */
         free(ctx, M_FPUKERN_CTX);
 }
 
 static union savefpu *
 fpu_kern_ctx_savefpu(struct fpu_kern_ctx *ctx)
 {
         vm_offset_t p;
 
         p = (vm_offset_t)&ctx->hwstate1;
         p = roundup2(p, XSAVE_AREA_ALIGN);
         return ((union savefpu *)p);
 }
 
 int
 fpu_kern_enter(struct thread *td, struct fpu_kern_ctx *ctx, u_int flags)
 {
         struct pcb *pcb;
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("using inuse ctx"));
+
         if ((flags & FPU_KERN_KTHR) != 0 && is_fpu_kern_thread(0)) {
-                ctx->flags = FPU_KERN_CTX_DUMMY;
+                ctx->flags = FPU_KERN_CTX_DUMMY | FPU_KERN_CTX_INUSE;
                 return (0);
         }
         pcb = td->td_pcb;
         KASSERT(!PCB_USER_FPU(pcb) || pcb->pcb_save ==
             get_pcb_user_save_pcb(pcb), ("mangled pcb_save"));
-        ctx->flags = 0;
+        ctx->flags = FPU_KERN_CTX_INUSE;
         if ((pcb->pcb_flags & PCB_NPXINITDONE) != 0)
                 ctx->flags |= FPU_KERN_CTX_NPXINITDONE;
         npxexit(td);
         ctx->prev = pcb->pcb_save;
         pcb->pcb_save = fpu_kern_ctx_savefpu(ctx);
         pcb->pcb_flags |= PCB_KERNNPX;
         pcb->pcb_flags &= ~PCB_NPXINITDONE;
         return (0);
 }
 
 int
 fpu_kern_leave(struct thread *td, struct fpu_kern_ctx *ctx)
 {
         struct pcb *pcb;
+
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) != 0,
+            ("leaving not inuse ctx"));
+        ctx->flags &= ~FPU_KERN_CTX_INUSE;
 
         if (is_fpu_kern_thread(0) && (ctx->flags & FPU_KERN_CTX_DUMMY) != 0)
                 return (0);
         pcb = td->td_pcb;
         critical_enter();
         if (curthread == PCPU_GET(fpcurthread))
                 npxdrop();
         critical_exit();

head/security/teleport/distinfo

 #endif
 #endif /* DEV_ISA */
 
 static MALLOC_DEFINE(M_FPUKERN_CTX, "fpukern_ctx",
     "Kernel contexts for FPU state");
 
 #define FPU_KERN_CTX_NPXINITDONE 0x01
 #define FPU_KERN_CTX_DUMMY       0x02
+#define FPU_KERN_CTX_INUSE       0x04
 
 struct fpu_kern_ctx {
         union savefpu *prev;
         uint32_t flags;
         char hwstate1[];
 };
 
 struct fpu_kern_ctx *
 fpu_kern_alloc_ctx(u_int flags)
 {
         struct fpu_kern_ctx *res;
         size_t sz;
 
         sz = sizeof(struct fpu_kern_ctx) + XSAVE_AREA_ALIGN +
             cpu_max_ext_state_size;
         res = malloc(sz, M_FPUKERN_CTX, ((flags & FPU_KERN_NOWAIT) ?
             M_NOWAIT : M_WAITOK) | M_ZERO);
         return (res);
 }
 
 void
 fpu_kern_free_ctx(struct fpu_kern_ctx *ctx)
 {
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("free'ing inuse ctx"));
         /* XXXKIB clear the memory ? */
         free(ctx, M_FPUKERN_CTX);
 }
 
 static union savefpu *
 fpu_kern_ctx_savefpu(struct fpu_kern_ctx *ctx)
 {
         vm_offset_t p;
 
         p = (vm_offset_t)&ctx->hwstate1;
         p = roundup2(p, XSAVE_AREA_ALIGN);
         return ((union savefpu *)p);
 }
 
 int
 fpu_kern_enter(struct thread *td, struct fpu_kern_ctx *ctx, u_int flags)
 {
         struct pcb *pcb;
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("using inuse ctx"));
+
         if ((flags & FPU_KERN_KTHR) != 0 && is_fpu_kern_thread(0)) {
-                ctx->flags = FPU_KERN_CTX_DUMMY;
+                ctx->flags = FPU_KERN_CTX_DUMMY | FPU_KERN_CTX_INUSE;
                 return (0);
         }
         pcb = td->td_pcb;
         KASSERT(!PCB_USER_FPU(pcb) || pcb->pcb_save ==
             get_pcb_user_save_pcb(pcb), ("mangled pcb_save"));
-        ctx->flags = 0;
+        ctx->flags = FPU_KERN_CTX_INUSE;
         if ((pcb->pcb_flags & PCB_NPXINITDONE) != 0)
                 ctx->flags |= FPU_KERN_CTX_NPXINITDONE;
         npxexit(td);
         ctx->prev = pcb->pcb_save;
         pcb->pcb_save = fpu_kern_ctx_savefpu(ctx);
         pcb->pcb_flags |= PCB_KERNNPX;
         pcb->pcb_flags &= ~PCB_NPXINITDONE;
         return (0);
 }
 
 int
 fpu_kern_leave(struct thread *td, struct fpu_kern_ctx *ctx)
 {
         struct pcb *pcb;
+
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) != 0,
+            ("leaving not inuse ctx"));
+        ctx->flags &= ~FPU_KERN_CTX_INUSE;
 
         if (is_fpu_kern_thread(0) && (ctx->flags & FPU_KERN_CTX_DUMMY) != 0)
                 return (0);
         pcb = td->td_pcb;
         critical_enter();
         if (curthread == PCPU_GET(fpcurthread))
                 npxdrop();
         critical_exit();

head/security/teleport/files/patch-version.mk

 #endif
 #endif /* DEV_ISA */
 
 static MALLOC_DEFINE(M_FPUKERN_CTX, "fpukern_ctx",
     "Kernel contexts for FPU state");
 
 #define FPU_KERN_CTX_NPXINITDONE 0x01
 #define FPU_KERN_CTX_DUMMY       0x02
+#define FPU_KERN_CTX_INUSE       0x04
 
 struct fpu_kern_ctx {
         union savefpu *prev;
         uint32_t flags;
         char hwstate1[];
 };
 
 struct fpu_kern_ctx *
 fpu_kern_alloc_ctx(u_int flags)
 {
         struct fpu_kern_ctx *res;
         size_t sz;
 
         sz = sizeof(struct fpu_kern_ctx) + XSAVE_AREA_ALIGN +
             cpu_max_ext_state_size;
         res = malloc(sz, M_FPUKERN_CTX, ((flags & FPU_KERN_NOWAIT) ?
             M_NOWAIT : M_WAITOK) | M_ZERO);
         return (res);
 }
 
 void
 fpu_kern_free_ctx(struct fpu_kern_ctx *ctx)
 {
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("free'ing inuse ctx"));
         /* XXXKIB clear the memory ? */
         free(ctx, M_FPUKERN_CTX);
 }
 
 static union savefpu *
 fpu_kern_ctx_savefpu(struct fpu_kern_ctx *ctx)
 {
         vm_offset_t p;
 
         p = (vm_offset_t)&ctx->hwstate1;
         p = roundup2(p, XSAVE_AREA_ALIGN);
         return ((union savefpu *)p);
 }
 
 int
 fpu_kern_enter(struct thread *td, struct fpu_kern_ctx *ctx, u_int flags)
 {
         struct pcb *pcb;
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("using inuse ctx"));
+
         if ((flags & FPU_KERN_KTHR) != 0 && is_fpu_kern_thread(0)) {
-                ctx->flags = FPU_KERN_CTX_DUMMY;
+                ctx->flags = FPU_KERN_CTX_DUMMY | FPU_KERN_CTX_INUSE;
                 return (0);
         }
         pcb = td->td_pcb;
         KASSERT(!PCB_USER_FPU(pcb) || pcb->pcb_save ==
             get_pcb_user_save_pcb(pcb), ("mangled pcb_save"));
-        ctx->flags = 0;
+        ctx->flags = FPU_KERN_CTX_INUSE;
         if ((pcb->pcb_flags & PCB_NPXINITDONE) != 0)
                 ctx->flags |= FPU_KERN_CTX_NPXINITDONE;
         npxexit(td);
         ctx->prev = pcb->pcb_save;
         pcb->pcb_save = fpu_kern_ctx_savefpu(ctx);
         pcb->pcb_flags |= PCB_KERNNPX;
         pcb->pcb_flags &= ~PCB_NPXINITDONE;
         return (0);
 }
 
 int
 fpu_kern_leave(struct thread *td, struct fpu_kern_ctx *ctx)
 {
         struct pcb *pcb;
+
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) != 0,
+            ("leaving not inuse ctx"));
+        ctx->flags &= ~FPU_KERN_CTX_INUSE;
 
         if (is_fpu_kern_thread(0) && (ctx->flags & FPU_KERN_CTX_DUMMY) != 0)
                 return (0);
         pcb = td->td_pcb;
         critical_enter();
         if (curthread == PCPU_GET(fpcurthread))
                 npxdrop();
         critical_exit();

head/security/teleport/files/teleport.in

 #endif
 #endif /* DEV_ISA */
 
 static MALLOC_DEFINE(M_FPUKERN_CTX, "fpukern_ctx",
     "Kernel contexts for FPU state");
 
 #define FPU_KERN_CTX_NPXINITDONE 0x01
 #define FPU_KERN_CTX_DUMMY       0x02
+#define FPU_KERN_CTX_INUSE       0x04
 
 struct fpu_kern_ctx {
         union savefpu *prev;
         uint32_t flags;
         char hwstate1[];
 };
 
 struct fpu_kern_ctx *
 fpu_kern_alloc_ctx(u_int flags)
 {
         struct fpu_kern_ctx *res;
         size_t sz;
 
         sz = sizeof(struct fpu_kern_ctx) + XSAVE_AREA_ALIGN +
             cpu_max_ext_state_size;
         res = malloc(sz, M_FPUKERN_CTX, ((flags & FPU_KERN_NOWAIT) ?
             M_NOWAIT : M_WAITOK) | M_ZERO);
         return (res);
 }
 
 void
 fpu_kern_free_ctx(struct fpu_kern_ctx *ctx)
 {
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("free'ing inuse ctx"));
         /* XXXKIB clear the memory ? */
         free(ctx, M_FPUKERN_CTX);
 }
 
 static union savefpu *
 fpu_kern_ctx_savefpu(struct fpu_kern_ctx *ctx)
 {
         vm_offset_t p;
 
         p = (vm_offset_t)&ctx->hwstate1;
         p = roundup2(p, XSAVE_AREA_ALIGN);
         return ((union savefpu *)p);
 }
 
 int
 fpu_kern_enter(struct thread *td, struct fpu_kern_ctx *ctx, u_int flags)
 {
         struct pcb *pcb;
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("using inuse ctx"));
+
         if ((flags & FPU_KERN_KTHR) != 0 && is_fpu_kern_thread(0)) {
-                ctx->flags = FPU_KERN_CTX_DUMMY;
+                ctx->flags = FPU_KERN_CTX_DUMMY | FPU_KERN_CTX_INUSE;
                 return (0);
         }
         pcb = td->td_pcb;
         KASSERT(!PCB_USER_FPU(pcb) || pcb->pcb_save ==
             get_pcb_user_save_pcb(pcb), ("mangled pcb_save"));
-        ctx->flags = 0;
+        ctx->flags = FPU_KERN_CTX_INUSE;
         if ((pcb->pcb_flags & PCB_NPXINITDONE) != 0)
                 ctx->flags |= FPU_KERN_CTX_NPXINITDONE;
         npxexit(td);
         ctx->prev = pcb->pcb_save;
         pcb->pcb_save = fpu_kern_ctx_savefpu(ctx);
         pcb->pcb_flags |= PCB_KERNNPX;
         pcb->pcb_flags &= ~PCB_NPXINITDONE;
         return (0);
 }
 
 int
 fpu_kern_leave(struct thread *td, struct fpu_kern_ctx *ctx)
 {
         struct pcb *pcb;
+
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) != 0,
+            ("leaving not inuse ctx"));
+        ctx->flags &= ~FPU_KERN_CTX_INUSE;
 
         if (is_fpu_kern_thread(0) && (ctx->flags & FPU_KERN_CTX_DUMMY) != 0)
                 return (0);
         pcb = td->td_pcb;
         critical_enter();
         if (curthread == PCPU_GET(fpcurthread))
                 npxdrop();
         critical_exit();

head/security/teleport/pkg-descr

 #endif
 #endif /* DEV_ISA */
 
 static MALLOC_DEFINE(M_FPUKERN_CTX, "fpukern_ctx",
     "Kernel contexts for FPU state");
 
 #define FPU_KERN_CTX_NPXINITDONE 0x01
 #define FPU_KERN_CTX_DUMMY       0x02
+#define FPU_KERN_CTX_INUSE       0x04
 
 struct fpu_kern_ctx {
         union savefpu *prev;
         uint32_t flags;
         char hwstate1[];
 };
 
 struct fpu_kern_ctx *
 fpu_kern_alloc_ctx(u_int flags)
 {
         struct fpu_kern_ctx *res;
         size_t sz;
 
         sz = sizeof(struct fpu_kern_ctx) + XSAVE_AREA_ALIGN +
             cpu_max_ext_state_size;
         res = malloc(sz, M_FPUKERN_CTX, ((flags & FPU_KERN_NOWAIT) ?
             M_NOWAIT : M_WAITOK) | M_ZERO);
         return (res);
 }
 
 void
 fpu_kern_free_ctx(struct fpu_kern_ctx *ctx)
 {
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("free'ing inuse ctx"));
         /* XXXKIB clear the memory ? */
         free(ctx, M_FPUKERN_CTX);
 }
 
 static union savefpu *
 fpu_kern_ctx_savefpu(struct fpu_kern_ctx *ctx)
 {
         vm_offset_t p;
 
         p = (vm_offset_t)&ctx->hwstate1;
         p = roundup2(p, XSAVE_AREA_ALIGN);
         return ((union savefpu *)p);
 }
 
 int
 fpu_kern_enter(struct thread *td, struct fpu_kern_ctx *ctx, u_int flags)
 {
         struct pcb *pcb;
 
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) == 0, ("using inuse ctx"));
+
         if ((flags & FPU_KERN_KTHR) != 0 && is_fpu_kern_thread(0)) {
-                ctx->flags = FPU_KERN_CTX_DUMMY;
+                ctx->flags = FPU_KERN_CTX_DUMMY | FPU_KERN_CTX_INUSE;
                 return (0);
         }
         pcb = td->td_pcb;
         KASSERT(!PCB_USER_FPU(pcb) || pcb->pcb_save ==
             get_pcb_user_save_pcb(pcb), ("mangled pcb_save"));
-        ctx->flags = 0;
+        ctx->flags = FPU_KERN_CTX_INUSE;
         if ((pcb->pcb_flags & PCB_NPXINITDONE) != 0)
                 ctx->flags |= FPU_KERN_CTX_NPXINITDONE;
         npxexit(td);
         ctx->prev = pcb->pcb_save;
         pcb->pcb_save = fpu_kern_ctx_savefpu(ctx);
         pcb->pcb_flags |= PCB_KERNNPX;
         pcb->pcb_flags &= ~PCB_NPXINITDONE;
         return (0);
 }
 
 int
 fpu_kern_leave(struct thread *td, struct fpu_kern_ctx *ctx)
 {
         struct pcb *pcb;
+
+        KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) != 0,
+            ("leaving not inuse ctx"));
+        ctx->flags &= ~FPU_KERN_CTX_INUSE;
 
         if (is_fpu_kern_thread(0) && (ctx->flags & FPU_KERN_CTX_DUMMY) != 0)
                 return (0);
         pcb = td->td_pcb;
         critical_enter();
         if (curthread == PCPU_GET(fpcurthread))
                 npxdrop();
         critical_exit();