diff --git a/go/beacon_srv/internal/beaconing/extender.go b/go/beacon_srv/internal/beaconing/extender.go index 9b0ffeeab0..e3da46e16a 100644 --- a/go/beacon_srv/internal/beaconing/extender.go +++ b/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 } diff --git a/go/beacon_srv/internal/onehop/sender.go b/go/beacon_srv/internal/onehop/sender.go index fcf1aa8fc7..21c0b0b171 100644 --- a/go/beacon_srv/internal/onehop/sender.go +++ b/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() } diff --git a/go/border/braccept/parser/scion.go b/go/border/braccept/parser/scion.go index 4d9c44fba9..6d07c4fcb2 100644 --- a/go/border/braccept/parser/scion.go +++ b/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{}) { diff --git a/go/border/rpkt/extn_onehoppath.go b/go/border/rpkt/extn_onehoppath.go index 0d7a24838e..91156afe53 100644 --- a/go/border/rpkt/extn_onehoppath.go +++ b/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 diff --git a/go/lib/scrypto/mac.go b/go/lib/scrypto/mac.go index 617a5e86fb..df5898ec12 100644 --- a/go/lib/scrypto/mac.go +++ b/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 -} diff --git a/go/lib/spath/BUILD.bazel b/go/lib/spath/BUILD.bazel index efb68555f5..064fffd9da 100644 --- a/go/lib/spath/BUILD.bazel +++ b/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", ], ) diff --git a/go/lib/spath/hop.go b/go/lib/spath/hop.go index 76b0a6d282..f5fc0b1b60 100644 --- a/go/lib/spath/hop.go +++ b/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. diff --git a/go/lib/spath/path.go b/go/lib/spath/path.go index db0059ec00..d9c0591913 100644 --- a/go/lib/spath/path.go +++ b/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 { diff --git a/go/lib/spath/path_test.go b/go/lib/spath/path_test.go index 48982317ed..2bc3965378 100644 --- a/go/lib/spath/path_test.go +++ b/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.