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

OpenSSL 3.1.5001 #217

Open
AndyQ opened this issue May 4, 2024 · 6 comments
Open

OpenSSL 3.1.5001 #217

AndyQ opened this issue May 4, 2024 · 6 comments
Labels
help wanted Extra attention is needed

Comments

@AndyQ
Copy link
Owner

AndyQ commented May 4, 2024

I've added a branch - OpenSSL3 which is the start of upgrading to OpenSSL 3.1.5001 (current version).

It works only on BAC at the moment, for some reason (unknown at the moment), PACE fails in step 4 when sending the auth token to the passport (invalid parameters received).

If anyone fancies taking a look and seeing if they can see what I've missed that would be great!

Note - this branch has only the bare minimum code changes to get it compiling. There has been a LOT of changes between OpenSSL1 and 3 - lots of functions have been deprecated (which I need to fix and figure out the alternatives - I think its just a migation from the low level functions to the higher level ECP_PKEY ones - as per the migration guide https://www.openssl.org/docs/man3.0/man7/migration_guide.html)

@AndyQ AndyQ added the help wanted Extra attention is needed label May 4, 2024
@petersulaf
Copy link

Hi, has anyone figured this out?
Upgrading do OpenSSL3 is inevitable because of the strict apple policies (privacy manifest, signing, bitcode etc...)

@Thormeard
Copy link

During the development one of the pods was statically including OpenSSL3 and forcing passport scanner to also use OpenSSL3. And a problem is that if your passport have certificates with ECDSA - then you have an error during verification:

error 94 at 1 depth lookup: Certificate public key has explicit ECC parameters

And according to ICAO all passports with ECDSA MUST use explicit curve parameters paragraph 4.1.6.3.

Apparently OpenSSL is not allowing to use explicit cure parameters in v3. There is a lot of comments about that (and openssl responded with something like "you should change your ICAO standards").

Note that certificates that are using RSA is still parsed/verified no problem.

Just another thing to keep in mind/test if you are going to update to v3. Would be happy to help/test with the passports I have.

@thomzon
Copy link

thomzon commented Jul 25, 2024

Hi @AndyQ !

First, thanks for your great work on this, it's very much appreciated.

I'm in the process of trying to migrate to OpenSSL 3. I've made locally pretty much the same as you did on your branch, i.e. the bare minimum to make it compile after switching to OpenSSL 3. I have the same PACE failure.

I'm going to look into this, and will share if I find a solution, but I'm wondering: maybe this issue shouldn't really be tackled by trying to fix the current code, but by rewriting it using non-deprecated APIs? Or is the issue such that it shouldn't make a difference?

@thomzon
Copy link

thomzon commented Jul 25, 2024

I can't reproduce the issue that @Thormeard is describing, on my side it seems like a different one. It fails like for you @AndyQ when sending the last general auth command.

I've looked into it more and comparing the authentication token generation between the previous version and the OpenSSL 3 version. It seems the AES CMAC generation yields different results with the same inputs between both version, and I think this causes the error. Somehow the CMAC API (which is now deprecated) has changed and produces something different.

Rather than trying to fix it, I'm trying to re-implement the AES CMAC generation by using the EVP_MAC API, as recommended in the OpenSSL 3 migration guide. Unfortunately I'm a bit stuck, the EVP_MAC_init function always fail for me, after trying all the configurations I could think of. Here is the current code:

guard let lib = OSSL_LIB_CTX_new()
else {
    return []
}
defer { OSSL_LIB_CTX_free(lib) }

guard let macAlgo = EVP_MAC_fetch(lib, "CMAC", nil)
else {
    return []
}
defer { EVP_MAC_free(macAlgo) }

guard let ctx = EVP_MAC_CTX_new(macAlgo)
else {
    return []
}
defer { EVP_MAC_CTX_free(ctx) }

var params: [OSSL_PARAM] = []
var cipherString = "aes-128-cbc".cString(using: .utf8)!
let cipherParam = OSSL_PARAM_construct_utf8_string(
    OSSL_MAC_PARAM_CIPHER,
    &cipherString,
    0
)
params.append(cipherParam)
params.append(OSSL_PARAM_construct_end())
var keyValue = key
guard EVP_MAC_init(ctx, &keyValue, key.count, params) == 1
else {
    return []
}
...

Please note it's a draft and I'm hardcoding the cipher as in my test it's always this one, but of course it should be adapted to throw instead of returning an empty array, and to have a dynamic cipher depending on the key size.
If anyone can see at a glance what I'm doing wrong, please correct me.

@AndyQ
Copy link
Owner Author

AndyQ commented Jul 25, 2024

I agree we should be moving away from the deprecated functions - thats a lot more work than I currently have time for at the moment sadly!

I'll take a look at your code above when I get a chance and see if I can see anything too.

@thomzon
Copy link

thomzon commented Aug 16, 2024

So turns out the AES CMAC calculation worked perfectly fine, the issue is elsewhere, probably linked to what @Thormeard posted.

I'll keep investigating this, I'll let you know if I find anything else.

Just in case, I did manage to convert the AES CMAC generation to the new APIs, here is the function I have locally.

    static func generateAESCMAC(key: [UInt8], message: [UInt8]) throws -> [UInt8] {
        guard let lib = OSSL_LIB_CTX_new()
        else {
            throw OpenSSLError.cmac("Cannot setup lib ctx")
        }
        defer { OSSL_LIB_CTX_free(lib) }
        
        guard let macAlgo = EVP_MAC_fetch(lib, "CMAC", nil)
        else {
            throw OpenSSLError.cmac("Cannot fetch CMAC algo")
        }
        defer { EVP_MAC_free(macAlgo) }

        guard let ctx = EVP_MAC_CTX_new(macAlgo)
        else {
            throw OpenSSLError.cmac("Cannot create CMAC ctx")
        }
        defer { EVP_MAC_CTX_free(ctx) }

        var cipherString = "AES-128-CBC".cString(using: .utf8)!
        try OSSL_MAC_PARAM_CIPHER.withCString { cipherKey in
            try cipherString.withUnsafeMutableBytes { cipherBytes in
                let cipherParam = OSSL_PARAM_construct_utf8_string(
                    cipherKey,
                    cipherBytes.baseAddress,
                    0
                )
                let params = [cipherParam, OSSL_PARAM_construct_end()]
                guard EVP_MAC_init(ctx, key, key.count, params) == 1
                else {
                    throw OpenSSLError.cmac("Cannot init CMAC")
                }
            }
        }
                
        guard EVP_MAC_update(ctx, message, message.count) == 1
        else {
            throw OpenSSLError.cmac("Cannot update CMAC")
        }
        
        var cmacResult = [UInt8](repeating: 0, count: Int(EVP_MAX_MD_SIZE))
        var cmacLength: size_t = 0
        
        if EVP_MAC_final(ctx, &cmacResult, &cmacLength, cmacResult.count) == 1 {
            return Array(cmacResult[0..<cmacLength])
        } else {
            throw OpenSSLError.cmac("Cannot final CMAC")
        }
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants