Repository navigation
Conversation
| /// ``` | ||
| @usableFromInline | ||
| struct CMSEncapsulatedContentInfo: DERImplicitlyTaggable, BERImplicitlyTaggable, Hashable, Sendable { | ||
| public struct CMSEncapsulatedContentInfo: DERImplicitlyTaggable, BERImplicitlyTaggable, Hashable, Sendable { |
There was a problem hiding this comment.
Let's keep this type behind the same SPI as the rest of the CMS stuff.
There was a problem hiding this comment.
This change is not present in the latest version of this PR.
| struct CMSVersion: RawRepresentable, Hashable, Sendable { | ||
| @usableFromInline | ||
| var rawValue: Int | ||
| public struct CMSVersion: RawRepresentable, Hashable, Sendable { |
There was a problem hiding this comment.
Let's keep this behind the same SPI as the rest of the CMS stuff.
There was a problem hiding this comment.
This change is not present in the latest version of this PR.
|
|
||
| @usableFromInline | ||
| var parameters: ASN1Any? | ||
| public private(set) var parameters: ASN1Any? |
There was a problem hiding this comment.
I don't think there's any particular reason to make these private(set).
There was a problem hiding this comment.
Changed to just plain public in latest version of this PR.
| @usableFromInline | ||
| @available(macOS 10.15, iOS 13, watchOS 6, tvOS 13, macCatalyst 13, visionOS 1.0, *) | ||
| enum Digest: Sendable { | ||
| public enum Digest: Sendable { |
There was a problem hiding this comment.
Can we avoid making this enum public directly? It makes it very hard to evolve. Preferably we'd wrap it in a struct.
There was a problem hiding this comment.
I see that since my original PR was opened, Digest has gone from an enum to a struct, and the cases you were (presumably) worried about exposing are now hidden in the inner enum type Backing.
Am I correct in assuming that making Digest and the computeDigest(for:using:) function public is no longer a concern given this refactoring?
|
|
||
| @_spi(CMS) | ||
| @inlinable | ||
| public static func createSigningTimeASN1(signingTime: Date) throws -> Data { |
There was a problem hiding this comment.
Is this particular function pulling its weight? It isn't used by anything inside this module, and it does something quite specific, so I'm inclined to want to pull it out of this module and keep it in the code that uses it.
There was a problem hiding this comment.
This change is not present in the latest version of this PR.
|
Can I also ask that we provide some unit tests for new things with public API surface? Just to make sure they're doing what is expected. |
3136a76 to
2ec894c
Compare
|
@Lukasa I've addressed the earlier PR comments & rebased on top of the latest In the meantime, I rebased #288 on top of #307 so the new tests would pass. Once #307 gets merged, I'll rebase this on the latest |
f794749 to
ad593b9
Compare
|
|
||
| @_spi(CMS) | ||
| @inlinable | ||
| public static func getSignedAttributesBytes<Data: DataProtocol>( |
There was a problem hiding this comment.
Is there a reason this needs to be an SPI?
There was a problem hiding this comment.
No technical reason; I will remove that.
There was a problem hiding this comment.
Pushed an update that removes the @_spi(CMS) — note that the signWithTrustedTimestamp function is still marked as SPI. I left that as-is b/c I wasn't sure if that signature was something we were ready to formalize & make available publicly.
In order to make `pkg-signer*` work, we needed to make some changes to `swift-certificates` exposing certain entities publicly and adding trusted timestamp support to CMS.
Motivation:
We had a need to generate CMS structures with an embedded trusted timestamp that indicates the time at which a signing operation was performed, and discovered that this was not supported by the existing Swift Certificates implementation.
Modifications:
To address this need, we adapted existing functionality, adding several functions allowing the signing time to be embedded in a CMS structure.
In order to make this work, we also needed to add public access to several entities in the code.
Result:
This new CMS SPI allows Swift Certificates to be used for generating trusted timestamp signatures from CMS structures that embed the signing time, such as those for codesigning.
We have been using this exact same code in a high-volume production environment for several months, and can confirm that the CMS structures generated by this are usable for creating valid signatures.