-
Notifications
You must be signed in to change notification settings - Fork 268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: handle extra padding if key length > 2048 #648
fix: handle extra padding if key length > 2048 #648
Conversation
6e170c1
to
f02922d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Could you also please add a test for this? Then I'll merge it.
It's not clear for me how and where to write tests for the secure_channel_instance. There are only a few tests for the |
Hmm, fair enough. Let me have a look |
I don't think this is so bad since Maybe start with something like this (code does NOT compile :)) func TestSignAndEncrypt(t *testing.T) {
tests := []struct{
name string
c *channelInstance
m *Message
b []byte
want []byte
err error
}{
{"none",
&channelInstance{
sc: SecureChannel{cfg: Config{SecurityMode: None}}
},
...
}
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
signed, err := tt.c.signAndEncrypt(tt.m, tt.b)
if got, want := err.String(), tt.err.String(); got != want {
t.Fatalf("got error %v want %v", got, want)
}
if got, want := signed, tt.want; !bytes.Equal(got, want) {
...
}
}
}
}
// create some helper funcs to create ciphertext from plaintext
// to make testing easier |
Also, |
If it makes it easier you could put the golden plaintext data into |
@Maddin-619 let me know if that works for you or if you need more help. |
I have problems using the real algorithems, because the encryption always leads to a different ciphertext. So we can't assert with static bytes.
In general it should be better to isolate the testee without the dependencies. So I would prefer solution 2. |
0a7a842
to
b39884e
Compare
Connecting to a opc ua server with a 4096 bit key length gave me an error. The server could not decode the
OpenSecureChannelRequest
because theExtraPaddingSize
byte was missing.If the key length is greater than 2048 bits there should be an
ExtraPaddingSize
byte in the message footer:UA Part 6: Mappings - 6.7.2.5 Message Footer
The PR should fix that problem while encoding and decoding messages.
I implemented the fix in accordance with the c reference implmentation (open62541)