Skip to content

Commit 636247e

Browse files
committed
Fix logic issues
1 parent 96a1532 commit 636247e

File tree

8 files changed

+199
-17
lines changed

8 files changed

+199
-17
lines changed

packages/pq-key-encoder/ts/README.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,13 @@ interface PQJwk {
148148
## Notes
149149

150150
- Inputs are validated for correct key size and encoding structure
151-
- DER encoding uses AlgorithmIdentifier with explicit NULL parameters
151+
- DER encoding uses AlgorithmIdentifier with absent parameters (per NIST PQ specs); decoder accepts both absent and NULL for interoperability
152152
- JWK uses non-standard `kty: 'PQC'` for post-quantum keys
153+
- `fromJWK` returns only private key bytes when both `x` and `d` are present (public key is validated but not returned). To get both keys, parse separately:
154+
```typescript
155+
const privateKey = fromJWK(jwk);
156+
const publicKey = fromJWK({ kty: jwk.kty, alg: jwk.alg, x: jwk.x });
157+
```
153158

154159
## License
155160

packages/pq-key-encoder/ts/src/asn1/parse.ts

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -128,18 +128,39 @@ function parseAlgorithmIdentifier(
128128
return { oid, bytesRead: algorithm.bytesRead };
129129
}
130130

131+
// Context-specific tags for optional PKCS#8 fields (RFC 5958 OneAsymmetricKey)
132+
const TAG_CONTEXT_0 = 0xa0; // [0] Attributes OPTIONAL
133+
const TAG_CONTEXT_1 = 0xa1; // [1] PublicKey OPTIONAL
134+
135+
/** Skip optional trailing context-specific fields in PKCS#8/OneAsymmetricKey. */
136+
function scanOptionalTrailingFields(
137+
sequence: Uint8Array,
138+
offset: number,
139+
): { hasPublicKey: boolean } {
140+
let currentOffset = offset;
141+
let hasPublicKey = false;
142+
while (currentOffset < sequence.length) {
143+
const tlv = readTLV(sequence, currentOffset);
144+
if (tlv.tag !== TAG_CONTEXT_0 && tlv.tag !== TAG_CONTEXT_1) {
145+
throw new InvalidEncodingError('Unexpected trailing data in key sequence.');
146+
}
147+
if (tlv.tag === TAG_CONTEXT_1) {
148+
hasPublicKey = true;
149+
}
150+
currentOffset += tlv.bytesRead;
151+
}
152+
return { hasPublicKey };
153+
}
154+
131155
/** Parse the key TLV and infer key type from its tag. */
132156
function parseKeyTlv(
133157
sequence: Uint8Array,
134158
offset: number,
135-
): { keyBytes: Uint8Array; keyType: 'public' | 'private' } {
159+
): { keyBytes: Uint8Array; keyType: 'public' | 'private'; bytesRead: number } {
136160
if (offset >= sequence.length) {
137161
throw new InvalidEncodingError('Missing key data.');
138162
}
139163
const keyTlv = readTLV(sequence, offset);
140-
if (offset + keyTlv.bytesRead !== sequence.length) {
141-
throw new InvalidEncodingError('Unexpected trailing data in key sequence.');
142-
}
143164

144165
if (keyTlv.tag === TAG_BIT_STRING) {
145166
if (keyTlv.value.length === 0) {
@@ -149,10 +170,10 @@ function parseKeyTlv(
149170
if (unusedBits !== 0) {
150171
throw new InvalidEncodingError('BIT STRING unused bits must be 0 for key data.');
151172
}
152-
return { keyBytes: keyTlv.value.slice(1), keyType: 'public' };
173+
return { keyBytes: keyTlv.value.slice(1), keyType: 'public', bytesRead: keyTlv.bytesRead };
153174
}
154175
if (keyTlv.tag === TAG_OCTET_STRING) {
155-
return { keyBytes: keyTlv.value, keyType: 'private' };
176+
return { keyBytes: keyTlv.value, keyType: 'private', bytesRead: keyTlv.bytesRead };
156177
}
157178
throw new InvalidEncodingError('Expected BIT STRING or OCTET STRING for key data.');
158179
}
@@ -174,22 +195,43 @@ export function parseAlgorithmAndKeyWithType(input: Uint8Array): {
174195
const sequence = outer.value;
175196
const first = readTLV(sequence, 0);
176197
let algorithmOffset = 0;
177-
let expectedKeyType: 'public' | 'private' | undefined;
198+
let version: number | null = null;
178199

179200
if (first.tag === TAG_INTEGER) {
180-
if (first.value.length !== 1 || first.value[0] !== 0x00) {
201+
// RFC 5958: version 0 = PKCS#8, version 1 = OneAsymmetricKey with optional publicKey
202+
const versionValue = first.value.length === 1 ? first.value[0] : -1;
203+
if (versionValue !== 0 && versionValue !== 1) {
181204
throw new InvalidEncodingError('Unsupported PrivateKeyInfo version.');
182205
}
183206
algorithmOffset = first.bytesRead;
184-
expectedKeyType = 'private';
207+
version = versionValue;
185208
}
186209

187210
const { oid, bytesRead } = parseAlgorithmIdentifier(sequence, algorithmOffset);
188211
const keyOffset = algorithmOffset + bytesRead;
189-
const { keyBytes, keyType } = parseKeyTlv(sequence, keyOffset);
212+
const { keyBytes, keyType, bytesRead: keyBytesRead } = parseKeyTlv(sequence, keyOffset);
213+
214+
// PKCS#8 private keys must have version INTEGER
215+
if (keyType === 'private' && version === null) {
216+
throw new InvalidEncodingError('PKCS#8 private key missing version INTEGER.');
217+
}
190218

191-
if (expectedKeyType && keyType !== expectedKeyType) {
192-
throw new InvalidEncodingError('Unexpected key type for PrivateKeyInfo.');
219+
// SPKI public keys must not have version INTEGER
220+
if (keyType === 'public' && version !== null) {
221+
throw new InvalidEncodingError('SPKI public key has unexpected version INTEGER.');
222+
}
223+
224+
// Allow optional trailing fields for PKCS#8 (attributes, publicKey)
225+
const trailingOffset = keyOffset + keyBytesRead;
226+
if (trailingOffset < sequence.length) {
227+
if (keyType === 'private') {
228+
const { hasPublicKey } = scanOptionalTrailingFields(sequence, trailingOffset);
229+
if (hasPublicKey && version !== 1) {
230+
throw new InvalidEncodingError('PKCS#8 publicKey requires version 1.');
231+
}
232+
} else {
233+
throw new InvalidEncodingError('Unexpected trailing data in SPKI.');
234+
}
193235
}
194236

195237
return { oid, keyBytes, keyType };

packages/pq-key-encoder/ts/src/pem.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,12 @@ export function fromPEM(pem: string): KeyData {
5757
const decoded = decodeBase64(body);
5858

5959
if (label === 'PUBLIC KEY' || label === 'PRIVATE KEY') {
60-
return fromDER(decoded);
60+
const key = fromDER(decoded);
61+
const expectedType = label === 'PUBLIC KEY' ? 'public' : 'private';
62+
if (key.type !== expectedType) {
63+
throw new InvalidEncodingError(`PEM label "${label}" does not match key type "${key.type}".`);
64+
}
65+
return key;
6166
}
6267

6368
throw new InvalidEncodingError(`Unsupported PEM label: ${label}.`);

packages/pq-key-encoder/ts/src/pkcs8.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ export function normalizePrivateKeyBytes(alg: KeyData['alg'], keyBytes: Uint8Arr
4848
}
4949

5050
const outer = readTLV(keyBytes);
51+
52+
// RFC 8410-style: single inner OCTET STRING containing raw key
53+
if (
54+
outer.tag === TAG_OCTET_STRING &&
55+
outer.bytesRead === keyBytes.length &&
56+
outer.value.length === expected
57+
) {
58+
return outer.value;
59+
}
60+
61+
// OpenSSL-style: SEQUENCE containing seed + expanded key as OCTET STRINGs
5162
if (outer.tag !== TAG_SEQUENCE || outer.bytesRead !== keyBytes.length) {
5263
return keyBytes;
5364
}

packages/pq-key-encoder/ts/tests/asn1/parse.test.ts

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,19 @@ import { encodeLength } from '../../src/asn1/length';
33
import { parseAlgorithmAndKey, readTLV } from '../../src/asn1/parse';
44
import {
55
encodeBitString,
6+
encodeInteger,
67
encodeNull,
78
encodeObjectIdentifier,
89
encodeOctetString,
910
encodeSequence,
1011
} from '../../src/asn1/primitives';
1112
import { InvalidEncodingError } from '../../src/errors';
1213

14+
function encodeContextSpecific(tag: number, value: Uint8Array): Uint8Array {
15+
const length = encodeLength(value.length);
16+
return Uint8Array.from([tag, ...length, ...value]);
17+
}
18+
1319
describe('asn1 parse', () => {
1420
it('reads TLV with short-form length', () => {
1521
const tlv = new Uint8Array([0x04, 0x03, 0x01, 0x02, 0x03]);
@@ -48,15 +54,96 @@ describe('asn1 parse', () => {
4854
expect(Array.from(parsed.keyBytes)).toEqual([0xde, 0xad, 0xbe, 0xef]);
4955
});
5056

51-
it('extracts OID + key bytes from OCTET STRING', () => {
57+
it('extracts OID + key bytes from OCTET STRING (PKCS8)', () => {
5258
const oid = '1.3.6.1.4.1.2.267.7.6.5';
59+
const version = encodeInteger(0);
5360
const algorithm = encodeSequence([encodeObjectIdentifier(oid), encodeNull()]);
5461
const keyBytes = new Uint8Array([0x00, 0x11, 0x22, 0x33]);
5562
const key = encodeOctetString(keyBytes);
56-
const pkcs8 = encodeSequence([algorithm, key]);
63+
const pkcs8 = encodeSequence([version, algorithm, key]);
5764

5865
const parsed = parseAlgorithmAndKey(pkcs8);
5966
expect(parsed.oid).toBe(oid);
6067
expect(Array.from(parsed.keyBytes)).toEqual([0x00, 0x11, 0x22, 0x33]);
6168
});
69+
70+
it('accepts OneAsymmetricKey with publicKey when version is 1', () => {
71+
const oid = '1.3.6.1.4.1.2.267.7.6.5';
72+
const version = encodeInteger(1);
73+
const algorithm = encodeSequence([encodeObjectIdentifier(oid), encodeNull()]);
74+
const keyBytes = new Uint8Array([0xaa, 0xbb, 0xcc, 0xdd]);
75+
const privateKey = encodeOctetString(keyBytes);
76+
const publicKey = encodeBitString(new Uint8Array([0x01, 0x02, 0x03]));
77+
const publicKeyField = encodeContextSpecific(0xa1, publicKey);
78+
const pkcs8 = encodeSequence([version, algorithm, privateKey, publicKeyField]);
79+
80+
const parsed = parseAlgorithmAndKey(pkcs8);
81+
expect(parsed.oid).toBe(oid);
82+
expect(Array.from(parsed.keyBytes)).toEqual([0xaa, 0xbb, 0xcc, 0xdd]);
83+
});
84+
85+
it('rejects publicKey field when version is 0', () => {
86+
const oid = '1.3.6.1.4.1.2.267.7.6.5';
87+
const version = encodeInteger(0);
88+
const algorithm = encodeSequence([encodeObjectIdentifier(oid), encodeNull()]);
89+
const keyBytes = new Uint8Array([0x10, 0x20, 0x30, 0x40]);
90+
const privateKey = encodeOctetString(keyBytes);
91+
const publicKey = encodeBitString(new Uint8Array([0x09, 0x08, 0x07]));
92+
const publicKeyField = encodeContextSpecific(0xa1, publicKey);
93+
const pkcs8 = encodeSequence([version, algorithm, privateKey, publicKeyField]);
94+
95+
expect(() => parseAlgorithmAndKey(pkcs8)).toThrow(InvalidEncodingError);
96+
});
97+
98+
it('accepts AlgorithmIdentifier with absent parameters', () => {
99+
const oid = '1.3.6.1.4.1.2.267.7.4.4';
100+
const algorithm = encodeSequence([encodeObjectIdentifier(oid)]); // No NULL
101+
const keyBytes = new Uint8Array([0x11, 0x22, 0x33, 0x44]);
102+
const key = encodeBitString(keyBytes);
103+
const spki = encodeSequence([algorithm, key]);
104+
105+
const parsed = parseAlgorithmAndKey(spki);
106+
expect(parsed.oid).toBe(oid);
107+
expect(Array.from(parsed.keyBytes)).toEqual([0x11, 0x22, 0x33, 0x44]);
108+
});
109+
110+
it('accepts AlgorithmIdentifier with explicit NULL parameters', () => {
111+
const oid = '1.3.6.1.4.1.2.267.7.4.4';
112+
const algorithm = encodeSequence([encodeObjectIdentifier(oid), encodeNull()]);
113+
const keyBytes = new Uint8Array([0x55, 0x66, 0x77, 0x88]);
114+
const key = encodeBitString(keyBytes);
115+
const spki = encodeSequence([algorithm, key]);
116+
117+
const parsed = parseAlgorithmAndKey(spki);
118+
expect(parsed.oid).toBe(oid);
119+
expect(Array.from(parsed.keyBytes)).toEqual([0x55, 0x66, 0x77, 0x88]);
120+
});
121+
122+
it('rejects AlgorithmIdentifier with non-NULL parameters', () => {
123+
const oid = '1.3.6.1.4.1.2.267.7.4.4';
124+
// Use OCTET STRING as unsupported parameter type
125+
const badParams = encodeOctetString(new Uint8Array([0x01, 0x02]));
126+
const algorithm = encodeSequence([encodeObjectIdentifier(oid), badParams]);
127+
const keyBytes = new Uint8Array([0xaa, 0xbb, 0xcc, 0xdd]);
128+
const key = encodeBitString(keyBytes);
129+
const spki = encodeSequence([algorithm, key]);
130+
131+
expect(() => parseAlgorithmAndKey(spki)).toThrow(InvalidEncodingError);
132+
expect(() => parseAlgorithmAndKey(spki)).toThrow('Unsupported AlgorithmIdentifier parameters');
133+
});
134+
135+
it('rejects AlgorithmIdentifier with trailing data after NULL', () => {
136+
const oid = '1.3.6.1.4.1.2.267.7.4.4';
137+
// Manually construct AlgorithmIdentifier with NULL + extra bytes
138+
const oidEncoded = encodeObjectIdentifier(oid);
139+
const nullEncoded = encodeNull();
140+
const extraBytes = new Uint8Array([0x00, 0x00]);
141+
const algorithmValue = new Uint8Array([...oidEncoded, ...nullEncoded, ...extraBytes]);
142+
const algorithm = encodeSequence([algorithmValue]);
143+
const keyBytes = new Uint8Array([0x12, 0x34, 0x56, 0x78]);
144+
const key = encodeBitString(keyBytes);
145+
const spki = encodeSequence([algorithm, key]);
146+
147+
expect(() => parseAlgorithmAndKey(spki)).toThrow(InvalidEncodingError);
148+
});
62149
});

packages/pq-key-encoder/ts/tests/pem.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, it } from 'bun:test';
2+
import { InvalidEncodingError } from '../src/errors';
23
import * as api from '../src/index';
34
import { toPKCS8 } from '../src/pkcs8';
45
import { toSPKI } from '../src/spki';
@@ -94,4 +95,14 @@ describe('pem', () => {
9495
bytes: key,
9596
});
9697
});
98+
99+
it('rejects PEM when label does not match key type', () => {
100+
const key = makeKey(1632, 5);
101+
const pkcs8 = toPKCS8({ alg: 'ML-KEM-512', type: 'private', bytes: key });
102+
const encoded = encodeBase64(pkcs8);
103+
const pem = ['-----BEGIN PUBLIC KEY-----', encoded, '-----END PUBLIC KEY-----'].join('\n');
104+
105+
const { fromPEM } = api as unknown as PemApi;
106+
expect(() => fromPEM(pem)).toThrow(InvalidEncodingError);
107+
});
97108
});

packages/pq-key-encoder/ts/tests/pkcs8.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,23 @@ describe('pkcs8', () => {
8686
bytes: rawKey,
8787
});
8888
});
89+
90+
it('extracts raw key from RFC 8410-style inner OCTET STRING', () => {
91+
// RFC 8410 style: privateKey = OCTET STRING { OCTET STRING { raw_bytes } }
92+
const rawKey = makeKey(1632, 9);
93+
const algorithm = encodeAlgorithmIdentifier('ML-KEM-512');
94+
// Double-wrapped: outer OCTET STRING contains DER of inner OCTET STRING
95+
const innerOctetString = encodeOctetString(rawKey);
96+
const pkcs8 = encodeSequence([
97+
encodeInteger(0),
98+
algorithm,
99+
encodeOctetString(innerOctetString),
100+
]);
101+
102+
expect(fromPKCS8(pkcs8)).toEqual({
103+
alg: 'ML-KEM-512',
104+
type: 'private',
105+
bytes: rawKey,
106+
});
107+
});
89108
});

packages/pq-key-encoder/ts/tests/validation.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, it } from 'bun:test';
22
import {
33
encodeBitString,
4+
encodeInteger,
45
encodeNull,
56
encodeObjectIdentifier,
67
encodeOctetString,
@@ -56,9 +57,10 @@ function makeSpki(oid: string, keyBytes: Uint8Array): Uint8Array {
5657
}
5758

5859
function makePkcs8(oid: string, keyBytes: Uint8Array): Uint8Array {
60+
const version = encodeInteger(0);
5961
const algorithm = encodeSequence([encodeObjectIdentifier(oid), encodeNull()]);
6062
const privateKey = encodeOctetString(keyBytes);
61-
return encodeSequence([algorithm, privateKey]);
63+
return encodeSequence([version, algorithm, privateKey]);
6264
}
6365

6466
describe('validation', () => {

0 commit comments

Comments
 (0)