MAC: Avoid always-nil error (#2602)

The `spat.Hop.CalcMac` unecessarily returns an error. It is always nil.

This PR removes the error from the returned values and reduces the
error handling branches in the code.

go/beacon_srv/internal/beaconing/extender.go

@@ -166,8 +166,6 @@ func (s *segExtender) createHopF(inIfid, egIfid common.IFIDType, prev common.Raw
 		// Do not include the flags of the hop field in the mac input.
 		prev = prev[1:]
 	}
-	if hop.Mac, err = hop.CalcMac(s.mac, util.TimeToSecs(ts), prev); err != nil {
-		return nil, common.NewBasicError("Unable to create MAC", err)
-	}
+	hop.Mac = hop.CalcMac(s.mac, util.TimeToSecs(ts), prev)
 	return hop, nil
 }

go/beacon_srv/internal/onehop/sender.go

@@ -92,9 +92,6 @@ func (s *Sender) CreatePkt(msg *Msg) (*snet.SCIONPacket, error) {
 // CreatePath creates the one-hop path and initializes it.
 func (s *Sender) CreatePath(ifid common.IFIDType, now time.Time) (*Path, error) {
 	s.MAC.Reset()
-	path, err := spath.NewOneHop(s.IA.I, ifid, time.Now(), spath.DefaultHopFExpiry, s.MAC)
-	if err != nil {
-		return nil, err
-	}
+	path := spath.NewOneHop(s.IA.I, ifid, time.Now(), spath.DefaultHopFExpiry, s.MAC)
 	return (*Path)(path), path.InitOffsets()
 }

go/border/braccept/parser/scion.go

@@ -323,13 +323,9 @@ func (scn *ScionTaggedLayer) GenerateMac(hMac hash.Hash, infTag, hfTag, hfMacTag
 		hfMac.Write(buf)
 	}
 	hMac.Reset()
-	var err error
 	/// CalcMac assumes TsInt in network order
-	//hf.Mac, err = hf.CalcMac(hMac, common.Order.PutUint32(inf.TsInt, buf))
-	hf.Mac, err = hf.CalcMac(hMac, inf.TsInt, buf[1:])
-	if err != nil {
-		panic(err)
-	}
+	//hf.Mac = hf.CalcMac(hMac, common.Order.PutUint32(inf.TsInt, buf))
+	hf.Mac = hf.CalcMac(hMac, inf.TsInt, buf[1:])
 }
 
 func (scn *ScionTaggedLayer) addTag(tag string, v interface{}) {

go/border/rpkt/extn_onehoppath.go

@@ -79,11 +79,8 @@ func (o *rOneHopPath) HopF() (HookResult, *spath.HopField, error) {
 		ExpTime:     spath.DefaultHopFExpiry,
 	}
 	hfmac := o.rp.Ctx.Conf.HFMacPool.Get().(hash.Hash)
-	mac, err := hopF.CalcMac(hfmac, infoF.TsInt, prevHof)
+	mac := hopF.CalcMac(hfmac, infoF.TsInt, prevHof)
 	o.rp.Ctx.Conf.HFMacPool.Put(hfmac)
-	if err != nil {
-		return HookError, nil, err
-	}
 	hopF.Mac = mac
 	hopF.Write(o.rp.Raw[hOff:])
 	// Return HookContinue so that the default HopF parsing will read the newly

go/lib/scrypto/mac.go

@@ -39,11 +39,3 @@ func InitMac(key common.RawBytes) (hash.Hash, error) {
 	}
 	return mac, nil
 }
-
-func Mac(mac hash.Hash, msg common.RawBytes) (common.RawBytes, error) {
-	mac.Write(msg)
-	tmp := make([]byte, 0, mac.Size())
-	tag := mac.Sum(tmp)
-	mac.Reset()
-	return tag, nil
-}

go/lib/spath/BUILD.bazel

@@ -12,7 +12,6 @@ go_library(
     deps = [
         "//go/lib/addr:go_default_library",
         "//go/lib/common:go_default_library",
-        "//go/lib/scrypto:go_default_library",
         "//go/lib/util:go_default_library",
     ],
 )

go/lib/spath/hop.go

@@ -24,7 +24,6 @@ import (
 	"time"
 
 	"github.com/scionproto/scion/go/lib/common"
-	"github.com/scionproto/scion/go/lib/scrypto"
 )
 
 const (
@@ -111,17 +110,20 @@ func (h *HopField) String() string {
 		h.ConsIngress, h.ConsEgress, h.ExpTime, h.Xover, h.VerifyOnly, h.Mac)
 }
 
+// Verify checks the MAC. The same restrictions on prev as in CalcMac apply, and
+// the function may panic otherwise.
 func (h *HopField) Verify(macH hash.Hash, tsInt uint32, prev common.RawBytes) error {
-	if mac, err := h.CalcMac(macH, tsInt, prev); err != nil {
-		return err
-	} else if !bytes.Equal(h.Mac, mac) {
+	mac := h.CalcMac(macH, tsInt, prev)
+	if !bytes.Equal(h.Mac, mac) {
 		return common.NewBasicError(ErrorHopFBadMac, nil, "expected", mac, "actual", h.Mac)
 	}
 	return nil
 }
 
 // CalcMac calculates the MAC of a HopField and its preceding HopField, if any.
-// prev does not contain flags byte.
+// prev does not contain flags byte. This implies that the length of prev can
+// either be 0 or k*8+7, where k >=0.
+// WARN: If prev is of different length, this function panics.
 //
 // MAC input block format:
 //
@@ -137,16 +139,24 @@ func (h *HopField) Verify(macH hash.Hash, tsInt uint32, prev common.RawBytes) er
 //  |                           PrevHopF                            |
 //  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 //
-func (h *HopField) CalcMac(mac hash.Hash, tsInt uint32,
-	prev common.RawBytes) (common.RawBytes, error) {
-
+func (h *HopField) CalcMac(mac hash.Hash, tsInt uint32, prev common.RawBytes) common.RawBytes {
+	// If the previous hopfield is set, it must be of length k*8+7 (k >= 0),
+	if len(prev) != 0 && (len(prev)&0x7) != 7 {
+		panic(fmt.Sprintf("Bad previous hop field length len=%d", len(prev)))
+	}
 	all := make(common.RawBytes, macInputLen)
 	common.Order.PutUint32(all, tsInt)
 	all[4] = 0 // Ignore flags
 	common.Order.PutUint32(all[5:], h.expTimeIfIdsPack())
 	copy(all[9:], prev)
-	tag, err := scrypto.Mac(mac, all)
-	return tag[:MacLen], err
+
+	mac.Reset()
+	// Write must not return an error: https://godoc.org/hash#Hash
+	if _, err := mac.Write(all); err != nil {
+		panic(err)
+	}
+	tmp := make([]byte, 0, mac.Size())
+	return mac.Sum(tmp)[:MacLen]
 }
 
 // Pack packs the hop field.

go/lib/spath/path.go

@@ -48,7 +48,7 @@ func New(raw common.RawBytes) *Path {
 // NewOneHop creates a new one hop path with. If necessary, the caller has
 // to initialize the offsets.
 func NewOneHop(isd addr.ISD, ifid common.IFIDType, ts time.Time, exp ExpTimeType,
-	hfmac hash.Hash) (*Path, error) {
+	hfmac hash.Hash) *Path {
 
 	info := InfoField{
 		ConsDir: true,
@@ -60,14 +60,11 @@ func NewOneHop(isd addr.ISD, ifid common.IFIDType, ts time.Time, exp ExpTimeType
 		ConsEgress: ifid,
 		ExpTime:    exp,
 	}
-	var err error
-	if hop.Mac, err = hop.CalcMac(hfmac, info.TsInt, nil); err != nil {
-		return nil, err
-	}
+	hop.Mac = hop.CalcMac(hfmac, info.TsInt, nil)
 	raw := make(common.RawBytes, InfoFieldLength+2*HopFieldLength)
 	info.Write(raw[:InfoFieldLength])
 	hop.Write(raw[InfoFieldLength:])
-	return New(raw), nil
+	return New(raw)
 }
 
 func (p *Path) Copy() *Path {

go/lib/spath/path_test.go

@@ -141,13 +141,9 @@ func TestNewOneHop(t *testing.T) {
 	mac, err := scrypto.InitMac(make(common.RawBytes, 16))
 	xtest.FailOnErr(t, err)
 	// Compute the correct tag for the first hop field.
-	tag, err := (&HopField{ConsEgress: 11, ExpTime: 4}).CalcMac(mac, 3, nil)
-	xtest.FailOnErr(t, err)
-	mac.Reset()
-
+	tag := (&HopField{ConsEgress: 11, ExpTime: 4}).CalcMac(mac, 3, nil)
 	Convey("The one hop path should be created correctly", t, func() {
-		p, err := NewOneHop(1, 11, util.SecsToTime(3), 4, mac)
-		SoMsg("err", err, ShouldBeNil)
+		p := NewOneHop(1, 11, util.SecsToTime(3), 4, mac)
 		err = p.InitOffsets()
 		SoMsg("InitOffsets", err, ShouldBeNil)
 		// Check the info field is set correctly.