Skip to content

Commit 811cd63

Browse files
emlunemlun
committed
Return offsets instead of byte array copies during DER parsing
1 parent efa4b00 commit 811cd63

File tree

2 files changed

+55
-43
lines changed

2 files changed

+55
-43
lines changed

‎yubico-util/src/main/java/com/yubico/internal/util/BinaryUtil.java‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -261,8 +261,8 @@ private static class ParseDerAnyResult {
261261
DerTagClass tagClass;
262262
boolean constructed;
263263
byte tagValue;
264-
byte[] content;
265-
int nextOffset;
264+
int valueStart;
265+
int valueEnd;
266266
}
267267

268268
@Value
@@ -342,14 +342,15 @@ private static ParseDerAnyResult parseDerAny(@NonNull byte[] der, int offset) {
342342
DerTagClass.parse(tag),
343343
(tag & 0x20) != 0,
344344
(byte) (tag & 0x1f),
345-
Arrays.copyOfRange(der, contentLen.nextOffset, contentEnd),
345+
contentLen.nextOffset,
346346
contentEnd);
347347
}
348348
}
349349

350350
/**
351-
* Parse a DER header with the given tag value, constructed bit and tag class, and return a copy
352-
* of the value octets. If any of the three criteria do not match, return empty instead.
351+
* Parse a DER header with the given tag value, constructed bit and tag class, and return the
352+
* start and end offsets of the value octets. If any of the three criteria do not match, return
353+
* empty instead.
353354
*
354355
* @param der DER source to read from.
355356
* @param offset The offset in <code>der</code> from which to start reading.
@@ -359,11 +360,12 @@ private static ParseDerAnyResult parseDerAny(@NonNull byte[] der, int offset) {
359360
* bit) of the tag octet.
360361
* @param expectTagClass The expected tag class. This is the 2 most significant bits of the tag
361362
* octet.
362-
* @return A copy of the value octets, if the parsed tag matches <code>expectTag</code>, <code>
363+
* @return The start and end offsets of the value octets, if the parsed tag matches <code>
364+
* expectTag</code>, <code>
363365
* constructed</code> and <code>expectTagClass</code>, otherwise empty. {@link
364366
* ParseDerResult#nextOffset} is always returned.
365367
*/
366-
public static ParseDerResult<Optional<byte[]>> parseDerTaggedOrSkip(
368+
public static ParseDerResult<Optional<Integer>> parseDerTaggedOrSkip(
367369
@NonNull byte[] der,
368370
int offset,
369371
byte expectTag,
@@ -373,16 +375,16 @@ public static ParseDerResult<Optional<byte[]>> parseDerTaggedOrSkip(
373375
if (result.tagValue == expectTag
374376
&& result.constructed == constructed
375377
&& result.tagClass == expectTagClass) {
376-
return new ParseDerResult<>(Optional.of(result.content), result.nextOffset);
378+
return new ParseDerResult<>(Optional.of(result.valueStart), result.valueEnd);
377379
} else {
378-
return new ParseDerResult<>(Optional.empty(), result.nextOffset);
380+
return new ParseDerResult<>(Optional.empty(), result.valueEnd);
379381
}
380382
}
381383

382384
/**
383-
* Parse a DER header with the given tag value, constructed bit and tag class, and return a copy
384-
* of the value octets. If any of the three criteria do not match, throw an {@link
385-
* IllegalArgumentException}.
385+
* Parse a DER header with the given tag value, constructed bit and tag class, and return the
386+
* start and end offsets of the value octets. If any of the three criteria do not match, throw an
387+
* {@link IllegalArgumentException}.
386388
*
387389
* @param der DER source to read from.
388390
* @param offset The offset in <code>der</code> from which to start reading.
@@ -392,11 +394,12 @@ public static ParseDerResult<Optional<byte[]>> parseDerTaggedOrSkip(
392394
* bit) of the tag octet.
393395
* @param expectTagClass The expected tag class. This is the 2 most significant bits of the tag
394396
* octet.
395-
* @return A copy of the value octets, if the parsed tag matches <code>expectTag</code>, <code>
397+
* @return The start and end offsets of the value octets, if the parsed tag matches <code>
398+
* expectTag</code>, <code>
396399
* constructed</code> and <code>expectTagClass</code>, otherwise empty. {@link
397400
* ParseDerResult#nextOffset} is always returned.
398401
*/
399-
private static ParseDerResult<byte[]> parseDerTagged(
402+
private static ParseDerResult<Integer> parseDerTagged(
400403
@NonNull byte[] der,
401404
int offset,
402405
byte expectTag,
@@ -406,7 +409,7 @@ private static ParseDerResult<byte[]> parseDerTagged(
406409
if (result.tagValue == expectTag) {
407410
if (result.constructed == constructed) {
408411
if (result.tagClass == expectTagClass) {
409-
return new ParseDerResult<>(result.content, result.nextOffset);
412+
return new ParseDerResult<>(result.valueStart, result.valueEnd);
410413
} else {
411414
throw new IllegalArgumentException(
412415
String.format(
@@ -476,16 +479,19 @@ public static <T> ParseDerResult<List<T>> parseDerSequenceContents(
476479
*/
477480
public static <T> ParseDerResult<List<T>> parseDerSequence(
478481
@NonNull byte[] der, int offset, @NonNull ParseDerSequenceElementFunction<T> parseElement) {
479-
final ParseDerResult<byte[]> seq =
482+
final ParseDerResult<Integer> seq =
480483
parseDerTagged(der, offset, (byte) 0x10, true, DerTagClass.UNIVERSAL);
481484
final ParseDerResult<List<T>> res =
482-
parseDerSequenceContents(seq.result, 0, seq.result.length, parseElement);
485+
parseDerSequenceContents(der, seq.result, seq.nextOffset, parseElement);
483486
return new ParseDerResult<>(res.result, seq.nextOffset);
484487
}
485488

486489
/** Parse an Octet String. */
487490
public static ParseDerResult<byte[]> parseDerOctetString(@NonNull byte[] der, int offset) {
488-
return parseDerTagged(der, offset, (byte) 0x04, false, DerTagClass.UNIVERSAL);
491+
ParseDerResult<Integer> res =
492+
parseDerTagged(der, offset, (byte) 0x04, false, DerTagClass.UNIVERSAL);
493+
return new ParseDerResult<>(
494+
Arrays.copyOfRange(der, res.result, res.nextOffset), res.nextOffset);
489495
}
490496

491497
public static byte[] encodeDerObjectId(@NonNull byte[] oid) {

‎yubico-util/src/main/java/com/yubico/internal/util/CertificateParser.java‎

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -208,30 +208,31 @@ public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPoin
208208
(innerSequenceDer, distributionPointChoiceOffset) -> {
209209
// DistributionPoint ::= SEQUENCE {
210210
// distributionPoint [0] DistributionPointName OPTIONAL,
211-
final BinaryUtil.ParseDerResult<Optional<byte[]>> dpElement =
211+
final BinaryUtil.ParseDerResult<Optional<Integer>> dpElementOffsets =
212212
BinaryUtil.parseDerTaggedOrSkip(
213213
innerSequenceDer,
214214
distributionPointChoiceOffset,
215215
(byte) 0,
216216
true,
217217
BinaryUtil.DerTagClass.CONTEXT_SPECIFIC);
218-
if (dpElement.result.isPresent()) {
218+
if (dpElementOffsets.result.isPresent()) {
219219

220220
// DistributionPointName ::= CHOICE {
221221
// fullName [0] GeneralNames,
222-
final BinaryUtil.ParseDerResult<Optional<byte[]>> dpNameElement =
223-
BinaryUtil.parseDerTaggedOrSkip(
224-
dpElement.result.get(),
225-
0,
226-
(byte) 0,
227-
true,
228-
BinaryUtil.DerTagClass.CONTEXT_SPECIFIC);
222+
final BinaryUtil.ParseDerResult<Optional<Integer>>
223+
dpNameElementOffsets =
224+
BinaryUtil.parseDerTaggedOrSkip(
225+
innerSequenceDer,
226+
dpElementOffsets.result.get(),
227+
(byte) 0,
228+
true,
229+
BinaryUtil.DerTagClass.CONTEXT_SPECIFIC);
229230

230-
if (dpNameElement.result.isPresent()) {
231+
if (dpNameElementOffsets.result.isPresent()) {
231232
return BinaryUtil.parseDerSequenceContents(
232-
dpNameElement.result.get(),
233-
0,
234-
dpNameElement.result.get().length,
233+
innerSequenceDer,
234+
dpNameElementOffsets.result.get(),
235+
dpNameElementOffsets.nextOffset,
235236
(generalNamesDer, generalNamesElementOffset) -> {
236237
// fullName [0] GeneralNames,
237238
// GeneralNames ::= SEQUENCE SIZE (1..MAX) OF GeneralName
@@ -243,21 +244,26 @@ public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPoin
243244
// https://datatracker.ietf.org/doc/html/rfc5280#appendix-A.2
244245
// so the SEQUENCE tag in GeneralNames is implicit.
245246
// The IA5String tag is also implicit from the CHOICE tag.
246-
final BinaryUtil.ParseDerResult<Optional<byte[]>> generalName =
247-
BinaryUtil.parseDerTaggedOrSkip(
248-
generalNamesDer,
249-
generalNamesElementOffset,
250-
(byte) 6,
251-
false,
252-
BinaryUtil.DerTagClass.CONTEXT_SPECIFIC);
253-
if (generalName.result.isPresent()) {
247+
final BinaryUtil.ParseDerResult<Optional<Integer>>
248+
generalNameOffsets =
249+
BinaryUtil.parseDerTaggedOrSkip(
250+
generalNamesDer,
251+
generalNamesElementOffset,
252+
(byte) 6,
253+
false,
254+
BinaryUtil.DerTagClass.CONTEXT_SPECIFIC);
255+
if (generalNameOffsets.result.isPresent()) {
254256
String uriString =
255257
new String(
256-
generalName.result.get(), StandardCharsets.US_ASCII);
258+
Arrays.copyOfRange(
259+
generalNamesDer,
260+
generalNameOffsets.result.get(),
261+
generalNameOffsets.nextOffset),
262+
StandardCharsets.US_ASCII);
257263
try {
258264
return new BinaryUtil.ParseDerResult<>(
259265
Optional.of(new URL(uriString)),
260-
generalName.nextOffset);
266+
generalNameOffsets.nextOffset);
261267
} catch (MalformedURLException e) {
262268
throw new IllegalArgumentException(
263269
String.format(
@@ -267,15 +273,15 @@ public static ParseCrlDistributionPointsExtensionResult parseCrlDistributionPoin
267273
}
268274
} else {
269275
return new BinaryUtil.ParseDerResult<>(
270-
Optional.empty(), generalName.nextOffset);
276+
Optional.empty(), generalNameOffsets.nextOffset);
271277
}
272278
});
273279
}
274280
}
275281

276282
// Ignore all other forms of distribution points
277283
return new BinaryUtil.ParseDerResult<>(
278-
Collections.emptyList(), dpElement.nextOffset);
284+
Collections.emptyList(), dpElementOffsets.nextOffset);
279285
}));
280286

281287
return distributionPoints.result.stream()

0 commit comments

Comments
 (0)