Security Extension#1
Conversation
created SPSE as a specialized Extension layer struct, to keep pointer to authenticator buffer added ExtensionDataToSPSELayer to create SPSE layer out of spse.Extn create SCIONLayer with serialization logic to replace spkt.WriteScnPacket added Key field, Sum and Verify functions (now only with AES-CMAC) to spse.Extn
…directly to enable storing pointer to MAC buffer
…e initialized from outside
| Type uint8 | ||
| Class common.L4ProtocolType | ||
| Data []byte | ||
| AuthenticatedBytes common.RawBytes |
There was a problem hiding this comment.
[]byte instead.
I am not convinced we need the authenticated bytes directly inside the extension representation.
There was a problem hiding this comment.
We can indeed just store those bytes in local buffers. But we would need to return those buffers to the caller somehow, and thus we violated the signature of SerializeTo. This is part of the argument for why I don't think gopacket does much for us here.
| SCIONPacketInfo: pktInf, | ||
| } | ||
| s := NewSCIONLayer(pkt) | ||
| s.Serialize() |
There was a problem hiding this comment.
we don't check s.Bytes later. How do we know Serialize is correct?
| if err != nil { | ||
| t.Fatal(err.Error()) | ||
| } | ||
| t.Log(bf.Bytes()) |
| t.Fatalf("Length should be %d, but is %d", len(bytes), len(s.Bytes)) | ||
| } | ||
| for i := range s.Bytes[:n] { | ||
| if bytes[i] != s.Bytes[i] { |
|
|
||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
We need to check serialization/deserialization, as well as the authenticators. For that, we should have more than 1 e2e extension and more than 1 hbh extension; with and without spse.
- Refactored ExtensionDataToSPSE - Replaced 3 with common.ExtnSubHdrLen in extensions - Replaced common.RawBytes with []byte - Replaced Type and Class fields with one Type field of type common.ExtnType in extensions - Removed multiple append calls from SCIONLayer.serialize
Added Serialize() function to SCIONPacket to abstract away SCIONLayer Changed SCIONLayer to scionLayer (private)
juagargi
left a comment
juagargi
left a comment
There was a problem hiding this comment.
ATM braccept fails to build.
Reviewed 4 of 14 files at r1, 7 of 10 files at r2.
Reviewable status: 5 of 13 files reviewed, 26 unresolved discussions (waiting on @juagargi and @radwasherif)
go/lib/layers/extensions.go, line 295 at r2 (raw file):
extn spse.Extn) (*SPSE, error) { bytes, err := extn.Pack() authStartOffset := spse.SecModeLength + len(extn.Metadata)
no need for a variable, just call it inline. Same for the other variable below.
go/lib/layers/extensions.go, line 300 at r2 (raw file):
Previously, radwasherif (Radwa Sherif) wrote…
Done
👍
go/lib/layers/extensions_layer.go, line 149 at r2 (raw file):
Previously, radwasherif (Radwa Sherif) wrote…
Done.
I still see the 3 instead of common.ExtnSubHdrLen
2 participants
This change is