15 คะแนน โดย outsideris 2021-03-21 | 1 ความคิดเห็น | แชร์ทาง WhatsApp

เมื่อวันที่ 8 มีนาคม GitHub ได้บังคับให้ออกจากระบบผู้ใช้ทั้งหมดบน GitHub.com เนื่องจากช่องโหว่ด้านความปลอดภัย

  • วันที่ 2 มีนาคม มีรายงานว่าผู้ใช้รายหนึ่งล็อกอินแล้วกลับถูกยืนยันตัวตนเป็นผู้ใช้อีกราย ผู้ใช้คนนั้นออกจากระบบทันที แต่ได้รายงานปัญหานี้ไว้และมีการเริ่มสอบสวนในทันที ไม่กี่ชั่วโมงต่อมา ผู้ใช้อีกรายก็รายงานปัญหาลักษณะคล้ายกัน

  • ผลการตรวจสอบเบื้องต้นพบว่าเซสชันของผู้ใช้รายหนึ่งถูกใช้งานร่วมกันจาก 2 IP ในช่วงเวลาที่มีการรายงาน

  • เมื่อตรวจสอบการเปลี่ยนแปลงโครงสร้างพื้นฐานล่าสุด พบว่ามีการอัปเกรดส่วนของ load balancer และ routing เมื่อไม่นานมานี้ และพบว่ามีการแก้ไข HTTP keepalives ซึ่งดูเหมือนจะเกี่ยวข้อง แต่เมื่อสืบต่อกลับพบว่าไม่เกี่ยวกัน

  • ถึงอย่างนั้น ระหว่างการตรวจสอบโครงสร้างพื้นฐาน ก็พบว่าคำขอที่ได้รับเซสชันผิดถูกประมวลผลบนเครื่องและโปรเซสเดียวกันพอดี

  • หลังตรวจสอบล็อก พบว่า response body เป็นปกติ และมีเพียงคุกกี้ที่ถูกส่งผิด โดยคุกกี้ของผู้ใช้อีกรายที่ถูกประมวลผลในโปรเซสเดียวกันถูกส่งลงไปผิดกรณี ในเคสที่มีรายงาน หนึ่งกรณีเป็นคำขอ 2 ครั้งที่ต่อเนื่องกัน และอีกกรณีหนึ่งมีคำขออื่นคั่นอยู่ 2 ครั้งระหว่างคำขอสองครั้งนั้น

  • จากจุดนี้ จึงตั้งสมมติฐานว่ามีสถานะรั่วไหลระหว่างคำขอที่ถูกประมวลผลใน Ruby โปรเซสเดียวกัน และเริ่มตั้งคำถามว่าสิ่งนี้เกิดขึ้นได้อย่างไร

  • เมื่อตรวจสอบการเปลี่ยนแปลงล่าสุด พบว่ามีการปรับปรุงให้ตรรกะตรวจสอบฟีเจอร์ที่เปิดใช้งานสำหรับผู้ใช้ ซึ่งเดิมทำระหว่างประมวลผลคำขอ เปลี่ยนไปทำใน background thread ที่อัปเดตเป็นช่วง ๆ เพื่อเพิ่มประสิทธิภาพ จึงโฟกัสการสืบสวนไปที่พฤติกรรมด้าน thread safety นี้

  • แอปพลิเคชันหลักของ GitHub.com คือ Ruby on Rails และมีคอมโพเนนต์จำนวนมากที่ไม่ได้ถูกเขียนมาให้ทำงานแบบหลายเธรด

  • แม้ในแอปพลิเคชันจะมีการใช้เธรดอยู่ก่อนแล้ว แต่ background thread ตัวใหม่นี้ทำให้เกิดพฤติกรรมที่ไม่คาดคิดในรูทีนจัดการข้อยกเว้น

  • เมื่อเกิดข้อยกเว้นใน background thread error log จะมีทั้งข้อมูลของ background thread และข้อมูลของคำขอที่กำลังรันอยู่รวมอยู่ด้วย

  • ในตอนแรก คิดว่าการที่ข้อมูลของคำขอที่ไม่เกี่ยวข้องถูกบันทึกลงล็อกจาก background thread เป็นเพียงความไม่สอดคล้องที่เกิดจากปัญหาในระบบรายงานภายใน

  • เนื่องจาก Rails สร้าง controller object ใหม่สำหรับแต่ละคำขอ จึงคิดว่าน่าจะปลอดภัย

  • ดังนั้นจึงยังไม่ชัดเจนว่าปัญหานี้เกิดขึ้นได้อย่างไร

  • จุดเปลี่ยนเริ่มปรากฏเมื่อพบว่า Unicorn ซึ่งใช้งานเป็น Rack HTTP server ในแอปพลิเคชัน Rails ไม่ได้สร้าง env object ใหม่แยกกันสำหรับทุกคำขอ

  • แทนที่จะเป็นเช่นนั้น Unicorn จะจัดสรร Ruby hash สำหรับแต่ละคำขอแล้วล้าง (clear) มัน

  • จากตรงนี้ จึงรู้ว่า logs ของ background thread ไม่ใช่ความคลาดเคลื่อนของระบบรายงาน แต่เป็นการที่ข้อมูลคำขอถูกแชร์กันจริง

  • ทีมพยายามทำให้ race condition นี้เกิดซ้ำในสภาพแวดล้อมพัฒนา และพบว่าการจะเกิดสถานการณ์นี้ได้ต้องเริ่มจากคำขอแบบ anonymous

  1. เมื่อมีคำขอแบบ anonymous เข้ามา (คำขอ #1) จะมีการลงทะเบียน callback กับไลบรารีรายงานข้อผิดพลาด โดย callback นี้เก็บ reference ไปยัง Rails controller object ที่เข้าถึง request environment object ของ Rack ที่ Unicorn จัดเตรียมให้

  2. เมื่อเกิดข้อยกเว้นใน background process ระบบจะคัดลอก context ทั้งหมดเพื่อใช้ในการรายงาน และรวม callback นี้ไว้ด้วย

  3. บน main thread มีคำขอใหม่ที่ล็อกอินแล้วเริ่มขึ้น (คำขอ #2)

  4. ใน background thread ระบบรายงานข้อยกเว้นจะประมวลผล context callback มันพยายามอ่านตัวระบุเซสชันของผู้ใช้ แต่ไม่พบ จึงส่งคำขอไปยังระบบยืนยันตัวตนผ่าน Rails controller ของคำขอ #1 เนื่องจาก Rack ใช้วัตถุเดียวกันกับทุกคำขอ controller จึงไปพบ session cookie ของคำขอ #2

  5. บน main thread คำขอ #2 เสร็จสิ้น

  6. มีคำขอที่ล็อกอินแล้วอีกคำขอหนึ่งเข้ามา (คำขอ #3) การยืนยันตัวตนเสร็จสิ้นแล้ว

  7. ใน background thread controller จะเขียน session cookie ลงใน cookie jar ที่อยู่ใน Rack environment เพื่อให้การยืนยันตัวตนเสร็จสมบูรณ์ ณ เวลานี้ มันคือ cookie jar สำหรับคำขอ #3

  8. ผู้ใช้ได้รับ response ของคำขอ #3 แต่เนื่องจากใน cookie jar ถูกเขียน session cookie ของคำขอ #2 ลงไป ผู้ใช้จึงถูกยืนยันตัวตนเป็นผู้ใช้ของคำขอ #2

สรุปคือ หากเกิดข้อยกเว้นขึ้นและการประมวลผลคำขอหลายชุดเกิดตามลำดับที่ทำให้สถานการณ์นี้สมบูรณ์ เซสชันของ response หนึ่งจะถูกแทนที่ด้วย response ก่อนหน้า สิ่งนี้เกิดขึ้นเฉพาะใน cookie header เท่านั้น ส่วน response อื่น ๆ เช่น HTML ยังคงเป็นข้อมูลของผู้ใช้ที่ผ่านการยืนยันตัวตนเดิม

บั๊กนี้จะเกิดขึ้นเฉพาะเมื่อเงื่อนไขที่ซับซ้อนทั้งหมดนี้เกิดขึ้นพร้อมกันเท่านั้น

  • เพื่อแก้ปัญหานี้ ทีมได้ลบ background thread ที่เพิ่งเพิ่มเข้ามาออก และนำขึ้น production เมื่อวันที่ 5 มีนาคม

  • หลังจากนั้นได้สร้างแพตช์ให้ Unicorn เพื่อไม่ให้มีการแชร์ environment และนำขึ้นใช้งานเมื่อวันที่ 8 มีนาคม

  • หลังวิเคราะห์ล็อกแล้วพบว่าปัญหานี้เกิดขึ้นได้ไม่บ่อย แต่เพื่อจัดการความเสี่ยงที่อาจเกิดขึ้น จึงทำให้เซสชันของผู้ใช้ทั้งหมดเป็นโมฆะ

  • หลังแก้ปัญหาแล้ว GitHub ได้ร่วมมือกับผู้ดูแล Unicorn เพื่อนำการแก้ไขนี้ส่งกลับไปยัง upstream ด้วย

1 ความคิดเห็น

 
kunggom 2021-03-22

การประมวลผลแบบขนานซับซ้อนจริง ๆ ครับ/ค่ะ ช่วงสุดสัปดาห์ผม/ฉันก็เพิ่งปวดหัวอยู่นานเหมือนกันตอนพยายามรันโค้ดที่เพิ่งเขียนไปเมื่อไม่นานนี้แบบขนานตามจำนวนเธรดของ CPU เพื่อใช้ศึกษาเอง ถึงจะทำสำเร็จแล้ว แต่ก็ยังรู้สึกคาใจอยู่นิดหน่อยว่าได้ทำถูกต้องจริงหรือเปล่า