RustCrypto / RSA

RSA implementation in pure Rust
Apache License 2.0
536 stars 148 forks source link

`RsaPrivateKey::from_pkcs1_pem` error: PEM Base64 error: invalid Base64 encoding #362

Closed thep0y closed 1 year ago

thep0y commented 1 year ago

Reproduction code:

use rsa::pkcs1::DecodeRsaPrivateKey;
use rsa::RsaPrivateKey;

fn main() {
  let pem = "-----BEGIN RSA PRIVATE KEY-----
MIIEowIBAAKCAQEAhL3pWP55B1fFkvYOOtcfOEUhvBodHpLnc9KmuAU4H8V+ocyKTckacBhJrwIT8ILMzpflfqpzl+dvpJIsx3IbNB1Pmy0V949+vCoiBEa0Maw1dgJfIwghrsKL41wqzeaFqC6CehiefvwTKrS8zxIK1gDHipsimNsDbqCYmojDxhyorO4CC0iXld+1+LQxajf9ZdG1nLDd9dgVGiMkEDUSGXKCt6NpoDHp/6ptAcYSvnn5W4YUNWdy+wVxT0jyrbP1gNUDG1DVtFLrZXNQ9vY5wRyaix8vUccQ3s5o186tPgpup++lU3v2NgzybF9JzJWPdanDv5cJcJyFe43LNdD9mwIDAQABAoIBAE6ohsUgjjFmtoIeYlJFtY9Pj+z6AHVkkdiVZAu5tAdHrYFNRktPtuXjzo3xCkXEDH8DjY+gi7zg/Mwlfnl1SV95WvnWHqVDF9OCmAO2rgKdTFSwlRWaNPNs/x3sOMUoK918Kf8V5Z3T27u7vAJsa5VigtvEkBzJ81+ztJomHvSemrMGoQNPSWenaRHsxbkTtvHWOHLLYjzahFDUbzVx2QdVeBPXPOnG4Nkk1uIbYasqSJNUAlaSCshpgIJQ3M7I1Cef7hcLcOUn9OamkUXipEQRKNu6XhZ32yZbhkcvM7irtWtLRaPOj98U7/Ful0W1qM2/C1jTJ3qYP1CBrZKsW8ECgYEA3b/hneLaseoE6ATFWhjrdQY9VPjJjz7VQGuDzy3lLDiRL2/9KrhU1HyAYQFaC03+dZj9Nr2GDua88vrdwPtOZoe5REn+U+YyAHhNsqKYGoVL/RbyHagmDDEJfhPOv3tMElI9hZlfsF5QRsqS9CnhLej7GJNA5wEhXBKeJL54JTsCgYEAmT6b5F2wy2jBi7o9q6Io+AFOcqEzsf0cpoeAJQU3YRThkQZoCDmcvfh21jl7A5538YulVo8xnPcSeq3SzmW9p34HW7UNZaI4ZeH9flecimNuEmWAM6pJ3jnuJwFSZSs8rQOdRh5EkLDHkswssQ0jrUaSJPzKrc4Vpvt9RadlgyECgYEAgOMic+6DRp3KtEDxpGiPkrlJtLXWEZhnwsW7GxSF+6n1WSkycT4qsEadJ8TtXVy75gZCRyrpXIfokyIU+jIXY6jHmlWXqZRGw9co8gdneSK5BCXuHCa71qI57jn9FXbIxG1grOJ0p8Jpznu35orhAxpDuAj+1EXn6eg4WwsKMjkCgYBaAzOQdRPmtvaQu2mECjEkU6gfnt59mt4cVxUHKcQ4qwo/pFKxGh4eW/Z5qLPAEXIEmHEaoeuTdnENTFK513sKCfYKgROIcjvMZG0ArDeP9g1ukt41+r1+4eooURdzw2zVd30G9bpyftQLPxC1QrGFEyG+xhnK83U2axxPMJ9o4QKBgAcDHoC8boYYJ7fk72RbJv4G6uGKkdPTeu1TzwMHeTB6uEa0jTKHwxc83O0rYVgBVjh2WckEU18zhsAT/YjpNyq5wl2ZXzoeHrAANTZmtVxBCEDmkwy535hjzo4NIvDJoUoAysfxR8+/e9MGDJxYoHsOVnXx5JUWRO0XbB5F2o1Y
-----END RSA PRIVATE KEY-----";
  let _ = RsaPrivateKey::from_pkcs1_pem(pem).unwrap();
}

What is the error and where did it occur? How should I fix this error?

thep0y commented 1 year ago

My mistake, I didn't realize that Pem file requires multiple lines with 64 characters per line.

dmoebius commented 1 year ago

@thep0y I would really like to have this issue reopened. While it's technically correct that RFC 7468 (the spec for PEM documents) states in chapter 2, that:

Generators MUST wrap the base64-encoded lines so that each line consists of exactly 64 characters except for the final line, which will encode the remainder of the data (within the 64-character line boundary), and they MUST NOT emit extraneous whitespace.

but it also states that:

Parsers MAY handle other line sizes.

in the very next sentence.

Other implementators of PEM parsers deal leniently with long-lined PEMs, e.g. NodeJS:

import crypto from 'node:crypto'
import {Buffer} from 'node:buffer'

const publicKey =
    '-----BEGIN PUBLIC KEY-----\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDgtQn2JZ34ZC28NWYpAUd98iZ37BUrX/aKzmFbt7clFSs6sXqHauqKWqdtLkF2KexO40H1YTX8z2lSgBBOAxLsvaklV8k4cBFK9snQXE9/DDaFt6Rr7iVZMldczhC0JNgTz+SHXT6CBHuX3e9SdB1Ua44oncaTWz7OBGLbCiK45wIDAQAB\n-----END PUBLIC KEY-----'
const msg = Buffer.from('my secret message')

const rsaEncrypt = (msg, pubKey) => {
    msg = Buffer.concat([msg, Buffer.alloc(128 - msg.length)]) //pad to 128 bytes
    return crypto.publicEncrypt(
        { key: pubKey, padding: crypto.constants.RSA_PKCS1_PADDING },
        msg,
    )
}

const encrypt = rsaEncrypt(msg, publicKey).toString('hex')
console.log(encrypt)

runs just fine, despite the public key having a single long line > 64 chars.

And Java, too:

import java.security.KeyFactory;
import java.security.PublicKey;
import java.security.spec.X509EncodedKeySpec;
import java.util.Base64;

public class parsepem {
    public static void main(String[] args) throws Exception {
        String pem = """
                -----BEGIN PUBLIC KEY-----
                MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDgtQn2JZ34ZC28NWYpAUd98iZ37BUrX/aKzmFbt7clFSs6sXqHauqKWqdtLkF2KexO40H1YTX8z2lSgBBOAxLsvaklV8k4cBFK9snQXE9/DDaFt6Rr7iVZMldczhC0JNgTz+SHXT6CBHuX3e9SdB1Ua44oncaTWz7OBGLbCiK45wIDAQAB
                -----END PUBLIC KEY-----""";

        byte[] encoded = Base64.getDecoder().decode(pem
            .replace("-----BEGIN PUBLIC KEY-----", "")
            .replaceAll("\n", "")
            .replace("-----END PUBLIC KEY-----", ""));

        X509EncodedKeySpec keySpec = new X509EncodedKeySpec(encoded);
        PublicKey publicKey = KeyFactory.getInstance("RSA").generatePublic(keySpec);
        System.out.println(publicKey);
    }
}

Of course we could always add the line breaks before feeding it into RsaPrivateKey::from_pkcs1_pem or RsaPublicKey::from_public_key_pem, but we don't always have full control where the PEM input comes from. There are libraries outthere using those methods with a PEM provided by a file. In this situations we cannot modify the input.

Or shall I open a new feature request for this?

thep0y commented 1 year ago

@dmoebius I can reopen this issue.

If you submit a pull request to fix this issue, that would be better, because I was also confused for a long time that Node's crypto is not limited to 64 characters per line.

dmoebius commented 1 year ago

Thanks for reopening. :)

I fear that I cannot really provide a PR because I'm a total Rust newbie. I might give it a try, but I cannot guarantee anything at all, sorry.

tarcieri commented 1 year ago

The pem-rfc7468 crate does support alternative line-widths, but it requires the line-width be specified in advance, and the functionality isn't exposed through APIs like RsaPrivateKey::from_pkcs1_pem. This exists largely to support OpenSSH private keys, which use a non-standard line width of 70.

The parser is specially written so that the parser avoids looking at the secret parts of the Base64 body (which is often private key data) using non-constant-time code, which isn't possible if the wrap width isn't known in advance and the parser has to go spelunking through the private key data for line endings. A stricter interpretation of the MAY makes this possible, and this parser is able to completely avoid any sort of branching or table lookups on this secret data. This avoids a format parser timing sidechannel which has been successfully exploited by researchers.

RFC7468 exists to tame PEM, which was wildly underspecified in previous versions, especially for PKIX usages like storing private keys. There's a way to interpret the usage of SHOULD vs MAY at least in this RFC (but perhaps more generally): the former are parts of the spec which are optional and encouraged, and the latter are optional and generally discouraged but exist for legacy interop purposes.

Given that, I don't think we'll be revisiting this decision.

dmoebius commented 1 year ago

Fair enough. Thanks for the explanation. :+1: