globaleaks / whistleblowing-software

GlobaLeaks is free, open-source whistleblowing software enabling anyone to easily set up and maintain a secure reporting platform.
https://www.globaleaks.org
Other
1.22k stars 267 forks source link

Accept DER encoded SSL material in admin/network/https API #2085

Open NSkelsey opened 7 years ago

NSkelsey commented 7 years ago

Current behavior

The current API only accepts files uploaded as PEM encoding because the API accepts json and interpreting byte formats was step to far to get the implementation out of the door. As a result we are left with this string that really should not have the last two sentences.

This configuration wizard will guide you through the setup of HTTPS. The interface can generate a Private Key and a Certificate Signing Request. The Certificate Signing Request can be provided to a Certificate Authority to retrieve a Certificate. After having validated your request and your ownership of the hostname, the Certificate Authority will issue you a Certificate and provide a file containing Intermediate Certificates. Load the Certificate and the file containing the Intermediate Certificates to complete the HTTPS setup. Please note that the file format expected for all files uploaded is PEM. Sometimes Certificate Authorities will provide certificates in DER format which must be converted to PEM.

Expected behavior

If we implement the processing for DER in the backend we can cut this language out of the UI and make everyone happy.

What is the motivation or use case for changing the behavior?

Translating requirements about file formats is a waste of time for our translators and we can easily implement the extra handling to simplify the UI.

evilaliv3 commented 7 years ago

I agree, good work to open this ticket.

I'm proceeding already splitting the sentence in two, and manually pushing the translations for the split strings on transifex; this way we are ready for possible removal.

evilaliv3 commented 7 years ago

The strings has been corrected in commit: https://github.com/globaleaks/GlobaLeaks/commit/14d59c2c1b79067c4cc686ebac08646875213e0b

Old translations have been saved and manually re-applied on transifex.

evilaliv3 commented 7 years ago

Changing the ticket in BUG so to keep track of the current fact that in case a user loads a DER file the application will currently fail.

NSkelsey commented 7 years ago

Okay, after a bit of research and review of our existing code, with a dive into RFC 1424 , it appears that to handle DER to PEM conversion we can place logic on the client side that detects a DER (or binary file) and produces the appropriate PEM encoded file. The PEM encoding is the base64 of an underlying dir file, chopped at 64 bytes with a header and footer appended.

The backend assumes in many places that certificate, key and chain files coming from the DB and the outside world are to be validated and across both utils/tls.py and handlers/admin/https.py the expected format is pem. Changing that logic would take some very careful testing and time that we don't really want to spend.

Finally, just converting to PEM when a binary file is detected and read fits nicely into the existing frontend logic, which uses the FileReader API to process the loaded files before there upload.

evilaliv3 commented 7 years ago

Seems reasonable but converting to PEM on the client should be eventually be performed on the backend in order to bypass possible browsers incompatibilities.

An eventual good start on the javascript side is https://stackoverflow.com/questions/40314257/export-webcrypto-key-to-pem-format that is a small implementation.

fpietrosanti commented 7 years ago

may i give the option to avoid doing it, just detecting DER and preventing upload?

All top 5 CA use PEM as standard format, and provide on their website guidelines to convert from DER to PEM for legacy purposes https://medium.com/@mgajjar/top-5-trustworthy-ssl-certificate-authorities-worldwide-25eeb6d4e8c1

evilaliv3 commented 7 years ago

A schema for a minimal implementation that could be done on the backend using already implemented libraries is:

client:
    file = base64(file)

server:
    data = decode_base64()

    key = None

    try:
        key = read_pem(data)
    except:
        pass

    try:
        key = read_pem(der_to_pem(data))
    except:
        pass

    return key
NSkelsey commented 7 years ago

To handle encoding from DER to PEM the frontend without significantly changing the form of the submission, the frontend needs to handle the base64 of a TypedArray. It would appear that the function window.atob(str) would suffice for this purpose however passing a binary data read as txt into the function throws DOMException: Failed to execute 'atob' on 'Window': The string to be decoded is not correctly encoded.

The solution to this base64 encoding is described in MDN's docs as requiring [a shim](https://developer.mozilla.org/en-US/docs/Web/API/WindowBase64/Base64_encoding_and_decoding#Solution_2_%E2%80%93_rewrite_the_DOMs_atob()_and_btoa()_using_JavaScript's_TypedArrays_and_UTF-8). This requires a new client library called base64-js which does the whole base64 interpretation in javascript instead of deferring to the browsers internal functions.

Further the code to convert a DER encoded file into a PEM on the frontend, or detect and retry with failures on the backend is too much overhead to justify spending anymore time on this ticket. As a result, this ticket will likely be closed as a WONT FIX.

evilaliv3 commented 7 years ago

the right function to be used is atob not btoa, so i think the analysis above could be wrong in the starting point.

anyway similar issues apply to the btoa but the solution https://stackoverflow.com/questions/40314257/export-webcrypto-key-to-pem-format should work as it is the same kind of operation that we perform symmetrically in current crypto.js

evilaliv3 commented 7 years ago

I've verified the above solution and it works out of the box for what relates encoding a file read as Uint8Array in base64.

Also the code documented to convert in PEM is correct but it could not work for our scenario because it will be difficult to convert DER files in the client for intermediate files that containing multiple certificates would require us to parse the DER binary format.

A possibility more easy is instead on the backend to open the file loading it in DER and printing it in PEM; This would cause the openssl/crypto parser to handle it automagically.

As it require various changes on the current backend implementation let's postpone the activities on this ticket for a while.