Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support SHA256 as OCSP hash algorithm #190

Open
weekly71 opened this issue Sep 5, 2024 · 11 comments
Open

Support SHA256 as OCSP hash algorithm #190

weekly71 opened this issue Sep 5, 2024 · 11 comments

Comments

@weekly71
Copy link

weekly71 commented Sep 5, 2024

After 4 hours of trying to get OCSP check for Apples own(!) certificates to work, I've noticed this comment in sources IMG_20240905_235103_014.jpg
As it turned out, this library does not support any other algorithms else then SHA1.

  • Why isn't there anything about that in the documentation?
  • Will be SHA256 support added anytime soon?
@Lukasa
Copy link
Contributor

Lukasa commented Sep 6, 2024

@dnadoba may have more of the context here, but I believe this is just something we didn't get around to doing. We're not immediately planning to do this work, but we'd be happy to work with someone to produce a pull request to enable the feature, and would also be happy to accept documentation PRs that call out the limitation.

@weekly71
Copy link
Author

weekly71 commented Sep 6, 2024

Is it just not enabled or it doesn't work? Because after forking and forcefully enabling it I still get malformedRequest

@dnadoba
Copy link
Member

dnadoba commented Sep 6, 2024

IIRC the problem is that there is no way of knowing upfront which hash algorithm the server supports. Therefore back then we needed to use SHA1 which is what was the only required hash algorithm by the specification for maximum compatibility.

I don’t recall if I have tested SHA256 with a real sever or not.

Do you have some code snipped to reproduce the behavior you described?

@weekly71
Copy link
Author

weekly71 commented Sep 6, 2024

Do you have some code snipped to reproduce the behavior you described?

`import Foundation
import X509

enum ImportError: Error {
case wrongPassword
case unknownError(status: OSStatus)
}

enum CheckError: Error {
case requestFailed
}

struct ApplePKCS12 {
let name: String
let id: String
let fullName: String

let countryCode: String

let expirationDate: Date

private let certificate: Certificate

public init(pkcs12Data: Data, password: String) throws {
    let passwordOption: NSDictionary = [kSecImportExportPassphrase as String: password]
    var items: CFArray?
    let status = SecPKCS12Import(pkcs12Data as CFData, passwordOption, &items)

    switch status {
    case errSecAuthFailed:
        throw ImportError.wrongPassword
    case errSecSuccess:
        break
    default:
        throw ImportError.unknownError(status: status)
    }

    let pkcs12 = (items! as! [[String: Any]])[0]
    certificate = try Certificate((pkcs12[kSecImportItemCertChain as String] as! [SecCertificate])[0])

    let subject = certificate.subject.map { $0.description }
    name = String(subject[1].dropFirst(2))
    id = String(subject[2].dropFirst(3))
    fullName = String(subject[3].dropFirst(3))
    
    countryCode = String(subject[0].dropFirst(2))

    expirationDate = certificate.notValidAfter
}

public func isSigned() async throws -> Bool  {
    let caCerts = ["AppleWWDRCA", "AppleWWDRCAG2", "AppleWWDRCAG3", "AppleWWDRCAG4", "AppleWWDRCAG5", "AppleWWDRCAG6"]
    var certStore = CertificateStore()
    
    for cert in caCerts {
        let url = URL(string: cert.last == "A"
                      ? "https://developer.apple.com/certificationauthority/AppleWWDRCA.cer"
                      : "https://www.apple.com/certificateauthority/\(cert).cer")!
        let (data, response) = try await URLSession.shared.data(from: url)
        print((response as? HTTPURLResponse)?.statusCode ?? "request failed")
        certStore.append(try Certificate(SecCertificateCreateWithData(nil, data as CFData)!))
        //let verifier = OCSPVerifier(failureMode: .hard, requester: AppleOCSPRequester())
        //let result = try await verifier.validate(certificate: certificate, caCertificate: try Certificate(SecCertificateCreateWithData(nil, data as CFData)!))
    }
    
    var verifier = Verifier(rootCertificates: certStore) {
        OCSPVerifierPolicy(failureMode: OCSPFailureMode.hard, requester: AppleOCSPRequester(), validationTime: Date())
    }
    let result = await verifier.validate(leafCertificate: certificate, intermediates: CertificateStore()) { diag in
        print(diag.multilineDescription)
    }
    switch result {
    case .couldNotValidate:
        return false
    case .validCertificate:
        return true
    }
    //return false
}

struct AppleOCSPRequester: OCSPRequester {
    func query(request: [UInt8], uri: String) async -> X509.OCSPRequesterQueryResult {
        print("ocsp queried!", request, uri)
        do {
            var urlRequest = URLRequest(url: URL(string: uri)!)
            urlRequest.addValue("application/ocsp-request", forHTTPHeaderField: "Content-Type")
            urlRequest.httpMethod = "POST"
            urlRequest.httpBody = Data(request)
            let (data, response) = try await URLSession.shared.data(for: urlRequest)
            return OCSPRequesterQueryResult.response([UInt8](data))
        } catch {
            return OCSPRequesterQueryResult.terminalError(CheckError.requestFailed)
        }
    }
}

}`
for some reason GitHub doesn't let me to select the whole code chunk
What I'm trying to do is to port this class from python, where the OCSP checking works completely fine with same certificate. After some inspection I've found that generated request is slightly different from the Python one.
Here's the certificate I've tested with (its signed) (in zip because GitHub doesn't support p12 uploading)
cert.p12.zip
P.S. I've disabled extensions and ocsp url checks in my fork because for some reason certificates were always failing it, and i needed only ocsp check and nothing else

@weekly71
Copy link
Author

weekly71 commented Sep 6, 2024

After some additional checks, i can say there may be a bug inside OCSP request generation logic. The hash algorithm is same, output issuer name and key hashes as well, and the certificate serial number is also recognising the same way. But i still get malformedRequest in swift and successful response in python. The generated request is 2 bits longer then the one generated with python, and as i already said, has slightly different content.

@Lukasa
Copy link
Contributor

Lukasa commented Sep 6, 2024

Can you print both as base64 here?

@weekly71
Copy link
Author

weekly71 commented Sep 6, 2024

Python (Cryptography package)

Hashing algorithm

cryptography.hazmat.primitives.hashes.SHA256

Issuer key hash

ryzPHhOVi5/Ig8Nj7gMQbDPjP9s3y30l6Uhz/KwzDUA=

Issuer name hash

NW+RocgeFWe7R6bFOXFz5gJcIjp83kGL8TkiWAtNgKo=

Certificate serial number (encoded to hex with Swift Certificate.SerialNumber)

1a:75:bd:a0:1:de:b8:5d:fe:63:28:5d:1d:ca:d:17

Request/Response Headers

Bildschirmfoto 2024-09-06 um 18 33 33

Base64 Request

MG0wazBpMGcwZTANBglghkgBZQMEAgEFAAQgNW+RocgeFWe7R6bFOXFz5gJcIjp83kGL8TkiWAtN\ngKoEIK8szx4TlYufyIPDY+4DEGwz4z/bN8t9JelIc/ysMw1AAhAadb2gAd64Xf5jKF0dyg0X\n

Base64 Response

MIIKYQoBAKCCClowggpWBgkrBgEFBQcwAQEEggpHMIIKQzCBvKIWBBSc0fTAbf+92rNIeMlAO1sW dFx4zhgPMjAyNDA5MDYxNjM4NTNaMIGQMIGNMGUwDQYJYIZIAWUDBAIBBQAEIDVvkaHIHhVnu0em xTlxc+YCXCI6fN5Bi/E5IlgLTYCqBCCvLM8eE5WLn8iDw2PuAxBsM+M/2zfLfSXpSHP8rDMNQAIQ GnW9oAHeuF3+YyhdHcoNF4AAGA8yMDI0MDkwNjE2Mzg1M1qgERgPMjAyNDA5MDcwNDM4NTJaMA0G CSqGSIb3DQEBCwUAA4IBAQCQEYZKkneqTVhnIaSvNtnjbm5qxO06TTDrAu49i031W2e61ke5t2Ic We1Hm+v1VgpkKh2M9evvHFBjLfxL+8SwSFPsrTWAp50yHuowXHS/WdFEzCy1qqe3Qz9YYL+KFh6j S9QMMPJQBYVW+TGyiojugRO70KajS0DaVFLudOnBAGjZsQgXHF7mfhLVpAvI4heoMzXU96kaWUvA 7oZS1DFVgb3qeAaCj47olpokxVqCK1854Qsma1DBuK1I5GGa0ogfcjxDrx9P0q22EYNleEmnCk26 6ohNbdD5Ya6qmk0hir/Y6l902iz+Ko9L6uyRuzKBeuMNaIMSBVoKJ3SpuhCBoIIIbDCCCGgwggQP MIIC96ADAgECAhBtqRKxBcGGqukFqOTb8RaPMA0GCSqGSIb3DQEBCwUAMHUxRDBCBgNVBAMMO0Fw cGxlIFdvcmxkd2lkZSBEZXZlbG9wZXIgUmVsYXRpb25zIENlcnRpZmljYXRpb24gQXV0aG9yaXR5 MQswCQYDVQQLDAJHMzETMBEGA1UECgwKQXBwbGUgSW5jLjELMAkGA1UEBhMCVVMwHhcNMjQwODA4 MTU1NjU0WhcNMjQwOTE5MTU1NjUzWjCBhzFjMGEGA1UEAwxaQXBwbGUgV29ybGR3aWRlIERldmVs b3BlciBSZWxhdGlvbnMgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkgRzMgT0NTUCBSZXNwb25kZXIg UEcxIDIwMjQwODA4MRMwEQYDVQQKDApBcHBsZSBJbmMuMQswCQYDVQQGEwJVUzCCASIwDQYJKoZI hvcNAQEBBQADggEPADCCAQoCggEBALKlEecNqyfyX+2cuJf6LUVq6u1VrV+tn5JS0Qcof2OqqOVv hm0FwTAcryC6DmD7g3XKZYWVPFsV0w02IZsRth0diCpXmGR0euDiRTK1VngFabHu3ocBxM9B83o8 qoi8dWNgMuXlGZ5HFb1lGFHrdTBaloD0FztM+YhhRQAnyIcEaK1zgljgnvwFtFpuX/96kkO4DfyM kB8iDWzLknXZo/FfTx1QcY9feE/QDLo+s19SnO5eJo2IEPilHG/3AeKPsxfZDgbOBseuRUD5okDM WtxGelqcWNG4igA4b9yb8KaLfXIHRaG2Z6oqYuKxpgqQcUlORSD4TZb00XHYKPtj8pcCAwEAAaOB hzCBhDAMBgNVHRMBAf8EAjAAMB8GA1UdIwQYMBaAFAn+wBWQ+a9kCpISuSYoYwyX7KeyMA8GCSsG AQUFBzABBQQCBQAwEwYDVR0lBAwwCgYIKwYBBQUHAwkwHQYDVR0OBBYEFJzR9MBt/73as0h4yUA7 WxZ0XHjOMA4GA1UdDwEB/wQEAwIHgDANBgkqhkiG9w0BAQsFAAOCAQEAXyBh6x+9ilPj9zrg6a8l 2BsrBA9mGmkM7b5Lglz5OwZh+M2L2GcydYE4j0JOJVNBDwEOKFolUWSv5DSUgfUPQNkqFEKkfreL EHcTpilm4xqQIHZtbaQdxK1/Q1542551XMzvMprrG3m+y0Eo/GCgvIXhT4a8RZqGkoQjQy7CG4/T irzFkAxtenKcc4+IZuuURQcP7YeNpvSbhFOcGioV2YpAKsHj1Iv3N2yXXJVNWa5fz08DLylM82P5 V1D9ICbzAaRIgnRBrbdgO3iati6kqGcygcRdjQXsl9nHkIVvINSJoCcxsVz+br6yX4hWlBeafN6c gPq4nmbsAh8eFOFpNjCCBFEwggM5oAMCAQICEHyvaQoltzn+e5tEesF4xe4wDQYJKoZIhvcNAQEL BQAwYjELMAkGA1UEBhMCVVMxEzARBgNVBAoTCkFwcGxlIEluYy4xJjAkBgNVBAsTHUFwcGxlIENl cnRpZmljYXRpb24gQXV0aG9yaXR5MRYwFAYDVQQDEw1BcHBsZSBSb290IENBMB4XDTIwMDIxOTE4 MTM0N1oXDTMwMDIyMDAwMDAwMFowdTFEMEIGA1UEAww7QXBwbGUgV29ybGR3aWRlIERldmVsb3Bl ciBSZWxhdGlvbnMgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkxCzAJBgNVBAsMAkczMRMwEQYDVQQK DApBcHBsZSBJbmMuMQswCQYDVQQGEwJVUzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB ANj1ifyoWQuHx0yRLi1WkNN4HaQw6aVI7wtDvy2E+yRdRRvl61k2ElxUZJ5sYi20xpfKzDz21gaR /OUCphxqtHnVZ8us6T8HEeGEvEcdZo45oui0Se060uEQYHqOk4zKwNoMwIPQ4/mR1qeMyMFz4q5G 0WKdkqiQYDd9aJbNjeD8m/p4u+N7ry0X3Vv+gFkjdRe4EsHtG+U0zv4itZ5JsOVVv4+EojyousWG lRbkAMmfA9DMszoWe9YFXJ3rR64NtS2MDmlNQNBLTDsEno0hsawuQ1QwzsGKhpSYS99qDT/++xyu lxfCeAow4F8fVDtJtyUAGjeC0laXpVJJfGA4m+8CAwEAAaOB7zCB7DASBgNVHRMBAf8ECDAGAQH/ AgEAMB8GA1UdIwQYMBaAFCvQaUeUdgn+9GuNLkCm90dNfwheMEQGCCsGAQUFBwEBBDgwNjA0Bggr BgEFBQcwAYYoaHR0cDovL29jc3AuYXBwbGUuY29tL29jc3AwMy1hcHBsZXJvb3RjYTAuBgNVHR8E JzAlMCOgIaAfhh1odHRwOi8vY3JsLmFwcGxlLmNvbS9yb290LmNybDAdBgNVHQ4EFgQUCf7AFZD5 r2QKkhK5JihjDJfsp7IwDgYDVR0PAQH/BAQDAgEGMBAGCiqGSIb3Y2QGAgEEAgUAMA0GCSqGSIb3 DQEBCwUAA4IBAQCtZRPo9uCBd0QCR0K+X6U5IOpiqf3FkMlzE9Wem9CqD42Ny+0Bz2woQFvHVSRB +PzPwbUj6dzs8W/KgB13wsRhSSVnrw/KOSWt0+N6zDMoDQ4uoVdAc/rmXK4GUSnt44UMT2HcMhaL d9BEyl1yAzFGnK6bQBr69ODTPvovjGafl8RUWe/SSPQHmUlgWRnH3ZTRwMFsf3gh7wzrO2yZgktS YDi1OYJt7FIxU74PkUxJSXSPplHLhEdOHXUnbr350lzzf8JsCwk24mTkwjcDFBnV6mqUqqnb/vaa CGeL7yu4qhdJg6/Py7zpz+qVcbC0RaLM5YeqCsNBOnlc2lA0nZU7

Swift

Hashing algorithm

sha256

Issuer key hash

ryzPHhOVi5/Ig8Nj7gMQbDPjP9s3y30l6Uhz/KwzDUA=

Issuer name hash

NW+RocgeFWe7R6bFOXFz5gJcIjp83kGL8TkiWAtNgKo=

Certificate serial number

1a:75:bd:a0:1:de:b8:5d:fe:63:28:5d:1d:ca:d:17

Request/Response Headers

Bildschirmfoto 2024-09-06 um 18 42 59

Base64 Request

MG8wbTBnMGUwYzALBglghkgBZQMEAgEEIDVvkaHIHhVnu0emxTlxc+YCXCI6fN5Bi/E5IlgLTYCqBCCvLM8eE5WLn8iDw2PuAxBsM+M/2zfLfSXpSHP8rDMNQAIQGnW9oAHeuF3+YyhdHcoNF6ICMAA=

Base64 Response

MAMKAQE=

All values are copied directly from logs (and hashes are in base64)

@Lukasa
Copy link
Contributor

Lukasa commented Sep 16, 2024

Python request, decoded:

    0:d=0  hl=2 l= 109 cons: SEQUENCE          
    2:d=1  hl=2 l= 107 cons: SEQUENCE          
    4:d=2  hl=2 l= 105 cons: SEQUENCE          
    6:d=3  hl=2 l= 103 cons: SEQUENCE          
    8:d=4  hl=2 l= 101 cons: SEQUENCE          
   10:d=5  hl=2 l=  13 cons: SEQUENCE          
   12:d=6  hl=2 l=   9 prim: OBJECT            :sha256
   23:d=6  hl=2 l=   0 prim: NULL              
   25:d=5  hl=2 l=  32 prim: OCTET STRING      [HEX DUMP]:356F91A1C81E1567BB47A6C5397173E6025C223A7CDE418BF13922580B4D80AA
   59:d=5  hl=2 l=  32 prim: OCTET STRING      [HEX DUMP]:AF2CCF1E13958B9FC883C363EE03106C33E33FDB37CB7D25E94873FCAC330D40
   93:d=5  hl=2 l=  16 prim: INTEGER           :1A75BDA001DEB85DFE63285D1DCA0D17

Swift request, decoded:

    0:d=0  hl=2 l= 111 cons: SEQUENCE          
    2:d=1  hl=2 l= 109 cons: SEQUENCE          
    4:d=2  hl=2 l= 103 cons: SEQUENCE          
    6:d=3  hl=2 l= 101 cons: SEQUENCE          
    8:d=4  hl=2 l=  99 cons: SEQUENCE          
   10:d=5  hl=2 l=  11 cons: SEQUENCE          
   12:d=6  hl=2 l=   9 prim: OBJECT            :sha256
   23:d=5  hl=2 l=  32 prim: OCTET STRING      [HEX DUMP]:356F91A1C81E1567BB47A6C5397173E6025C223A7CDE418BF13922580B4D80AA
   57:d=5  hl=2 l=  32 prim: OCTET STRING      [HEX DUMP]:AF2CCF1E13958B9FC883C363EE03106C33E33FDB37CB7D25E94873FCAC330D40
   91:d=5  hl=2 l=  16 prim: INTEGER           :1A75BDA001DEB85DFE63285D1DCA0D17
  109:d=2  hl=2 l=   2 cons: cont [ 2 ]        
  111:d=3  hl=2 l=   0 cons: SEQUENCE

The following are the differences:

Python has used the explicit NULL encoding of the SHA256 hash function AlgorithmIdentifier. We did not. This is probably not material, but it may be.

We have a trailing context-specific 2 node containing an empty Sequence. Python does not. Glancing at the ASN.1 document that appears to be the extensions field.

@Lukasa
Copy link
Contributor

Lukasa commented Sep 16, 2024

@weekly71 Want to try out which of those two things matters?

To fix the first difference, change this line to pass ASN1Any(erasing: ASN1Null()) instead of the current nil.

To fix the second difference, rewrite this section of code to pass nil instead of the empty Extensions object if nonce is nil.

It would be valuable to know which of these two the responder is unhappy with.

@weekly71
Copy link
Author

@Lukasa

It would be valuable to know which of these two the responder is unhappy with.

After changing this
requestExtensions: try .init(builder: { if let nonce { nonce } })
to this
requestExtensions: (nonce != nil) ? try .init(builder: { nonce! }) : nil
It finally seems to work (response is 2500 bytes long). Thanks for helping!
Should I make a pull request to enable sha256 by default?

@Lukasa
Copy link
Contributor

Lukasa commented Sep 16, 2024

I'd be happy to have that change, yes.

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

No branches or pull requests

3 participants