| author | |
| committer | |
| log | c41b9d75083dc80b56c45673dab167be0f2727e9 |
| tree | b59efc3bc1582a6c3f199455007b9201992abd6d |
| parent | 05cff8a558fda0e974ea7d00f26f8f4ff6e1195f |
| signature |
I noticed this by stress testing my tls server implementation. From time to time curl (and other tools: ab, vegeta) will report invalid signature. I trace the problem to the way how std lib is encoding raw signature into der format. Using raw signature I got in some cases different encoding using std and openssl. Std is not producing minimal der when signature `r` or `s` integers has leading zero(es).
Here is an example to illustrate difference. Notice leading 00 in `s`
integer which is removed in openssl encoding but not in std encoding.
```Zig
const std = @import("std");
test "ecdsa signature to der" {
// raw signature r and s bytes
const raw = hexToBytes(
\\ 49 63 0c 94 95 2e ff 4b 02 bf 35 c4 97 9e a7 24
\\ 20 dc 94 de aa 1b 17 ff e1 49 25 3e 34 ef e8 d0
\\ c4 43 aa 7b a9 f3 9c b9 f8 72 7d d7 0c 9a 13 1e
\\
\\ 00 56 85 43 d3 d4 05 62 a1 1d d8 a1 45 44 b5 dd
\\ 62 9f d1 e0 ab f1 cd 4a 85 d0 1f 5d 11 d9 f8 89
\\ 89 d4 59 0c b0 6e ea 3c 19 6a f7 0b 1a 4a ce f1
);
// encoded by openssl
const expected = hexToBytes(
\\ 30 63 02 30
\\ 49 63 0c 94 95 2e ff 4b 02 bf 35 c4 97 9e a7 24
\\ 20 dc 94 de aa 1b 17 ff e1 49 25 3e 34 ef e8 d0
\\ c4 43 aa 7b a9 f3 9c b9 f8 72 7d d7 0c 9a 13 1e
\\
\\ 02 2f
\\ 56 85 43 d3 d4 05 62 a1 1d d8 a1 45 44 b5 dd
\\ 62 9f d1 e0 ab f1 cd 4a 85 d0 1f 5d 11 d9 f8 89
\\ 89 d4 59 0c b0 6e ea 3c 19 6a f7 0b 1a 4a ce f1
);
// encoded by std
const actual = hexToBytes(
\\ 30 64 02 30
\\ 49 63 0c 94 95 2e ff 4b 02 bf 35 c4 97 9e a7 24
\\ 20 dc 94 de aa 1b 17 ff e1 49 25 3e 34 ef e8 d0
\\ c4 43 aa 7b a9 f3 9c b9 f8 72 7d d7 0c 9a 13 1e
\\
\\ 02 30
\\ 00 56 85 43 d3 d4 05 62 a1 1d d8 a1 45 44 b5 dd
\\ 62 9f d1 e0 ab f1 cd 4a 85 d0 1f 5d 11 d9 f8 89
\\ 89 d4 59 0c b0 6e ea 3c 19 6a f7 0b 1a 4a ce f1
);
_ = actual;
const Ecdsa = std.crypto.sign.ecdsa.EcdsaP384Sha384;
const sig = Ecdsa.Signature.fromBytes(raw);
var buf: [Ecdsa.Signature.der_encoded_length_max]u8 = undefined;
const encoded = sig.toDer(&buf);
try std.testing.expectEqualSlices(u8, &expected, encoded);
}
pub fn hexToBytes(comptime hex: []const u8) [removeNonHex(hex).len / 2]u8 {
@setEvalBranchQuota(1000 * 100);
const hex2 = comptime removeNonHex(hex);
comptime var res: [hex2.len / 2]u8 = undefined;
_ = comptime std.fmt.hexToBytes(&res, hex2) catch unreachable;
return res;
}
fn removeNonHex(comptime hex: []const u8) []const u8 {
@setEvalBranchQuota(1000 * 100);
var res: [hex.len]u8 = undefined;
var i: usize = 0;
for (hex) |c| {
if (std.ascii.isHex(c)) {
res[i] = c;
i += 1;
}
}
return res[0..i];
}
```
Trimming leading zeroes from signature integers fixes encoding.1 files changed, 8 insertions(+), 6 deletions(-)
lib/std/crypto/ecdsa.zig+8-6| ... | @@ -135,20 +135,22 @@ pub fn Ecdsa(comptime Curve: type, comptime Hash: type) type { | ... | @@ -135,20 +135,22 @@ pub fn Ecdsa(comptime Curve: type, comptime Hash: type) type { |
| 135 | /// The function returns a slice, that can be shorter than der_encoded_length_max. | 135 | /// The function returns a slice, that can be shorter than der_encoded_length_max. |
| 136 | pub fn toDer(sig: Signature, buf: *[der_encoded_length_max]u8) []u8 { | 136 | pub fn toDer(sig: Signature, buf: *[der_encoded_length_max]u8) []u8 { |
| 137 | var w: std.Io.Writer = .fixed(buf); | 137 | var w: std.Io.Writer = .fixed(buf); |
| 138 | const r_len = @as(u8, @intCast(sig.r.len + (sig.r[0] >> 7))); | 138 | const sig_r = mem.trimLeft(u8, &sig.r, &.{0}); |
| 139 | const s_len = @as(u8, @intCast(sig.s.len + (sig.s[0] >> 7))); | 139 | const sig_s = mem.trimLeft(u8, &sig.s, &.{0}); |
| 140 | const r_len = @as(u8, @intCast(sig_r.len + (sig_r[0] >> 7))); | ||
| 141 | const s_len = @as(u8, @intCast(sig_s.len + (sig_s[0] >> 7))); | ||
| 140 | const seq_len = @as(u8, @intCast(2 + r_len + 2 + s_len)); | 142 | const seq_len = @as(u8, @intCast(2 + r_len + 2 + s_len)); |
| 141 | w.writeAll(&[_]u8{ 0x30, seq_len }) catch unreachable; | 143 | w.writeAll(&[_]u8{ 0x30, seq_len }) catch unreachable; |
| 142 | w.writeAll(&[_]u8{ 0x02, r_len }) catch unreachable; | 144 | w.writeAll(&[_]u8{ 0x02, r_len }) catch unreachable; |
| 143 | if (sig.r[0] >> 7 != 0) { | 145 | if (sig_r[0] >> 7 != 0) { |
| 144 | w.writeByte(0x00) catch unreachable; | 146 | w.writeByte(0x00) catch unreachable; |
| 145 | } | 147 | } |
| 146 | w.writeAll(&sig.r) catch unreachable; | 148 | w.writeAll(sig_r) catch unreachable; |
| 147 | w.writeAll(&[_]u8{ 0x02, s_len }) catch unreachable; | 149 | w.writeAll(&[_]u8{ 0x02, s_len }) catch unreachable; |
| 148 | if (sig.s[0] >> 7 != 0) { | 150 | if (sig_s[0] >> 7 != 0) { |
| 149 | w.writeByte(0x00) catch unreachable; | 151 | w.writeByte(0x00) catch unreachable; |
| 150 | } | 152 | } |
| 151 | w.writeAll(&sig.s) catch unreachable; | 153 | w.writeAll(sig_s) catch unreachable; |
| 152 | return w.buffered(); | 154 | return w.buffered(); |
| 153 | } | 155 | } |
| 154 | 156 |