-
Notifications
You must be signed in to change notification settings - Fork 0
Check if PEM bundle is valid after downloading #283
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
Open
reiniercriel
wants to merge
1
commit into
main
Choose a base branch
from
feat/pem-bundle-validation
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| package utils | ||
|
|
||
| import ( | ||
| "crypto/x509" | ||
| "encoding/pem" | ||
| "fmt" | ||
| "os" | ||
| "strings" | ||
| ) | ||
|
|
||
| // ReadAndValidatePEMBundle reads a PEM certificate bundle from path, validates | ||
| // that it contains at least one well-formed CERTIFICATE block, and returns the | ||
| // normalised PEM content. Symlinks and non-regular files are rejected. | ||
| func ReadAndValidatePEMBundle(path string) (string, error) { | ||
| info, err := os.Lstat(path) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| if info.Mode()&os.ModeSymlink != 0 { | ||
| return "", fmt.Errorf("refusing to read symlinked certificate bundle %s", path) | ||
| } | ||
| if !info.Mode().IsRegular() { | ||
| return "", fmt.Errorf("refusing to read non-regular certificate bundle %s", path) | ||
| } | ||
|
|
||
| data, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| normalized := strings.TrimSpace(strings.ReplaceAll(string(data), "\r\n", "\n")) | ||
| if normalized == "" { | ||
| return "", fmt.Errorf("certificate bundle %s is empty", path) | ||
| } | ||
|
|
||
| var ( | ||
| rest = []byte(normalized) | ||
| blocks []string | ||
| certCount int | ||
| ) | ||
|
|
||
| for len(rest) > 0 { | ||
| block, remaining := pem.Decode(rest) | ||
| if block == nil { | ||
| if strings.TrimSpace(string(rest)) != "" { | ||
| return "", fmt.Errorf("certificate bundle %s contains non-PEM content", path) | ||
| } | ||
| break | ||
| } | ||
|
|
||
| if block.Type != "CERTIFICATE" { | ||
| return "", fmt.Errorf("certificate bundle %s contains unsupported PEM block type %q", path, block.Type) | ||
| } | ||
|
|
||
| // Include the certificate regardless of whether Go's strict x509 parser | ||
| // accepts it — legacy root CAs (e.g. negative serial numbers) are valid | ||
| // for OpenSSL/pip but rejected by Go. We still parse to catch genuinely | ||
| // malformed DER; those are skipped rather than failing the whole bundle. | ||
| if _, err := x509.ParseCertificate(block.Bytes); err != nil { | ||
| rest = remaining | ||
| continue | ||
| } | ||
|
|
||
| blocks = append(blocks, strings.TrimSpace(string(pem.EncodeToMemory(block)))) | ||
| certCount++ | ||
| rest = remaining | ||
| } | ||
|
|
||
| if certCount == 0 { | ||
| return "", fmt.Errorf("certificate bundle %s does not contain any valid certificates", path) | ||
| } | ||
|
|
||
| return strings.Join(blocks, "\n"), nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package utils | ||
|
|
||
| import ( | ||
| "crypto/rand" | ||
| "crypto/rsa" | ||
| "crypto/x509" | ||
| "crypto/x509/pkix" | ||
| "encoding/pem" | ||
| "math/big" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
| ) | ||
|
|
||
| func TestReadAndValidatePEMBundleValidCertificate(t *testing.T) { | ||
| path := filepath.Join(t.TempDir(), "bundle.pem") | ||
| pemData := mustCreateTestCertificatePEM(t, "test-cert") | ||
| if err := os.WriteFile(path, []byte(pemData), 0o644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| got, err := ReadAndValidatePEMBundle(path) | ||
| if err != nil { | ||
| t.Fatalf("ReadAndValidatePEMBundle failed: %v", err) | ||
| } | ||
|
|
||
| if !strings.Contains(got, "BEGIN CERTIFICATE") { | ||
| t.Fatalf("expected certificate PEM in output, got %q", got) | ||
| } | ||
| } | ||
|
|
||
| func TestReadAndValidatePEMBundleRejectsNonPEMContent(t *testing.T) { | ||
| path := filepath.Join(t.TempDir(), "bundle.pem") | ||
| if err := os.WriteFile(path, []byte("not a certificate"), 0o644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| _, err := ReadAndValidatePEMBundle(path) | ||
| if err == nil { | ||
| t.Fatal("expected error for non-PEM content") | ||
| } | ||
| } | ||
|
|
||
| func TestReadAndValidatePEMBundleRejectsSymlink(t *testing.T) { | ||
| dir := t.TempDir() | ||
| target := filepath.Join(dir, "target.pem") | ||
| link := filepath.Join(dir, "link.pem") | ||
|
|
||
| pemData := mustCreateTestCertificatePEM(t, "symlink-test") | ||
| if err := os.WriteFile(target, []byte(pemData), 0o644); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if err := os.Symlink(target, link); err != nil { | ||
| t.Skipf("symlink creation not supported: %v", err) | ||
| } | ||
|
|
||
| _, err := ReadAndValidatePEMBundle(link) | ||
| if err == nil { | ||
| t.Fatal("expected error for symlinked bundle") | ||
| } | ||
| } | ||
|
|
||
| func mustCreateTestCertificatePEM(t *testing.T, commonName string) string { | ||
| t.Helper() | ||
|
|
||
| key, err := rsa.GenerateKey(rand.Reader, 2048) | ||
| if err != nil { | ||
| t.Fatalf("GenerateKey failed: %v", err) | ||
| } | ||
|
|
||
| template := &x509.Certificate{ | ||
| SerialNumber: big.NewInt(1), | ||
| Subject: pkix.Name{ | ||
| CommonName: commonName, | ||
| }, | ||
| NotBefore: time.Now().Add(-time.Hour), | ||
| NotAfter: time.Now().Add(time.Hour), | ||
| KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature, | ||
| BasicConstraintsValid: true, | ||
| IsCA: true, | ||
| } | ||
|
|
||
| der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) | ||
| if err != nil { | ||
| t.Fatalf("CreateCertificate failed: %v", err) | ||
| } | ||
|
|
||
| return string(pem.EncodeToMemory(&pem.Block{ | ||
| Type: "CERTIFICATE", | ||
| Bytes: der, | ||
| })) | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential file inclusion attack via reading file - high severity
If an attacker can control the input leading into the ReadFile function, they might be able to read sensitive files and launch further attacks with that information.
Show fix
Remediation: Ignore this issue only after you've verified or sanitized the input going into this function.
Reply
@AikidoSec ignore: [REASON]to ignore this issue.More info