Skip to content

Commit 56dfe45

Browse files
authored
Bump minimum swift-crypto v2 version to 2.0.1 (#3859)
Motivation: Test and upgrade to the latest swift-crypto [2.0.1](https://github.com/apple/swift-crypto/releases/tag/2.0.1) release, which includes a BoringSSL update: apple/swift-crypto#95 Modifications: Tests that use real certs, which execute the OCSP code path, crash on Linux (`swift:5.5` and `swift:5.5-focal` docker images). (rdar://85280061) The crash happens on L351 of `CertificatePolicy.swift` (`BoringSSLOCSPClient.checkStatus`) because `response`, an `OCSP_RESPONSE` pointer, is `NULL`. It is `NULL` because the logic in BoringSSL's `crypto/asn1/asn1_lib.c` has changed (google/boringssl@ee510f5), so the return value needs to be handled differently in `PackageCollectionsSigningLibc/asn1/a_d2i_fp.c`. I also don't think this change is correct: google/boringssl@efab69b#diff-dc711de78524fcd1edcc461a96ca36b80cac1bf03a67287a9b434f772a6f4e3dR170. - Guard against `NULL` OCSP response in `BoringSSLOCSPClient`. - Update logic in `PackageCollectionsSigningLibc/asn1/a_d2i_fp.c`: - Add `ASN1_get_object_without_inf` and `asn1_get_length_without_inf`, copied from [BoringSSL](https://github.com/google/boringssl/blob/ee510f58895c6a88b59f1b66a94939d08ebdfe5b/crypto/asn1/asn1_lib.c) but reverting the ["0x80 fix"](google/boringssl@efab69b#diff-dc711de78524fcd1edcc461a96ca36b80cac1bf03a67287a9b434f772a6f4e3dR170), such that we use the same logic regardless of swift-crypto version. We can probably remove them later when we switch over to swift-crypto 2.x completely (unless I'm correct the 0x80 fix). - Remove `inf` in `asn1_d2i_read_bio`. - Note that there are `ASN1_get_object_with_inf` and `asn1_get_length_with_inf` too. We can use these instead to keep the same behavior as swift-crypt 2.0.0 and less. I will delete these if reviewers are ok with the current approach.
1 parent 9d40d17 commit 56dfe45

File tree

5 files changed

+293
-23
lines changed

5 files changed

+293
-23
lines changed

Package.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ automatic linking type with `-auto` suffix appended to product's name.
5454
let autoProducts = [swiftPMProduct, swiftPMDataModelProduct]
5555

5656
let useSwiftCryptoV2 = ProcessInfo.processInfo.environment["SWIFTPM_USE_SWIFT_CRYPTO_V2"] != nil
57-
let minimumCryptoVersion: Version = useSwiftCryptoV2 ? "2.0.0" : "1.1.4"
57+
let minimumCryptoVersion: Version = useSwiftCryptoV2 ? "2.0.1" : "1.1.4"
5858
var swiftSettings: [SwiftSetting] = []
5959
if useSwiftCryptoV2 {
6060
swiftSettings.append(.define("CRYPTO_v2"))

Sources/PackageCollectionsSigning/Certificate/CertificatePolicy.swift

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,13 +348,23 @@ private struct BoringSSLOCSPClient {
348348

349349
let response = d2i_OCSP_RESPONSE_bio(bio, nil)
350350
defer { OCSP_RESPONSE_free(response) }
351+
352+
guard let response = response else {
353+
results.append(.failure(OCSPError.responseConversionFailure))
354+
return
355+
}
356+
351357
let basicResp = OCSP_response_get1_basic(response)
352358
defer { OCSP_BASICRESP_free(basicResp) }
359+
360+
guard let basicResp = basicResp else {
361+
results.append(.failure(OCSPError.responseConversionFailure))
362+
return
363+
}
353364

354365
// This is just the OCSP response status, not the certificate's status
355366
guard OCSP_response_status(response) == OCSP_RESPONSE_STATUS_SUCCESSFUL,
356-
CCryptoBoringSSL_OBJ_obj2nid(response?.pointee.responseBytes.pointee.responseType) == NID_id_pkix_OCSP_basic,
357-
let basicRespData = basicResp?.pointee.tbsResponseData.pointee else {
367+
CCryptoBoringSSL_OBJ_obj2nid(response.pointee.responseBytes.pointee.responseType) == NID_id_pkix_OCSP_basic else {
358368
results.append(.failure(OCSPError.badResponse))
359369
return
360370
}
@@ -373,6 +383,7 @@ private struct BoringSSLOCSPClient {
373383
}
374384

375385
// Inspect the OCSP response
386+
let basicRespData = basicResp.pointee.tbsResponseData.pointee
376387
for i in 0 ..< sk_OCSP_SINGLERESP_num(basicRespData.responses) {
377388
guard let singleResp = sk_OCSP_SINGLERESP_value(basicRespData.responses, numericCast(i)),
378389
let certStatus = singleResp.pointee.certStatus else {

Sources/PackageCollectionsSigningLibc/asn1/a_d2i_fp.c

Lines changed: 270 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,76 @@
1717
* https://www.openssl.org/source/license.html
1818
*/
1919

20+
/* Copyright (C) 1995-1998 Eric Young ([email protected])
21+
* All rights reserved.
22+
*
23+
* This package is an SSL implementation written
24+
* by Eric Young ([email protected]).
25+
* The implementation was written so as to conform with Netscapes SSL.
26+
*
27+
* This library is free for commercial and non-commercial use as long as
28+
* the following conditions are aheared to. The following conditions
29+
* apply to all code found in this distribution, be it the RC4, RSA,
30+
* lhash, DES, etc., code; not just the SSL code. The SSL documentation
31+
* included with this distribution is covered by the same copyright terms
32+
* except that the holder is Tim Hudson ([email protected]).
33+
*
34+
* Copyright remains Eric Young's, and as such any Copyright notices in
35+
* the code are not to be removed.
36+
* If this package is used in a product, Eric Young should be given attribution
37+
* as the author of the parts of the library used.
38+
* This can be in the form of a textual message at program startup or
39+
* in documentation (online or textual) provided with the package.
40+
*
41+
* Redistribution and use in source and binary forms, with or without
42+
* modification, are permitted provided that the following conditions
43+
* are met:
44+
* 1. Redistributions of source code must retain the copyright
45+
* notice, this list of conditions and the following disclaimer.
46+
* 2. Redistributions in binary form must reproduce the above copyright
47+
* notice, this list of conditions and the following disclaimer in the
48+
* documentation and/or other materials provided with the distribution.
49+
* 3. All advertising materials mentioning features or use of this software
50+
* must display the following acknowledgement:
51+
* "This product includes cryptographic software written by
52+
* Eric Young ([email protected])"
53+
* The word 'cryptographic' can be left out if the rouines from the library
54+
* being used are not cryptographic related :-).
55+
* 4. If you include any Windows specific code (or a derivative thereof) from
56+
* the apps directory (application code) you must include an acknowledgement:
57+
* "This product includes software written by Tim Hudson ([email protected])"
58+
*
59+
* THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND
60+
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
61+
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
62+
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
63+
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
64+
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
65+
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
66+
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
67+
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
68+
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
69+
* SUCH DAMAGE.
70+
*
71+
* The licence and distribution terms for any publically available version or
72+
* derivative of this code cannot be changed. i.e. this code cannot simply be
73+
* copied and put under another distribution licence
74+
* [including the GNU Public Licence.] */
75+
2076
#include <limits.h>
2177
#include <CCryptoBoringSSL_asn1.h>
2278
#include <CCryptoBoringSSL_buffer.h>
2379
#include <CCryptoBoringSSL_err.h>
2480
#include "CPackageCollectionSigning_asn1.h"
2581

2682
static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb);
83+
static int ASN1_get_object_without_inf(const unsigned char **pp, long *plength,
84+
int *ptag, int *pclass, long omax);
85+
static int asn1_get_length_without_inf(const unsigned char **pp, long *rl, long max);
86+
static int asn1_get_length_with_inf(const unsigned char **pp, int *inf,
87+
long *rl, long max);
88+
static int ASN1_get_object_with_inf(const unsigned char **pp, long *plength,
89+
int *ptag, int *pclass, long omax);
2790

2891
void *ASN1_d2i_bio(void *(*xnew) (void), d2i_of_void *d2i, BIO *in, void **x)
2992
{
@@ -57,7 +120,7 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
57120

58121
const unsigned char *q;
59122
long slen;
60-
int inf, tag, xclass;
123+
int ret, tag, xclass;
61124

62125
b = BUF_MEM_new();
63126
if (b == NULL) {
@@ -87,8 +150,8 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
87150

88151
p = (unsigned char *)&(b->data[off]);
89152
q = p;
90-
inf = ASN1_get_object(&q, &slen, &tag, &xclass, len - off);
91-
if (inf & 0x80) {
153+
ret = ASN1_get_object_without_inf(&q, &slen, &tag, &xclass, len - off);
154+
if (ret & 0x80) {
92155
unsigned long e;
93156

94157
e = ERR_GET_REASON(ERR_peek_error());
@@ -100,14 +163,7 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
100163
i = (int)(q - p); /* header length */
101164
off += i; /* end of data */
102165

103-
if (inf & 1) {
104-
/* no data body so go round again */
105-
if (eos == UINT32_MAX) {
106-
goto err;
107-
}
108-
eos++;
109-
want = HEADER_SIZE;
110-
} else if (eos && (slen == 0) && (tag == V_ASN1_EOC)) {
166+
if (eos && (slen == 0) && (tag == V_ASN1_EOC)) {
111167
/* eos value, so go back and read another header */
112168
eos--;
113169
if (eos == 0)
@@ -175,3 +231,206 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
175231
BUF_MEM_free(b);
176232
return -1;
177233
}
234+
235+
// https://github.com/google/boringssl/blob/ee510f58895c6a88b59f1b66a94939d08ebdfe5b/crypto/asn1/asn1_lib.c
236+
static int ASN1_get_object_without_inf(const unsigned char **pp, long *plength,
237+
int *ptag, int *pclass, long omax)
238+
{
239+
int i, ret;
240+
long l;
241+
const unsigned char *p = *pp;
242+
int tag, xclass;
243+
long max = omax;
244+
245+
if (!max)
246+
goto err;
247+
ret = (*p & V_ASN1_CONSTRUCTED);
248+
xclass = (*p & V_ASN1_PRIVATE);
249+
i = *p & V_ASN1_PRIMITIVE_TAG;
250+
if (i == V_ASN1_PRIMITIVE_TAG) { /* high-tag */
251+
p++;
252+
if (--max == 0)
253+
goto err;
254+
l = 0;
255+
while (*p & 0x80) {
256+
l <<= 7L;
257+
l |= *(p++) & 0x7f;
258+
if (--max == 0)
259+
goto err;
260+
if (l > (INT_MAX >> 7L))
261+
goto err;
262+
}
263+
l <<= 7L;
264+
l |= *(p++) & 0x7f;
265+
tag = (int)l;
266+
if (--max == 0)
267+
goto err;
268+
} else {
269+
tag = i;
270+
p++;
271+
if (--max == 0)
272+
goto err;
273+
}
274+
275+
/* To avoid ambiguity with V_ASN1_NEG, impose a limit on universal tags. */
276+
if (xclass == V_ASN1_UNIVERSAL && tag > V_ASN1_MAX_UNIVERSAL)
277+
goto err;
278+
279+
*ptag = tag;
280+
*pclass = xclass;
281+
if (!asn1_get_length_without_inf(&p, plength, max))
282+
goto err;
283+
284+
if (*plength > (omax - (p - *pp))) {
285+
OPENSSL_PUT_ERROR(ASN1, ASN1_R_TOO_LONG);
286+
/*
287+
* Set this so that even if things are not long enough the values are
288+
* set correctly
289+
*/
290+
ret |= 0x80;
291+
}
292+
*pp = p;
293+
return ret;
294+
err:
295+
OPENSSL_PUT_ERROR(ASN1, ASN1_R_HEADER_TOO_LONG);
296+
return 0x80;
297+
}
298+
299+
static int asn1_get_length_without_inf(const unsigned char **pp, long *rl, long max)
300+
{
301+
const unsigned char *p = *pp;
302+
unsigned long ret = 0;
303+
unsigned long i;
304+
305+
if (max-- < 1) {
306+
return 0;
307+
}
308+
if (*p == 0x80) {
309+
/* We do not support BER indefinite-length encoding. */
310+
return 0;
311+
}
312+
i = *p & 0x7f;
313+
if (*(p++) & 0x80) {
314+
if (i > sizeof(ret) || max < (long)i)
315+
return 0;
316+
while (i-- > 0) {
317+
ret <<= 8L;
318+
ret |= *(p++);
319+
}
320+
} else {
321+
ret = i;
322+
}
323+
/*
324+
* Bound the length to comfortably fit in an int. Lengths in this module
325+
* often switch between int and long without overflow checks.
326+
*/
327+
if (ret > INT_MAX / 2)
328+
return 0;
329+
*pp = p;
330+
*rl = (long)ret;
331+
return 1;
332+
}
333+
334+
// https://github.com/google/boringssl/blob/a7e807481bfab5fcf72cd6b396f57cb2990bf63a/crypto/asn1/asn1_lib.c
335+
static int ASN1_get_object_with_inf(const unsigned char **pp, long *plength,
336+
int *ptag, int *pclass, long omax)
337+
{
338+
int i, ret;
339+
long l;
340+
const unsigned char *p = *pp;
341+
int tag, xclass, inf;
342+
long max = omax;
343+
344+
if (!max)
345+
goto err;
346+
ret = (*p & V_ASN1_CONSTRUCTED);
347+
xclass = (*p & V_ASN1_PRIVATE);
348+
i = *p & V_ASN1_PRIMITIVE_TAG;
349+
if (i == V_ASN1_PRIMITIVE_TAG) { /* high-tag */
350+
p++;
351+
if (--max == 0)
352+
goto err;
353+
l = 0;
354+
while (*p & 0x80) {
355+
l <<= 7L;
356+
l |= *(p++) & 0x7f;
357+
if (--max == 0)
358+
goto err;
359+
if (l > (INT_MAX >> 7L))
360+
goto err;
361+
}
362+
l <<= 7L;
363+
l |= *(p++) & 0x7f;
364+
tag = (int)l;
365+
if (--max == 0)
366+
goto err;
367+
} else {
368+
tag = i;
369+
p++;
370+
if (--max == 0)
371+
goto err;
372+
}
373+
374+
/* To avoid ambiguity with V_ASN1_NEG, impose a limit on universal tags. */
375+
if (xclass == V_ASN1_UNIVERSAL && tag > V_ASN1_MAX_UNIVERSAL)
376+
goto err;
377+
378+
*ptag = tag;
379+
*pclass = xclass;
380+
if (!asn1_get_length_with_inf(&p, &inf, plength, max))
381+
goto err;
382+
383+
if (inf && !(ret & V_ASN1_CONSTRUCTED))
384+
goto err;
385+
386+
if (*plength > (omax - (p - *pp))) {
387+
OPENSSL_PUT_ERROR(ASN1, ASN1_R_TOO_LONG);
388+
/*
389+
* Set this so that even if things are not long enough the values are
390+
* set correctly
391+
*/
392+
ret |= 0x80;
393+
}
394+
*pp = p;
395+
return (ret | inf);
396+
err:
397+
OPENSSL_PUT_ERROR(ASN1, ASN1_R_HEADER_TOO_LONG);
398+
return 0x80;
399+
}
400+
401+
static int asn1_get_length_with_inf(const unsigned char **pp, int *inf,
402+
long *rl, long max)
403+
{
404+
const unsigned char *p = *pp;
405+
unsigned long ret = 0;
406+
unsigned long i;
407+
408+
if (max-- < 1)
409+
return 0;
410+
if (*p == 0x80) {
411+
*inf = 1;
412+
ret = 0;
413+
p++;
414+
} else {
415+
*inf = 0;
416+
i = *p & 0x7f;
417+
if (*(p++) & 0x80) {
418+
if (i > sizeof(ret) || max < (long)i)
419+
return 0;
420+
while (i-- > 0) {
421+
ret <<= 8L;
422+
ret |= *(p++);
423+
}
424+
} else
425+
ret = i;
426+
}
427+
/*
428+
* Bound the length to comfortably fit in an int. Lengths in this module
429+
* often switch between int and long without overflow checks.
430+
*/
431+
if (ret > INT_MAX / 2)
432+
return 0;
433+
*pp = p;
434+
*rl = (long)ret;
435+
return 1;
436+
}

0 commit comments

Comments
 (0)