Skip to content

fix: more robust handling of whitespace and alternate pem format #232

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: more robust handling of whitespace and alternate pem format #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2db92ee
fix: more robust handling of whitespace and alternate pem format with…
Vikas-Sajanani May 10, 2025
ae85938
fix: format changes for jdk 17 pipeline
Vikas-Sajanani May 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 58 additions & 16 deletions src/main/java/com/spotify/github/v3/clients/PKCS1PEMKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -32,26 +32,68 @@
*/
final class PKCS1PEMKey {

// Matches RSA PEM format
private static final Pattern PKCS1_PEM_KEY_PATTERN =
Pattern.compile("(?m)(?s)^---*BEGIN RSA PRIVATE KEY.*---*$(.*)^---*END.*---*$.*");
Pattern.compile("(?m)(?s)^\\s*-{3,}\\s*BEGIN\\s+RSA\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$(.*)^\\s*-{3,}\\s*END\\s+RSA\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$.*");

// Matches PKCS8 PEM format
private static final Pattern PKCS8_PEM_KEY_PATTERN =
Pattern.compile("(?m)(?s)^\\s*-{3,}\\s*BEGIN\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$(.*)^\\s*-{3,}\\s*END\\s+PRIVATE\\s+KEY\\s*-{3,}\\s*$.*");

private PKCS1PEMKey() {}

/**
* Try to interpret the supplied key as a PKCS#1 PEM file.
* Try to interpret the supplied key as a PEM file (PKCS#1 or PKCS#8).
*
* @param privateKey the private key to use
*/
public static Optional<KeySpec> loadKeySpec(final byte[] privateKey) {
final Matcher isPEM = PKCS1_PEM_KEY_PATTERN.matcher(new String(privateKey));
if (!isPEM.matches()) {
return Optional.empty();
final String keyString = new String(privateKey);

// Try to match PKCS1 (RSA) format first
Matcher isPKCS1 = PKCS1_PEM_KEY_PATTERN.matcher(keyString);
if (isPKCS1.matches()) {
return extractKeySpec(isPKCS1.group(1), true);
}

// Try to match PKCS8 format
Matcher isPKCS8 = PKCS8_PEM_KEY_PATTERN.matcher(keyString);
if (isPKCS8.matches()) {
return extractKeySpec(isPKCS8.group(1), false);
}

byte[] pkcs1Key = Base64.getMimeDecoder().decode(isPEM.group(1));
byte[] pkcs8Key = toPkcs8(pkcs1Key);
final PKCS8EncodedKeySpec keySpec = new PKCS8EncodedKeySpec(pkcs8Key);
return Optional.of(keySpec);
// Not a recognized PEM format
return Optional.empty();
}

/**
* Extract a KeySpec from the base64 content.
*
* @param base64Content the base64 content from the PEM file
* @param isPKCS1 whether this is PKCS1 format (needs conversion to PKCS8)
* @return an Optional containing the KeySpec if successful
*/
private static Optional<KeySpec> extractKeySpec(final String base64Content, final boolean isPKCS1) {
try {
// Remove all whitespace
String sanitizedContent = base64Content.replaceAll("\\s+", "");

// Check if content is empty after whitespace removal
if (sanitizedContent.isEmpty()) {
return Optional.empty();
}

// Decode the base64 content
byte[] decodedKey = Base64.getDecoder().decode(sanitizedContent);

// Convert to PKCS8 if necessary
byte[] pkcs8Key = isPKCS1 ? toPkcs8(decodedKey) : decodedKey;

return Optional.of(new PKCS8EncodedKeySpec(pkcs8Key));
} catch (IllegalArgumentException e) {
// Failed to decode base64 content
return Optional.empty();
}
}

/**
Expand All @@ -68,15 +110,15 @@ private static byte[] toPkcs8(final byte[] pkcs1Bytes) {
final int pkcs1Length = pkcs1Bytes.length;
final int totalLength = pkcs1Length + 22;
byte[] pkcs8Header = new byte[] {
0x30, (byte) 0x82, (byte) ((totalLength >> 8) & 0xff), (byte) (totalLength & 0xff), // Sequence + total length
0x2, 0x1, 0x0, // Integer (0)
0x30, 0xD, 0x6, 0x9, 0x2A, (byte) 0x86, 0x48, (byte) 0x86, (byte) 0xF7, 0xD, 0x1, 0x1, 0x1, 0x5, 0x0, // Sequence: 1.2.840.113549.1.1.1, NULL
0x4, (byte) 0x82, (byte) ((pkcs1Length >> 8) & 0xff), (byte) (pkcs1Length & 0xff) // Octet string + length
0x30, (byte) 0x82, (byte) ((totalLength >> 8) & 0xff), (byte) (totalLength & 0xff), // Sequence + total length
0x2, 0x1, 0x0, // Integer (0)
0x30, 0xD, 0x6, 0x9, 0x2A, (byte) 0x86, 0x48, (byte) 0x86, (byte) 0xF7, 0xD, 0x1, 0x1, 0x1, 0x5, 0x0, // Sequence: 1.2.840.113549.1.1.1, NULL
0x4, (byte) 0x82, (byte) ((pkcs1Length >> 8) & 0xff), (byte) (pkcs1Length & 0xff) // Octet string + length
};

byte[] pkcs8bytes = new byte[pkcs8Header.length + pkcs1Bytes.length];
System.arraycopy(pkcs8Header, 0, pkcs8bytes, 0, pkcs8Header.length);
System.arraycopy(pkcs1Bytes, 0, pkcs8bytes, pkcs8Header.length, pkcs1Bytes.length);
return pkcs8bytes;
}
}
}
110 changes: 110 additions & 0 deletions src/test/java/com/spotify/github/v3/clients/GitHubClientPEMTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*-
* -\-\-
* github-api
* --
* Copyright (C) 2016 - 2025 Spotify AB
* --
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* -/-/-
*/

package com.spotify.github.v3.clients;

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import com.google.common.io.Resources;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.junit.jupiter.api.Test;

/**
* Tests for GitHub with various PEM key formats
*/
public class GitHubClientPEMTest {

private static final Pattern BEGIN_PATTERN = Pattern.compile("(-----)BEGIN RSA PRIVATE KEY(-----)");
private static final Pattern END_PATTERN = Pattern.compile("(-----)END RSA PRIVATE KEY(-----)");

@Test
public void testJwtWithPoorlyFormattedPEM() throws Exception {
// Load a PEM key resource
byte[] originalKey = Resources.toByteArray(
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem"));
String pemContent = new String(originalKey, StandardCharsets.UTF_8);

// Manipulate just the BEGIN/END delimiters without touching the key content
String poorlyFormattedPem = pemContent;

// Add spaces to BEGIN
Matcher beginMatcher = BEGIN_PATTERN.matcher(poorlyFormattedPem);
if (beginMatcher.find()) {
poorlyFormattedPem = poorlyFormattedPem.replace(
beginMatcher.group(0),
beginMatcher.group(1) + " BEGIN RSA PRIVATE KEY " + beginMatcher.group(2)
);
}

// Add spaces to END
Matcher endMatcher = END_PATTERN.matcher(poorlyFormattedPem);
if (endMatcher.find()) {
poorlyFormattedPem = poorlyFormattedPem.replace(
endMatcher.group(0),
endMatcher.group(1) + " END RSA PRIVATE KEY " + endMatcher.group(2)
);
}

// Test that we can parse this PEM
JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(
poorlyFormattedPem.getBytes(StandardCharsets.UTF_8));
String token = tokenIssuer.getToken(123456);
assertNotNull(token);
}

@Test
public void testJwtWithExtraDashesPEM() throws Exception {
// Load a PEM key resource
byte[] originalKey = Resources.toByteArray(
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem"));
String pemContent = new String(originalKey, StandardCharsets.UTF_8);

// Manipulate just the BEGIN/END delimiters without touching the key content
String extraDashesPem = pemContent;

// Add extra dashes to BEGIN
Matcher beginMatcher = BEGIN_PATTERN.matcher(extraDashesPem);
if (beginMatcher.find()) {
extraDashesPem = extraDashesPem.replace(
beginMatcher.group(0),
"-------BEGIN RSA PRIVATE KEY-------"
);
}

// Add extra dashes to END
Matcher endMatcher = END_PATTERN.matcher(extraDashesPem);
if (endMatcher.find()) {
extraDashesPem = extraDashesPem.replace(
endMatcher.group(0),
"-------END RSA PRIVATE KEY-------"
);
}

// Test that we can parse this PEM
JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(
extraDashesPem.getBytes(StandardCharsets.UTF_8));
String token = tokenIssuer.getToken(123456);
assertNotNull(token);
}
}
27 changes: 14 additions & 13 deletions src/test/java/com/spotify/github/v3/clients/JwtTokenIssuerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -20,30 +20,31 @@

package com.spotify.github.v3.clients;

import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import com.google.common.io.Resources;
import java.net.URL;
import org.junit.jupiter.api.Test;

/**
* Simple test for JwtTokenIssuer to focus on core functionality
*/
public class JwtTokenIssuerTest {

private static final URL DER_KEY_RESOURCE =
Resources.getResource("com/spotify/github/v3/github-private-key");
Resources.getResource("com/spotify/github/v3/github-private-key");

// generated using this command: "openssl genrsa -out fake-github-app-key.pem 2048"
private static final URL PEM_KEY_RESOURCE =
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem");
Resources.getResource("com/spotify/github/v3/fake-github-app-key.pem");

@Test
public void loadsDERFileWithPKCS8Key() throws Exception {
public void loadsDERFile() throws Exception {
final byte[] key = Resources.toByteArray(DER_KEY_RESOURCE);
final JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(key);

final String token = tokenIssuer.getToken(42);
assertThat(token, not(nullValue()));
assertNotNull(token);
System.out.println("DER key JWT token generated successfully: " + token);
}

@Test
Expand All @@ -52,7 +53,7 @@ public void loadsPEMFile() throws Exception {
final JwtTokenIssuer tokenIssuer = JwtTokenIssuer.fromPrivateKey(key);

final String token = tokenIssuer.getToken(42);
assertThat(token, not(nullValue()));
assertNotNull(token);
System.out.println("PEM key JWT token generated successfully: " + token);
}

}
}
Loading
Loading