Безопасность: fail-closed auth (H1), доверенный прокси для IP (H2), CSRF на /order (M2)
По результатам security-аудита (docs, run-1). Три верифицированные находки. H1 (HIGH) — fail-open аутентификация. loadUsers() при ошибке чтения/парсинга users.json молча возвращал пустой стор → authOpen() считал это первичной настройкой → после удаления first-run файла весь /admin отдавался без входа. Теперь: постоянный маркер configured (пишется при создании первого админа и бэкфиллится при загрузке валидного стора); authOpen() открыт только на реально свежей системе; при пустом/битом сторе на настроенной системе — 503 и блокировка до восстановления файла (authLockedOut). Тест TestAuthFailClosed (4 сценария). H2 (HIGH) — интернет-слой доверял клиентскому X-Forwarded-For/X-Real-IP, что позволяло обойти лимит входа по IP и травить аудит. Теперь заголовкам доверяем только от доверенного прокси (по умолчанию loopback; расширяется BJ_TRUSTED_PROXIES), из XFF берём правый элемент. Исправлено и в Go-модуле (modules/lk/security.go), и в PHP-ретрансляторе. Тесты TestClientIPTrust (Go) и прогон логики PHP. M2 (MEDIUM) — POST /order (поручение на списание бумаг) не проверял CSRF, в отличие от /login и /impersonate. Добавлен csrfOK (форма уже несла токен). Тест TestOrderCSRF. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4YVo4UMopqT5DzicqpPjL
This commit is contained in:
zuevav
@@ -37,12 +37,14 @@ import (
|
||||
)
|
||||
|
||||
const (
|
||||
usersFile = "/var/lib/bj/.bj/users.json"
|
||||
sessionsFile = "/var/lib/bj/.bj/sessions.json"
|
||||
sessionCookie = "bj_session"
|
||||
sessionMaxLife = 24 * time.Hour
|
||||
)
|
||||
|
||||
// usersFile — стор учётных записей (var, чтобы подменять в тестах).
|
||||
var usersFile = "/var/lib/bj/.bj/users.json"
|
||||
|
||||
// ---------- роли и права ----------
|
||||
|
||||
type roleDef struct {
|
||||
@@ -224,19 +226,51 @@ type userStore struct {
|
||||
|
||||
var globalUsers = loadUsers()
|
||||
|
||||
// usersLoadFailed — файл учётных записей есть, но прочитать/разобрать его не
|
||||
// удалось (повреждение, права). В этом случае НЕЛЬЗЯ открывать админку без входа:
|
||||
// пустой стор из-за сбоя не должен выглядеть как «первичная настройка».
|
||||
var usersLoadFailed bool
|
||||
|
||||
func loadUsers() *userStore {
|
||||
s := &userStore{users: map[string]*User{}}
|
||||
if b, err := os.ReadFile(usersFile); err == nil {
|
||||
b, err := os.ReadFile(usersFile)
|
||||
switch {
|
||||
case err == nil:
|
||||
var list []*User
|
||||
if json.Unmarshal(b, &list) == nil {
|
||||
for _, u := range list {
|
||||
s.users[strings.ToLower(u.Login)] = u
|
||||
}
|
||||
if json.Unmarshal(b, &list) != nil {
|
||||
usersLoadFailed = true // файл есть, но это не корректный JSON — сбой, не «нет пользователей»
|
||||
return s
|
||||
}
|
||||
for _, u := range list {
|
||||
s.users[strings.ToLower(u.Login)] = u
|
||||
}
|
||||
if len(s.users) > 0 {
|
||||
markConfigured() // бэкфилл маркера для установок, заведённых до этой защиты
|
||||
}
|
||||
case os.IsNotExist(err):
|
||||
// файла нет: либо действительно первый запуск, либо стор потеряли на
|
||||
// настроенной системе — различаем по постоянному маркеру ниже (authOpen).
|
||||
default:
|
||||
usersLoadFailed = true // есть, но недоступен (права и т. п.)
|
||||
}
|
||||
return s
|
||||
}
|
||||
|
||||
// configuredMarker — постоянный признак «система уже была настроена» (заведён
|
||||
// хотя бы один пользователь). В отличие от first-run-файла он НЕ удаляется, и по
|
||||
// нему видно, что нулевой стор — это потеря данных, а не свежая установка.
|
||||
var configuredMarker = "/var/lib/bj/.bj/configured"
|
||||
|
||||
func isConfigured() bool { _, err := os.Stat(configuredMarker); return err == nil }
|
||||
|
||||
// markConfigured — отметить систему настроенной (идемпотентно, best-effort).
|
||||
func markConfigured() {
|
||||
if isConfigured() {
|
||||
return
|
||||
}
|
||||
_ = os.WriteFile(configuredMarker, []byte(time.Now().UTC().Format(time.RFC3339)+"\n"), 0o600)
|
||||
}
|
||||
|
||||
func (s *userStore) save() error {
|
||||
list := s.list()
|
||||
b, _ := json.MarshalIndent(list, "", " ")
|
||||
@@ -244,7 +278,13 @@ func (s *userStore) save() error {
|
||||
if err := os.WriteFile(tmp, b, 0o600); err != nil {
|
||||
return err
|
||||
}
|
||||
return os.Rename(tmp, usersFile)
|
||||
if err := os.Rename(tmp, usersFile); err != nil {
|
||||
return err
|
||||
}
|
||||
if len(list) > 0 {
|
||||
markConfigured() // с этого момента нулевой стор трактуется как потеря данных
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *userStore) list() []*User {
|
||||
@@ -328,7 +368,17 @@ func checkPassword(hash, p string) bool {
|
||||
}
|
||||
|
||||
// authOpen — режим первичной настройки: пользователей нет, вход не требуется.
|
||||
func authOpen() bool { return globalUsers.Count() == 0 }
|
||||
// authOpen — режим первичной настройки: вход не требуется. Открываем ТОЛЬКО на
|
||||
// действительно свежей системе — стор пуст, не было сбоя загрузки и система ещё
|
||||
// не отмечена настроенной. Потеря/порча users.json на настроенной системе сюда
|
||||
// НЕ попадает (fail-closed): её ловит запрет в authMiddleware.
|
||||
func authOpen() bool { return globalUsers.Count() == 0 && !usersLoadFailed && !isConfigured() }
|
||||
|
||||
// authLockedOut — стор недоступен/пуст на уже настроенной системе: админку
|
||||
// блокируем, пока файл не восстановят.
|
||||
func authLockedOut() bool {
|
||||
return globalUsers.Count() == 0 && (usersLoadFailed || isConfigured())
|
||||
}
|
||||
|
||||
// ---------- сессии ----------
|
||||
|
||||
@@ -890,6 +940,13 @@ func authMiddleware(rc *RuntimeConfig, next http.Handler) http.Handler {
|
||||
next.ServeHTTP(&authRW{ResponseWriter: w, user: nil}, r)
|
||||
return
|
||||
}
|
||||
if authLockedOut() {
|
||||
// Учётные записи потеряны/повреждены на настроенной системе.
|
||||
// НЕ открываем админку без входа — блокируем до восстановления файла.
|
||||
log.Printf("auth: админ-панель заблокирована — стор учётных записей недоступен (usersLoadFailed=%v, configured=%v)", usersLoadFailed, isConfigured())
|
||||
http.Error(w, "Учётные записи недоступны — админ-панель заблокирована. Восстановите /var/lib/bj/.bj/users.json из резервной копии и перезапустите bj-server.", http.StatusServiceUnavailable)
|
||||
return
|
||||
}
|
||||
var u *User
|
||||
if c, err := r.Cookie(sessionCookie); err == nil && c.Value != "" {
|
||||
if s := globalSessions.Get(c.Value, rc.Snapshot().Auth.idle()); s != nil {
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
package lkgateway
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// H1: пустой/битый users.json на настроенной системе НЕ должен открывать админку.
|
||||
func TestAuthFailClosed(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
oldUsers, oldMarker, oldStore, oldFailed := usersFile, configuredMarker, globalUsers, usersLoadFailed
|
||||
t.Cleanup(func() {
|
||||
usersFile, configuredMarker, globalUsers, usersLoadFailed = oldUsers, oldMarker, oldStore, oldFailed
|
||||
})
|
||||
usersFile = filepath.Join(dir, "users.json")
|
||||
configuredMarker = filepath.Join(dir, "configured")
|
||||
|
||||
reload := func() {
|
||||
usersLoadFailed = false
|
||||
globalUsers = loadUsers()
|
||||
}
|
||||
|
||||
// 1) свежая система: файла нет, маркера нет → открыто для первичной настройки
|
||||
reload()
|
||||
if !authOpen() || authLockedOut() {
|
||||
t.Fatalf("свежая система: ожидали open, got open=%v locked=%v", authOpen(), authLockedOut())
|
||||
}
|
||||
|
||||
// 2) корректный стор с админом → не открыто, не заблокировано, маркер создан
|
||||
if err := os.WriteFile(usersFile, []byte(`[{"login":"admin","role":"admin","active":true}]`), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
reload()
|
||||
if authOpen() || authLockedOut() {
|
||||
t.Fatalf("настроенная система: open=%v locked=%v", authOpen(), authLockedOut())
|
||||
}
|
||||
if !isConfigured() {
|
||||
t.Fatal("маркер настройки не создан при загрузке валидного стора")
|
||||
}
|
||||
|
||||
// 3) файл ПОВРЕЖДЁН (битый JSON) → fail-closed
|
||||
if err := os.WriteFile(usersFile, []byte(`{ не json`), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
reload()
|
||||
if authOpen() {
|
||||
t.Error("битый users.json открыл админку без входа — fail-open!")
|
||||
}
|
||||
if !authLockedOut() {
|
||||
t.Error("битый users.json должен блокировать админку")
|
||||
}
|
||||
|
||||
// 4) файл ПРОПАЛ, но система была настроена (маркер есть) → fail-closed
|
||||
os.Remove(usersFile)
|
||||
reload()
|
||||
if authOpen() {
|
||||
t.Error("пропавший users.json на настроенной системе открыл админку — fail-open!")
|
||||
}
|
||||
if !authLockedOut() {
|
||||
t.Error("пропавший стор на настроенной системе должен блокировать админку")
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,41 @@
|
||||
package lk
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// H2: X-Forwarded-For/X-Real-IP от НЕдоверенного источника не должны подменять IP.
|
||||
func TestClientIPTrust(t *testing.T) {
|
||||
req := func(remote string, hdr map[string]string) *http.Request {
|
||||
r, _ := http.NewRequest("POST", "/login", nil)
|
||||
r.RemoteAddr = remote
|
||||
for k, v := range hdr {
|
||||
r.Header.Set(k, v)
|
||||
}
|
||||
return r
|
||||
}
|
||||
// прямой клиент из интернета подставляет заголовки — игнорируем
|
||||
if got := clientIP(req("203.0.113.7:5000", map[string]string{"X-Real-IP": "1.2.3.4", "X-Forwarded-For": "9.9.9.9"})); got != "203.0.113.7" {
|
||||
t.Errorf("недоверенный источник: ожидали 203.0.113.7, получили %s (спуфинг прошёл)", got)
|
||||
}
|
||||
// доверенный прокси (loopback) сообщает реальный адрес — доверяем X-Real-IP
|
||||
if got := clientIP(req("127.0.0.1:5000", map[string]string{"X-Real-IP": "1.2.3.4"})); got != "1.2.3.4" {
|
||||
t.Errorf("loopback X-Real-IP: ожидали 1.2.3.4, получили %s", got)
|
||||
}
|
||||
// доверенный прокси, только XFF: берём ПРАВЫЙ элемент (левые подставил клиент)
|
||||
if got := clientIP(req("127.0.0.1:5000", map[string]string{"X-Forwarded-For": "9.9.9.9, 5.6.7.8"})); got != "5.6.7.8" {
|
||||
t.Errorf("loopback XFF: ожидали правый 5.6.7.8, получили %s", got)
|
||||
}
|
||||
// без заголовков — адрес соединения
|
||||
if got := clientIP(req("198.51.100.2:443", nil)); got != "198.51.100.2" {
|
||||
t.Errorf("без заголовков: ожидали 198.51.100.2, получили %s", got)
|
||||
}
|
||||
// настраиваемый доверенный прокси
|
||||
old := trustedProxies
|
||||
trustedProxies = loadTrustedProxies("10.0.0.0/8")
|
||||
defer func() { trustedProxies = old }()
|
||||
if got := clientIP(req("10.1.2.3:5000", map[string]string{"X-Real-IP": "1.2.3.4"})); got != "1.2.3.4" {
|
||||
t.Errorf("доверенный CIDR: ожидали 1.2.3.4, получили %s", got)
|
||||
}
|
||||
}
|
||||
+8
-1
@@ -1082,7 +1082,14 @@ func (s *Server) buildHandler() http.Handler {
|
||||
"PickISIN": r.URL.Query().Get("isin"), "Today": time.Now().Format("02.01.2006")})
|
||||
return
|
||||
}
|
||||
_ = r.ParseForm()
|
||||
// Изменяющее действие (поручение на списание бумаг) — только по своей форме
|
||||
// с CSRF-токеном, как /login и /impersonate. Иначе чужая страница могла бы
|
||||
// отправить поручение от имени вошедшего клиента.
|
||||
if !csrfOK(r) {
|
||||
setFlashCookie(w, r, "Форма устарела, откройте поручение заново.")
|
||||
http.Redirect(w, r, "/order", http.StatusSeeOther)
|
||||
return
|
||||
}
|
||||
var secs []map[string]any
|
||||
for _, isin := range r.Form["isin"] {
|
||||
q := strings.TrimSpace(r.FormValue("qty_" + isin))
|
||||
|
||||
@@ -614,3 +614,26 @@ func TestChartAndProfile(t *testing.T) {
|
||||
t.Error("POST /documents всё ещё принимает заказ документа")
|
||||
}
|
||||
}
|
||||
|
||||
// M2: POST /order без валидного CSRF-токена не должен уходить в ядро.
|
||||
func TestOrderCSRF(t *testing.T) {
|
||||
var transferCalled bool
|
||||
bj := fakeBJ(t)
|
||||
// добавим перехват transfer поверх fakeBJ нельзя (mux фиксирован) — проверяем по редиректу.
|
||||
s, err := New(Config{BJURL: bj.URL, BJKey: "lk-key", Title: "Кабинет"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
h := s.Handler()
|
||||
// без csrf-куки и поля — ожидаем редирект на /order, поручение не подаётся
|
||||
form := url.Values{"isin": {"RU0009029540"}, "qty_RU0009029540": {"10"}, "receiving_inn": {"7700000000"}}
|
||||
req := httptest.NewRequest(http.MethodPost, "/order", strings.NewReader(form.Encode()))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
req.AddCookie(&http.Cookie{Name: "bj_lk", Value: "sess-1"})
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/order" {
|
||||
t.Errorf("POST /order без CSRF: ожидали редирект на /order, got %d %s", rec.Code, rec.Header().Get("Location"))
|
||||
}
|
||||
_ = transferCalled
|
||||
}
|
||||
|
||||
@@ -97,14 +97,46 @@ function isLocal(): bool
|
||||
return $h === 'localhost' || $h === '127.0.0.1' || $h === '::1';
|
||||
}
|
||||
|
||||
// peerTrusted — непосредственный источник запроса (REMOTE_ADDR) — доверенный прокси:
|
||||
// loopback или адрес/CIDR из переменной окружения BJ_TRUSTED_PROXIES (через запятую).
|
||||
function peerTrusted(string $remote): bool
|
||||
{
|
||||
if ($remote === '') return false;
|
||||
if ($remote === '127.0.0.1' || $remote === '::1' || str_starts_with($remote, '127.')) return true;
|
||||
$list = getenv('BJ_TRUSTED_PROXIES') ?: '';
|
||||
foreach (explode(',', $list) as $cidr) {
|
||||
$cidr = trim($cidr);
|
||||
if ($cidr === '') continue;
|
||||
if (!str_contains($cidr, '/')) { if ($remote === $cidr) return true; continue; }
|
||||
[$net, $bits] = explode('/', $cidr, 2);
|
||||
$ipb = @inet_pton($remote); $netb = @inet_pton($net);
|
||||
if ($ipb === false || $netb === false || strlen($ipb) !== strlen($netb)) continue;
|
||||
$bytes = intdiv((int)$bits, 8); $rem = (int)$bits % 8;
|
||||
if (($bytes > 0 && substr($ipb, 0, $bytes) !== substr($netb, 0, $bytes))) continue;
|
||||
if ($rem === 0) return true;
|
||||
$mask = chr((0xff << (8 - $rem)) & 0xff);
|
||||
if ((ord($ipb[$bytes]) & ord($mask)) === (ord($netb[$bytes]) & ord($mask))) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// clientIP — адрес посетителя. X-Real-IP / X-Forwarded-For подставляет клиент, поэтому
|
||||
// доверяем им ТОЛЬКО от доверенного прокси; иначе берём адрес соединения. Иначе
|
||||
// злоумышленник подделал бы свой IP и обошёл лимит входа и журнал аудита в ядре.
|
||||
function clientIP(): string
|
||||
{
|
||||
foreach (['HTTP_X_REAL_IP', 'HTTP_X_FORWARDED_FOR'] as $k) {
|
||||
if (!empty($_SERVER[$k])) {
|
||||
return trim(explode(',', $_SERVER[$k])[0]);
|
||||
$remote = $_SERVER['REMOTE_ADDR'] ?? '';
|
||||
if (peerTrusted($remote)) {
|
||||
if (!empty($_SERVER['HTTP_X_REAL_IP'])) {
|
||||
return trim($_SERVER['HTTP_X_REAL_IP']);
|
||||
}
|
||||
if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) {
|
||||
// правый элемент добавил наш доверенный прокси; левые может подставить клиент
|
||||
$parts = explode(',', $_SERVER['HTTP_X_FORWARDED_FOR']);
|
||||
return trim($parts[count($parts) - 1]);
|
||||
}
|
||||
}
|
||||
return $_SERVER['REMOTE_ADDR'] ?? '';
|
||||
return $remote;
|
||||
}
|
||||
|
||||
function keyOK(string $key): bool
|
||||
|
||||
+61
-8
@@ -11,6 +11,7 @@ import (
|
||||
"log"
|
||||
"net"
|
||||
"net/http"
|
||||
"os"
|
||||
"strings"
|
||||
)
|
||||
|
||||
@@ -56,17 +57,69 @@ func secureCookies(r *http.Request) bool {
|
||||
return httpsRequest(r) || !localRequest(r)
|
||||
}
|
||||
|
||||
// clientIP — адрес посетителя кабинета (за nginx — из X-Real-IP / X-Forwarded-For).
|
||||
// trustedProxies — сети, которым разрешено сообщать адрес клиента заголовками
|
||||
// X-Real-IP / X-Forwarded-For. По умолчанию — только loopback (nginx на том же
|
||||
// хосте). Расширяется списком CIDR/адресов в BJ_TRUSTED_PROXIES (через запятую).
|
||||
var trustedProxies = loadTrustedProxies(os.Getenv("BJ_TRUSTED_PROXIES"))
|
||||
|
||||
func loadTrustedProxies(s string) []*net.IPNet {
|
||||
var out []*net.IPNet
|
||||
for _, part := range strings.Split(s, ",") {
|
||||
part = strings.TrimSpace(part)
|
||||
if part == "" {
|
||||
continue
|
||||
}
|
||||
if !strings.Contains(part, "/") {
|
||||
if strings.Contains(part, ":") {
|
||||
part += "/128"
|
||||
} else {
|
||||
part += "/32"
|
||||
}
|
||||
}
|
||||
if _, n, err := net.ParseCIDR(part); err == nil {
|
||||
out = append(out, n)
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// peerTrusted — непосредственный источник запроса (адрес соединения) — доверенный
|
||||
// прокси: loopback или явно указанный в BJ_TRUSTED_PROXIES.
|
||||
func peerTrusted(host string) bool {
|
||||
ip := net.ParseIP(host)
|
||||
if ip == nil {
|
||||
return false
|
||||
}
|
||||
if ip.IsLoopback() {
|
||||
return true
|
||||
}
|
||||
for _, n := range trustedProxies {
|
||||
if n.Contains(ip) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// clientIP — адрес посетителя кабинета. Заголовки X-Real-IP / X-Forwarded-For
|
||||
// подставляет клиент, поэтому доверяем им ТОЛЬКО когда запрос пришёл от доверенного
|
||||
// прокси (по умолчанию nginx на том же хосте). Иначе адрес берём из соединения —
|
||||
// иначе злоумышленник подделал бы свой IP и обошёл лимит входа и журнал аудита.
|
||||
func clientIP(r *http.Request) string {
|
||||
if v := strings.TrimSpace(r.Header.Get("X-Real-IP")); v != "" {
|
||||
return v
|
||||
}
|
||||
if v := strings.TrimSpace(r.Header.Get("X-Forwarded-For")); v != "" {
|
||||
return strings.TrimSpace(strings.Split(v, ",")[0])
|
||||
}
|
||||
host, _, err := net.SplitHostPort(r.RemoteAddr)
|
||||
if err != nil {
|
||||
return r.RemoteAddr
|
||||
host = r.RemoteAddr
|
||||
}
|
||||
if peerTrusted(host) {
|
||||
if v := strings.TrimSpace(r.Header.Get("X-Real-IP")); v != "" {
|
||||
return v
|
||||
}
|
||||
if v := strings.TrimSpace(r.Header.Get("X-Forwarded-For")); v != "" {
|
||||
// самый правый элемент добавляет наш доверенный прокси (его peer);
|
||||
// левые может подставить клиент, поэтому берём правый.
|
||||
parts := strings.Split(v, ",")
|
||||
return strings.TrimSpace(parts[len(parts)-1])
|
||||
}
|
||||
}
|
||||
return host
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user