Skip to content

Time-Stamp Protocol (RFC 3161)#10778

Open
SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:time_stamp_protocol
Open

Time-Stamp Protocol (RFC 3161)#10778
SparkiDev wants to merge 1 commit into
wolfSSL:masterfrom
SparkiDev:time_stamp_protocol

Conversation

@SparkiDev

Copy link
Copy Markdown
Contributor

Description

Implementation in wolfCrypt
OpenSSL compatibility layer in wolfSSL
Added tests, certificates, examples.

Testing

Different configuration with --enable-tsp.

@SparkiDev SparkiDev self-assigned this Jun 25, 2026
@SparkiDev SparkiDev force-pushed the time_stamp_protocol branch from 76af38a to dc97de1 Compare June 25, 2026 12:23
@github-actions

github-actions Bot commented Jun 25, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m0plus

  • FLASH: .text +40 B (+0.1%, 63,535 B / 262,144 B, total: 24% used)

gcc-arm-cortex-m3

  • FLASH: .text +32 B (+0.0%, 121,441 B / 262,144 B, total: 46% used)

gcc-arm-cortex-m4

  • FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +58 B, .text +64 B (+0.1%, 199,178 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m4-baremetal

  • FLASH: .text +64 B (+0.1%, 66,123 B / 262,144 B, total: 25% used)

gcc-arm-cortex-m4-crypto-only

  • FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +58 B (+0.0%, 173,736 B / 262,144 B, total: 66% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text +64 B (+0.0%, 179,864 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .rodata +56 B, .text +128 B (+0.0%, 768,316 B / 1,048,576 B, total: 73% used)

gcc-arm-cortex-m4-pkcs7

  • FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +58 B, .text +64 B (+0.1%, 211,499 B / 262,144 B, total: 81% used)

gcc-arm-cortex-m4-pq

  • FLASH: .rodata +68 B, .text +64 B (+0.0%, 278,068 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .rodata +64 B, .text +64 B (+0.0%, 323,600 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +58 B (+0.0%, 234,812 B / 262,144 B, total: 90% used)

gcc-arm-cortex-m7

  • FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +58 B (+0.0%, 199,114 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m7-pq

  • FLASH: .rodata +68 B, .text +64 B (+0.0%, 278,644 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .rodata.CSWTCH.1 +4 B, .rodata.str1.1 +58 B, .text +64 B (+0.1%, 234,876 B / 262,144 B, total: 90% used)

linuxkm-standard

  • Data: __patchable_function_entries +8 B (+0.0%, 46,016 B)

stm32-sim-stm32h753

@SparkiDev SparkiDev force-pushed the time_stamp_protocol branch 3 times, most recently from 2a0657a to 0d3affa Compare June 25, 2026 21:51
@SparkiDev

Copy link
Copy Markdown
Contributor Author

Jenkins: retest this please

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10778

Scan targets checked: wolfcrypt-bugs, wolfcrypt-port-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 2
1 finding(s) posted as inline comments (see file-level comments below)

Low (1)

asn_orig.c DecodeExtKeyUsage skips counting unrecognized OIDs

File: wolfcrypt/src/asn_orig.c:3959
Function: DecodeExtKeyUsage
Category: Copy-paste errors

The original-parser DecodeExtKeyUsage does continue on ASN_UNKNOWN_OID_E, bypassing the extExtKeyUsageOidCnt increment, so unknown KeyPurposeIds are not counted — contradicting its own comment and diverging from the template version in asn.c which does count them. A cert with timeStamping plus an extra unknown EKU would report count 1 and wrongly pass Tsp_CheckSignerCert's extExtKeyUsageOidCnt != 1 gate.

Recommendation: Count the OID before continue (or increment in the unknown-OID branch) so the original parser matches the template version's count semantics.

Referenced code: wolfcrypt/src/asn_orig.c:3959-3965 (7 lines)


This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread wolfcrypt/src/tsp.c
@SparkiDev SparkiDev force-pushed the time_stamp_protocol branch from 0d3affa to 0b151dc Compare June 26, 2026 07:21
Comment thread wolfcrypt/src/tsp.c
@SparkiDev

Copy link
Copy Markdown
Contributor Author

DecodeExtKeyUsage fixed

@SparkiDev

Copy link
Copy Markdown
Contributor Author

Jenkins: retest this please

Visual Studio Build: CRL script
generic aborted

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #10778

Scan targets checked: wolfcrypt-bugs, wolfcrypt-port-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src

No new issues found in the changed files. ✅

@dgarske dgarske self-requested a review June 29, 2026 18:43

@dgarske dgarske left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skoll Multi-Scan Review

Modes: review + review-securityOverall recommendation: COMMENT
Findings: 4 total — 4 posted, 0 skipped
4 finding(s) posted as inline comments (see file-level comments below)

Posted findings

  • [Low] [review-security] wolfssl_asn1_integer_new_buf() builds a negative-looking DER INTEGER when the value's MSB is setsrc/ssl_asn1.c:1186-1206
  • [Low] [review] Possible shift-by-32 (undefined behavior) when decoding an empty PKIFailureInfo BIT STRINGwolfcrypt/src/asn_tsp.c:wc_TspResponse_Decode
  • [Low] [review] ssl_tsp.c depends on ssl_asn1.c but the latter is conditionally excluded by OPENSSL_EXTRA_NO_ASN1src/ssl.c:453-459
  • [Low] [review+review-security] Large 2048-byte on-stack request buffer in new responder path does not use WOLFSSL_SMALL_STACK patternsrc/ssl_tsp.c:2243

Review generated by Skoll

Comment thread src/ssl_asn1.c
* @return ASN.1 INTEGER object on success.
* @return NULL on failure.
*/
static WOLFSSL_ASN1_INTEGER* wolfssl_asn1_integer_new_buf(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] wolfssl_asn1_integer_new_buf() builds a negative-looking DER INTEGER when the value's MSB is set · Logic

This new helper backs the OpenSSL-compat getters wolfSSL_TS_REQ_get_nonce(), wolfSSL_TS_TST_INFO_get_serial(), and wolfSSL_TS_TST_INFO_get_nonce(). It builds the ASN1_INTEGER's data buffer as [ASN_INTEGER][len][val...] copied straight from the wc magnitude bytes, and leaves a->negative = 0. The wc layer strips leading 0x00 pad bytes when decoding INTEGERs (asn.c zeroPadded path), so a positive serial/nonce whose first byte is >= 0x80 (common for random serials and the test nonce 0xc3..., serial 0x9a...) is re-wrapped into a DER INTEGER whose first content octet has the high bit set. By DER that encodes a NEGATIVE integer, yet a->negative stays 0 - an internally inconsistent object. Any conformant consumer (i2d_ASN1_INTEGER round-trip, ASN1_INTEGER_cmp, ASN1_INTEGER_get/BN conversion) would interpret it as negative or mismatch. The canonical builder wolfSSL_ASN1_INTEGER_set() in the same file correctly prepends a 0x00 pad in exactly this case, confirming the omission is a defect. No memory-safety or trust impact: these getters are inspection-only views and are not used in the TSP verification/trust path (which operates on the wc structures directly), and wolfSSL_i2d_TS_REQ re-encodes from the wc request, not from this view. The existing tests pass only because they compare raw bytes (num->data + 2) rather than exercising the numeric ASN1_INTEGER API on a high-bit value.

Fix: Mirror wolfSSL_ASN1_INTEGER_set(): compute pad = ((len > 0) && (val[0] & 0x80)) ? 1 : 0, size/require the buffer for len+pad+hdr, encode SetLength(len+pad, ...), write the 0x00 pad byte (if any) then the value, and keep a->negative = 0. This yields a self-consistent positive DER INTEGER matching OpenSSL output.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread wolfcrypt/src/asn_tsp.c
/* failInfo is optional. Length includes unused bits byte.
* Shift up by the number of bytes not encoded - bit 0 of the named
* bit string is the most significant bit of 32. */
if (dataASN[TSPRESPASN_IDX_STAT_FAIL].tag != 0) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Possible shift-by-32 (undefined behavior) when decoding an empty PKIFailureInfo BIT STRING · Undefined Behavior

The failInfo BIT STRING is read into a word32 and then left-aligned with resp->failInfo <<= (8 * (5 - dataASN[TSPRESPASN_IDX_STAT_FAIL].length)). The design relies on .length being the full BIT STRING content length (unused-bits byte + data bytes), so length ranges 1..5. A validly-framed but empty BIT STRING (03 01 00, length == 1) is accepted (tag != 0), giving a shift count of 8 * (5 - 1) == 32. Shifting a 32-bit value by 32 is undefined behavior in C (and would be flagged by UBSan). In practice the value is 0 so the result is benign on most targets, but it is still UB on a reachable, attacker-influenced decode path. This edge (present-but-empty failInfo) is not covered by the new decode tests in test_wc_TspResponse_Decode.

Fix: Guard the shift so the count stays in 0..24 (e.g. only shift when length is in [2,5], or reject an empty/over-long failInfo BIT STRING). Add a decode test for a present-but-empty 03 01 00 failInfo. Suggested fix:
if ((dataASN[TSPRESPASN_IDX_STAT_FAIL].tag != 0) &&
(dataASN[TSPRESPASN_IDX_STAT_FAIL].length >= 2) &&
(dataASN[TSPRESPASN_IDX_STAT_FAIL].length <= 5)) {
resp->failInfo <<=
(8 * (5 - dataASN[TSPRESPASN_IDX_STAT_FAIL].length));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/ssl.c
#endif /* OPENSSL_EXTRA_NO_ASN1 */

#define WOLFSSL_SSL_TSP_INCLUDED
#include "src/ssl_tsp.c"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] ssl_tsp.c depends on ssl_asn1.c but the latter is conditionally excluded by OPENSSL_EXTRA_NO_ASN1 · Build/Portability

src/ssl_tsp.c is included unconditionally, while src/ssl_asn1.c is included only #ifndef OPENSSL_EXTRA_NO_ASN1. ssl_tsp.c calls the file-static helper wolfssl_asn1_integer_new_buf() defined in ssl_asn1.c (and many other ASN.1 compat helpers). A build with OPENSSL_EXTRA + WOLFSSL_TSP + HAVE_PKCS7 + WOLFSSL_TSP_VERIFIER + OPENSSL_EXTRA_NO_ASN1 would activate the ssl_tsp.c body while ssl_asn1.c is omitted, producing an implicit-declaration/undefined-symbol build failure. This is a narrow, likely-unused configuration, but there is no guard preventing it.

Fix: Either add a compile-time guard that documents/forbids the unsupported combination, or gate the TSP OpenSSL-layer guards in ts.h/ssl_tsp.c on !defined(OPENSSL_EXTRA_NO_ASN1). Suggested guard:
#if defined(WOLFSSL_TSP) && defined(OPENSSL_EXTRA) &&
defined(OPENSSL_EXTRA_NO_ASN1)
#error "TSP OpenSSL compat layer requires the OpenSSL ASN.1 API (OPENSSL_EXTRA_NO_ASN1 not supported with WOLFSSL_TSP)"
#endif

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/ssl_tsp.c Outdated
#endif
const byte* genTimePtr = NULL;
word32 genTimeSz = 0;
byte reqDer[2048];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [Low] Large 2048-byte on-stack request buffer in new responder path does not use WOLFSSL_SMALL_STACK pattern · Security

wolfSSL_TS_RESP_create_response() declares byte reqDer[2048] on the stack (plus a WC_RNG, TspRequest, TspTstInfo, TspResponse). The wolfSSL coding convention is that stack buffers larger than ~100 bytes should use the WOLFSSL_SMALL_STACK heap-allocation pattern so the code remains usable on constrained/embedded targets and under -DWOLFSSL_SMALL_STACK. The companion examples (examples/tsp/*.c) already follow a heap-allocation pattern (TSP_DECL/TSP_ALLOC) for big buffers; the library compat layer should be at least as careful. The buffer size is fixed (not attacker-controlled), so there is no overflow - a request larger than 2048 bytes is read truncated and then fails to decode (returns NULL), which is acceptable responder-side behavior. This is a portability/footprint observation only. Both reviewers flagged this location; the review mode rated it severity Low / action NIT, the review-security mode rated it severity Info - the stricter Low view is kept here. ts_check_data() also uses a 256-byte buf[256], which is above the threshold.

Fix: Wrap reqDer (and optionally buf in ts_check_data) with the WOLFSSL_SMALL_STACK / !WOLFSSL_NO_MALLOC heap-allocation pattern, freeing on all return paths, to match library conventions for >100B stack buffers and keep the responder usable on small-stack targets:
#ifdef WOLFSSL_SMALL_STACK
byte* reqDer = (byte*)XMALLOC(2048, NULL, DYNAMIC_TYPE_TMP_BUFFER);
if (reqDer == NULL)
return NULL;
#else
byte reqDer[2048];
#endif

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment thread src/ssl_tsp.c
@@ -0,0 +1,2370 @@
/* ssl_tsp.c

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure this new file is added to all projects including the Visual Studio one, etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed
ssl_tsp.c and asn_tsp.c are included into other files but tsp.c needed to be added.

@dgarske dgarske removed the request for review from wolfSSL-Bot June 29, 2026 21:01
@SparkiDev SparkiDev force-pushed the time_stamp_protocol branch from 0b151dc to 151e6c8 Compare June 30, 2026 00:53
Implementation in wolfCrypt
OpenSSL compatibility layer in wolfSSL
Added tests, certificates, examples.
@SparkiDev SparkiDev force-pushed the time_stamp_protocol branch from 151e6c8 to 6cec72d Compare June 30, 2026 00:57
@SparkiDev

Copy link
Copy Markdown
Contributor Author

Jenkins: retest this please

@SparkiDev SparkiDev assigned dgarske and unassigned SparkiDev Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants