Add wrappers for Time-Stamp Protocol (RFC 3161) implementation of OpenSSL #1336

krisztian-kovacs · 2020-09-01T08:34:45Z

The aim of these patch set is to cover enough functionality to implement a client interfacing with a time-stamping authority to request and verify timestamps.

This pull request is more of a request to review -- I'm a first time contributor to rust-openssl. It has only been tested with OpenSSL 1.1.0.

sfackler · 2020-09-01T23:59:21Z

CI is unhappy with the use of TryFrom.

krisztian-kovacs · 2020-09-02T05:11:57Z

Indeed, I've seen the CI issue yesterday evening -- will come up with a fix today. Just to confirm: is 1.31 the MSRV rust-openssl is targeting?

krisztian-kovacs · 2020-09-03T10:14:37Z

It took me some time to get the FFI bindings right for all versions of OpenSSL and LibreSSL, but hopefully it's OK now.

krisztian-kovacs · 2020-09-09T11:33:21Z

Does the code look reasonable? Or is there something I could make to improve it?

sfackler · 2020-11-01T18:43:52Z

openssl/src/ts.rs

+/// A builder used to construct a `TsMsgImprint`
+pub struct TsMsgImprintBuilder(TsMsgImprint);
+
+impl TsMsgImprintBuilder {


TS_MSG_IMPRINT isn't reference counted, so there's no need for a separate builder type.

Fixed, thanks!

sfackler · 2020-11-01T18:44:29Z

openssl/src/ts.rs

+    /// Sets the algorithm identifier of the message digest algorithm.
+    ///
+    /// This corresponds to `TS_MSG_IMPRINT_set_algo`.
+    pub fn set_algo(&mut self, digest: &MessageDigest) -> Result<(), ErrorStack> {


It seems like this should take an X509Algorithm rather than a MessageDigest.

Hmm, in that case I think it should be possible to easily construct an X509Algorithm from a MessageDigest. Now there's no public interface for constructing an X509Algorithm at all.
Also, according to RFC 3161 only message digest algorithms are valid here: "The hash algorithm indicated in the hashAlgorithm field SHOULD be a known hash algorithm (one-way and collision resistant)." -- that's why I thought that using a MessageDigest on this interface would be more precise and useful.

Oh, and OpenSSL is inconsistent here: TS_MSG_IMPRINT_set_algo() uses X509_ALGOR, but its counterpart on the server side, TS_RESP_CTX_add_md() takes an EVP_MD. The server-side only accepts imprints with digest algorithms explicitly added via TS_RESP_CTX_add_md().

sfackler · 2020-11-01T18:45:09Z

openssl/src/ts.rs

+
+pub struct TsReqBuilder(TsReq);
+
+impl TsReqBuilder {


Similarly, no need for a builder.

Fixed, thanks!

sfackler · 2020-11-01T18:46:50Z

openssl/src/ts.rs

+    /// Corresponds to `TS_REQ_to_TS_VERIFY_CTX`.
+    pub fn from_req(request: &TsReqRef) -> Result<TsVerifyContext, ErrorStack> {
+        unsafe {
+            ffi::init();


This isn't necessary since a TsReqRef already exists.

Removed, thanks!

sfackler · 2020-11-01T18:47:15Z

openssl/src/ts.rs

+        unsafe {
+            ffi::init();
+            let ctx: *mut ffi::TS_VERIFY_CTX = cvt_p(ffi::TS_VERIFY_CTX_new())?;
+            let ctx = cvt_p(ffi::TS_REQ_to_TS_VERIFY_CTX(request.as_ptr(), ctx))?;


The TS_VERIFY_CTX is leaked if this fails.

Fixed, thanks. It seems there's no point in constructing a verify context and passing it to TS_REQ_to_TS_VERIFY_CTX() anyway and we're better off with passing in NULL.

sfackler · 2020-11-01T18:47:40Z

openssl/src/ts.rs

+
+pub struct TsRespContextBuilder(TsRespContext);
+
+impl TsRespContextBuilder {


Probably same here WRT not needing a builder.

Fixed, thanks for the review!

krisztian-kovacs · 2021-03-24T09:03:04Z

I've rebased these changes to the latest master.
@sfackler do you see any outstanding issues that prevent merging this PR?

These are required by the timestamping code.

This change adds a partial wrapper for the Time-Stamp Protocol (RFC 3161) implementation in OpenSSL. The aim is to have enough coverage to create a client that can interact with a Time Stamp Authority by creating time-stamp requests and verifying responses returned by the authority.

This change adds the missing wrappers to be able to generate a minimally functional Time Stamp Authority, signing time stamp requests and generating time stamp responses.

Kijewski · 2022-01-31T09:46:34Z

Hi, can you tell me what the state of the PR is? I'd like to use this feature in a project of mine. (No rush, of course! :) )

dfintha · 2022-02-10T12:47:34Z

Hi, I'm also interested, when this PR might get merged, it would be pretty useful. :)

JM4ier · 2024-08-03T07:54:02Z

@sfackler does this PR still have work that needs to be done before it can be merged? (apart from the merge conflict I mean)

If so, I'll gladly implement it on a separate PR, as timestamping is a feature I need.

krisztian-kovacs force-pushed the timestamping branch from 43d7e39 to ec2c8db Compare September 1, 2020 14:51

krisztian-kovacs force-pushed the timestamping branch 7 times, most recently from 718c43c to 64f106c Compare September 3, 2020 09:57

krisztian-kovacs force-pushed the timestamping branch from 64f106c to b8afda8 Compare October 14, 2020 08:58

krisztian-kovacs force-pushed the timestamping branch 3 times, most recently from 3ff8821 to af2b0a3 Compare October 29, 2020 12:42

sfackler reviewed Nov 1, 2020

View reviewed changes

krisztian-kovacs force-pushed the timestamping branch 2 times, most recently from aa20354 to 4b67566 Compare November 5, 2020 15:45

krisztian-kovacs force-pushed the timestamping branch from 4b67566 to 99db458 Compare January 5, 2021 13:21

kkovaacs force-pushed the timestamping branch from 99db458 to 5779860 Compare March 24, 2021 08:50

krisztian-kovacs added 3 commits July 27, 2021 13:35

openssl-sys: add wrappers for timestamping functions

c3cd1e8

openssl-sys/x509: add missing functions for X509_ALGOR

29d717b

These are required by the timestamping code.

openssl/ts: add functionality for a basic Time Stamp Authority

32aab77

This change adds the missing wrappers to be able to generate a minimally functional Time Stamp Authority, signing time stamp requests and generating time stamp responses.

kkovaacs force-pushed the timestamping branch from 5779860 to 32aab77 Compare July 27, 2021 11:37

JM4ier mentioned this pull request Aug 29, 2024

Timestamp Protocol Wrapper (RFC 3161) #2286

Open


		pub struct TsRespContextBuilder(TsRespContext);

		impl TsRespContextBuilder {

Uh oh!

Add wrappers for Time-Stamp Protocol (RFC 3161) implementation of OpenSSL #1336

Are you sure you want to change the base?

Add wrappers for Time-Stamp Protocol (RFC 3161) implementation of OpenSSL #1336

Uh oh!

Conversation

krisztian-kovacs commented Sep 1, 2020

Uh oh!

sfackler commented Sep 1, 2020

Uh oh!

krisztian-kovacs commented Sep 2, 2020

Uh oh!

krisztian-kovacs commented Sep 3, 2020

Uh oh!

krisztian-kovacs commented Sep 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krisztian-kovacs commented Mar 24, 2021

Uh oh!

Kijewski commented Jan 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dfintha commented Feb 10, 2022

Uh oh!

JM4ier commented Aug 3, 2024

Uh oh!

Uh oh!

Kijewski commented Jan 31, 2022 •

edited

Loading